[SP1] 활동후기 페이지 요청 많이 가는 에러 수정 #191

SeojinSeojin · 2023-09-20T07:44:02Z

close 활동후기 페이지의 비정상적인 서버 요청 횟수 수정 #190

Summary

에러가 발생한 이유

원래 아래 경우를 가정하고 코드를 짰습니다.

1페이지 요청 보내기 (status: loading)
1페이지 응답 받기 (status: ok)
2페이지 요청 보내기 (status: loading)
2페이지 응답 받기 (status: ok)

...
그래서 loading일 때는 요청을 안 보내고, ok일 때만 요청을 보낼 수 있도록,
ok인 상황에서만 무한 스크롤 페이저의 count가 증가되도록 코드를 짜 두었어요

However....
실제 경우에는 이렇게 된거죵 ..

1페이지 요청 보내기 (status: loading)
2페이지 요청 보내기 (status: loading)
1페이지 응답 받기 (status: ok)

그럼 2페이지 요청이 진행중인데도 status가 ok라서 다음 페이지들에게도 요청이 간 것입니다!

해결 방법

제 뇌피셜이라 좋은 코드는 아닐지도 몰라요 ..!!
마음껏 피드백 주셔용

1. jobRecordRef 이라는 ref를 만듭니다. (ref인 이유는 state로 쓰기 싫어서 입니다)

const jobRecordRef = useRef<Record<number, 'QUEUED' | 'DONE'>>({});

여기서는 코드가

1페이지 요청 보내기 (status: loading)
2페이지 요청 보내기 (status: loading)
1페이지 응답 받기 (status: ok)
이 상황처럼 꼬이더라도, 아래와 같이 저장됩니다.

// 위 상황에서 jobRecodRef.current 에 저장된 값
{ 1 : 'DONE', 2 : 'QUEUED' };

2. isIncrementable 이라는 함수를 만들어서, 현재 요청들이 다 처리되었는지 확인합니다.

  const isIncrementable = () =>
    Object.values(jobRecordRef.current).every((value) => value === 'DONE');

이 값을 infinite scroll 하는 훅에 넘겨서 , isIncrementable이 false면 (하나라도 'DONE'이 아니면) count를 증가시키지 않습니다.

Comment

아님 아예 집합 형식으로 만들어서,

1페이지 요청 보내기 - [1]
2페이지 요청 보내기 - [1, 2]
1페이지 응답 받기 - [2]
2페이지 응답 받기 - [ ]

이런 식으로 하고, 작업중인게 비어 있으면 요청을 보내지 않도록 해도 괜찮을 것 같기도 합니다 ..!!

SeojinSeojin · 2023-09-20T07:44:49Z

@joohaem 우리 추억이 담긴 코드라 리뷰어 넣었지만 부담은 너무 느끼지 마시구 시간 되실 때에 봐주셔요 . . . .!!!

SeojinSeojin · 2023-09-20T07:45:36Z

src/hooks/useStackedFetchBase/index.ts

@@ -14,7 +13,7 @@ function useStackedFetchBase<T>(
  });

  useEffect(() => {
-    const fetchList = debounce(async () => {
+    const fetchList = async () => {


debounce 는 사용하지 않아서 지웟습니다 ..!!

SeojinSeojin · 2023-09-20T07:46:17Z

src/views/ReviewPage/hooks/useFetch.ts

  const [canGetMoreReviews, setCanGetMoreReviews, setCanNotGetMoreReviews] = useBooleanState(true);

+  useEffect(() => {
+    jobRecordRef.current[count] = 'QUEUED';
+  }, [count]);


이걸 willFetch 함수 안에서 하려 그랬는데,
useCallback은 의존성 배열에 뭐가 담겨 있어도 빠릿빠릿하게 호출되진 않더라구요..

SeojinSeojin · 2023-09-20T07:46:56Z

src/views/ReviewPage/hooks/useFetch.ts

  useEffect(() => {
    function initializeStates() {
      setCount(1);
      setCanGetMoreReviews();
+      for (const key in jobRecordRef.current) {


페이지가 바뀌면 (예시: 전체 선택했다가 기획 선택하면) jobRecordRef.current 를 초기화해줍니다 ({}로 만들어줍니다)

joohaem

😀 WoW 😀

1페이지 요청 보내기 (status: loading)
2페이지 요청 보내기 (status: loading)
1페이지 응답 받기 (status: ok)

요거 그래서 1페이지 요청되면 바로 observer box를 없애서 다음 요청을 막는 식으로 했었던 것 같은데 여전히 이슈가 있었나요????!@@$#%

이러나 저러나 ref 이용한 해결법이 좋네요!

joohaem · 2023-09-20T08:00:33Z

src/views/ReviewPage/hooks/useInfiniteScroll.ts

@@ -1,12 +1,14 @@
 import { useState } from 'react';
 import useIntersectionObserver from '@src/hooks/useIntersectionObserver';

-export default function useInfiniteScroll() {
+export default function useInfiniteScroll(isIncrementable: () => boolean) {


@SeojinSeojin
좋은 인터페이스가 될지에는 의문이 있어요..! '정말 필요한 게 맞나?', '그냥 하나의 엣지케이스를 위해 뚫은 인터페이스이지 않나?' 하는 노파심이랄까요😇😇

그리고 () => boolean 타입인데 is~ 의 이름을 가지는 이유가 무엇일까요?

observer box 를 없애는 방식에 썼던 변수가 제대로 위의 문제로 제대로 업데이트되지 않는 상황이었습니다 ..!!

좋은 인터페이스가 될지에는 의문이 있어요..! ..

맞아요 ....... [스크롤 이벤트]랑 [서버 요청]이랑 알잘딱 독립적으로 진행되도록 수정하는 것이 베스트일 것 같습니다 ..!!!

그러나 요청이 다 제대로 왔는지에 대한 상태를 확인하는 변수로부터 현재 스크롤 가능한지 확인하는 방법도 괜찮을 것 같다고 생각했습니다!
1,2,3,4,...페이지의 요청 완료 여부를 한 변수에서 확인하는 데에는 어려움이 있어서요..!!

그럼 isIncrementable 을 훅에서 리턴해서, 컴포넌트에서 쓰도록 하는것이 좋을까요 ..?
그러나 결국 저것을 위해 사용될 텐데 그래도 자율성을 주는 것이 좋을까요 ..??

그리고 () => boolean 타입인데 is~ 의 이름을 가지는 이유가 무엇일까요?

거의 변수처럼 쓰이는 함수이기 때문이었습니당 헤헤...
좋은 이름으로 고치겠습니다!!

@joohaem 좋은 인터페이스인가를 염려한 이유가 스크롤 이벤트에서 서버에서의 요청 상태를 isIncrementable로 확인하는 부분이 서진이 말처럼 [스크롤 이벤트]랑 [서버 요청]이랑 독립적으로 진행되지 않는다 라서 그런걸까요!?
이해한게 맞는지 궁금해서요~

@f0rever0
A 라서 B 였던 건 아니고, C를 고려하다가 B를 생각해서 서진이가 A라는 해법을 생각한 맥락인 것 같네요?!

C는 무엇이냐 하면, 훅의 인자나 interface나 마찬가지로 "개발"을 위해 뚫는 경우가 많아요. 이건 그 훅이나 객체가 가지는 정체성을 흐리는 행위고, 이렇게 뚫는 인터페이스가 많아질 수록 의존성이 엉키게 되면서 스파게티 코드가 될 가능성이 높아진다고 생각해요
isIncrementable이라는 인자값이 useInfiniteScroll 이라는 훅의 인자값이라고 기대가 되나요? 전 잘 안돼요. 왜냐하면 다른 부분들의 useInfiniteScroll 들에서는 인자값 없이 사용해왔고, 그 자체의 성격을 가지고 있으니까요. 인자값이 추가되어 거기에 의존한다면, 정말 차라리 useInfiniteScroll 이라는 훅의 네이밍이 바뀌어야 하지 않나 싶어요. 혹은 인자값을 훅의 사용자 및 내부가 기대할 수 있게끔 (could~ 이라던가?) 뚫는다거나, 서진이 해법처럼 테마에 따라 나누는 것이 깔끔할 수 있고요.

@joohaem 음 그렇군요!! 더 나은 해법이 있는지 고민해봐야겠네요. 설명 감사합니다 !!! ☺️

@SeojinSeojin

그럼 isIncrementable 을 훅에서 리턴해서, 컴포넌트에서 쓰도록 하는것이 좋을까요 ..?
그러나 결국 저것을 위해 사용될 텐데 그래도 자율성을 주는 것이 좋을까요 ..??

얕게나마 생각해보았지만 역시나만시나 서진이가 짠 방향보다 나은 것은 생각나지 않습니다...
컴포넌트까지 가기는 아닌 것 같고,, 훅 내에서 이렇게 처리하는 게 맞는 정도인 것 같네요!

f0rever0

queue와 done으로 페이지별 상태를 관리하는 아이디어도 좋은 것 같아요!

jobRecordRef 이라는 ref를 만든 이유가 "state로 쓰기 싫어서" 라고 했는데 왜 쓰기 싫었는지 설명해 줄 수 있나요~?( 무언가 불편했다거나,,, 그냥 싫었다거나,,,!)

f0rever0 · 2023-09-20T12:38:56Z

src/views/ReviewPage/hooks/useFetch.ts

@@ -23,6 +36,7 @@ const useFetch = (selected: ExtraPart) => {
    setCanNotGetMoreReviews();


question;

willFetch가 실행될때 setCanNotGetMoreReviews();를 실행하고 api 요청을 하는 이유가 무엇인가요?

@f0rever0
canGetMoreReviews 가 어떤 변수인가요?

useBooleanState 에서 canGetMoreReviews을 false로 만들어 주는 함수인데,
willFetch에서 canGetMoreReviews를 false시키고 response.hasNextPage ? setCanGetMoreReviews() : setCanNotGetMoreReviews();로 api 요청을 통해 다시 값을 재정의 해주는 이유가 궁금했어요~

const willFetch = useCallback(async () => { setCanNotGetMoreReviews(); const response = await api.reviewAPI.getReviews(selected, count); response.hasNextPage ? setCanGetMoreReviews() : setCanNotGetMoreReviews(); jobRecordRef.current[count] = 'DONE'; return response.reviews; }, [selected, count, setCanGetMoreReviews, setCanNotGetMoreReviews]);

f0rever0 · 2023-09-20T12:41:26Z

src/views/ReviewPage/hooks/useFetch.ts

+  useEffect(() => {
+    jobRecordRef.current[count] = 'QUEUED';
+  }, [count]);
+
  useEffect(() => {


question;

-> useEffect에서 의존성으로 setCanGetMoreReviews, setCount 을 넣어주는 이유는 무엇인가요?
두 값은 변하지 않는다고 이해해가지구 selected가 변경될때만 useEffect가 실행되면 되는거 아닌가? 라고 생각이 들었어요!

setCanGetMoreReviews, setCount 가 처음 컴포넌트 읽는 순간부터 unmount 시점까지 안변하나요??

setCanGetMoreReviews가 useBooleanState에서 const setTrue = useCallback(() => { setBool(true); }, []); 이고
setCount는 useInfiniteScroll에서 const [count, setCount] = useState(1);인데
둘다 변수가 아니라고 생각해서 처음 마운트때 이후로 바뀌지 않는다고 이해했어요!

잘못 이해한거라면 알려주세요...!!

오오 생각 정리 좋아요!

이게 이전에는 set~ 함수 또한 변수고, 렌더링 때와 한번 마운트 되고의 값이 다르니 react-hooks/exhaustive-deps 같은 린트에 "set~도 넣어줘!" 라며 warning을 띄워줬는데, 요새에는 사람들도 "stable 한 값인데 이거 넣어야 하냐" 하고 안넣는 추세인 듯 해요!
ref: reactjs/react.dev#4506

아하 그렇게 setState 변수로 보아서 넣는 경우도 있군요!
감사합니다!! 👍

joohaem · 2023-09-21T00:18:11Z

src/views/ReviewPage/hooks/useFetch.ts

+      for (const key in jobRecordRef.current) {
+        if (Object.hasOwnProperty.call(jobRecordRef.current, key)) {
+          delete jobRecordRef.current[key];
+        }
+      }


반복 돌지 않고 joRecordRef.current = {} 때려주는 게 "initialize"에 좀 더 적합하지 않나 싶어요 !!
그렇게 않은 이유가 있을까요???

근간은 아니지만,, delete 키워드 찾다가 생각하게 된 것입니다😇
https://javascript.plainenglish.io/avoid-the-delete-keyword-in-javascript-87ff2a47f26c

SeojinSeojin · 2023-09-21T00:46:27Z

귀중한 리뷰덜 넘 감사합니다 .!.!! 이 은혜 멋진 코드로 꼭 갚겠습니다!!!

However.. 지금 뇌를 제대로 쓸 수 있는 상황이 아니라서요, 2-3시 정도에 고심 후 답변을 드리겠습니다!!

SeojinSeojin · 2023-09-23T04:36:28Z

여러분 .. 어느새 2days ago가 되었네요 ..... 귀중한 리뷰를 보며 해답을 계속 생각해보겠습니다 ..!!
일단 정말 간단한 해결책이 있는데 제가 못 찾은 것일 수 있으니 그것을 뒤적뒤적 해보고, 안 되면 인터페이스라도 깔끔하게 보이도록 코드를 짜보겠습니다!!!

SeojinSeojin · 2023-10-05T15:10:05Z

useIntersectionObserver
useFetch

이 친구들이 무결하게 작동하는지 확인한 후에 윗단 코드를 고칠 수 있을 것 같아요!!
이 PR은 일단 close하고 계속 알아보겠습니다!!

fix: restrict request in infinite scroll

ad24e89

pull-request-size bot added the size/S label Sep 20, 2023

SeojinSeojin requested review from joohaem, f0rever0 and solar3070 September 20, 2023 07:44

SeojinSeojin commented Sep 20, 2023

View reviewed changes

joohaem reviewed Sep 20, 2023

View reviewed changes

SeojinSeojin requested a review from joohaem September 20, 2023 08:10

f0rever0 approved these changes Sep 20, 2023

View reviewed changes

joohaem reviewed Sep 21, 2023

View reviewed changes

solar3070 assigned SeojinSeojin Sep 22, 2023

SeojinSeojin closed this Oct 5, 2023

solar3070 deleted the refactor/#190_review-page-infinite-scroll branch October 26, 2023 08:42

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[SP1] 활동후기 페이지 요청 많이 가는 에러 수정 #191

[SP1] 활동후기 페이지 요청 많이 가는 에러 수정 #191

SeojinSeojin commented Sep 20, 2023 •

edited

Loading

SeojinSeojin commented Sep 20, 2023

SeojinSeojin Sep 20, 2023

SeojinSeojin Sep 20, 2023

SeojinSeojin Sep 20, 2023

joohaem left a comment •

edited

Loading

joohaem Sep 20, 2023

SeojinSeojin Sep 20, 2023

SeojinSeojin Sep 20, 2023

SeojinSeojin Sep 20, 2023

f0rever0 Sep 20, 2023

joohaem Sep 21, 2023 •

edited

Loading

f0rever0 Sep 21, 2023

joohaem Sep 21, 2023 •

edited

Loading

f0rever0 left a comment

f0rever0 Sep 20, 2023

joohaem Sep 21, 2023

f0rever0 Sep 21, 2023

f0rever0 Sep 20, 2023

joohaem Sep 21, 2023

f0rever0 Sep 21, 2023

joohaem Sep 21, 2023 •

edited

Loading

f0rever0 Sep 21, 2023

joohaem Sep 21, 2023

joohaem Sep 21, 2023

SeojinSeojin commented Sep 21, 2023

SeojinSeojin commented Sep 23, 2023

SeojinSeojin commented Oct 5, 2023

		@@ -23,6 +36,7 @@ const useFetch = (selected: ExtraPart) => {
		setCanNotGetMoreReviews();

[SP1] 활동후기 페이지 요청 많이 가는 에러 수정 #191

[SP1] 활동후기 페이지 요청 많이 가는 에러 수정 #191

Conversation

SeojinSeojin commented Sep 20, 2023 • edited Loading

Summary

에러가 발생한 이유

해결 방법

Comment

SeojinSeojin commented Sep 20, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

joohaem left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

joohaem Sep 21, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

joohaem Sep 21, 2023 • edited Loading

Choose a reason for hiding this comment

f0rever0 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

joohaem Sep 21, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SeojinSeojin commented Sep 21, 2023

SeojinSeojin commented Sep 23, 2023

SeojinSeojin commented Oct 5, 2023

SeojinSeojin commented Sep 20, 2023 •

edited

Loading

joohaem left a comment •

edited

Loading

joohaem Sep 21, 2023 •

edited

Loading

joohaem Sep 21, 2023 •

edited

Loading

joohaem Sep 21, 2023 •

edited

Loading