Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FE] refactor: MSW 환경에서 무한 스크롤 API 응답을 서버와 통일하고, 빈 리뷰에 대한 옵저버 로직 개선 #754

Merged
merged 5 commits into from
Oct 2, 2024
Merged

[FE] refactor: MSW 환경에서 무한 스크롤 API 응답을 서버와 통일하고, 빈 리뷰에 대한 옵저버 로직 개선 #754

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
798c037
refactor: useInfiniteScroll에서 isLastPage 제거
soosoo22 Sep 28, 2024
45fe30e
refactor: 목 핸들러에서 마지막 리뷰 id를 반환하는 로직 개선
soosoo22 Sep 28, 2024
6aae5ee
refactor: 리뷰가 더이상 없을 경우, hasNextPage를 false로 반환
soosoo22 Sep 28, 2024
63c09c3
refactor: 목 데이터에서 lastReviewId 값 수정
soosoo22 Sep 28, 2024
7b562ac
refactor: isLastPage를 사용한 fetchNextPage 호출
soosoo22 Oct 2, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion frontend/src/hooks/review/useGetReviewList/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,9 @@ const useGetReviewList = () => {
staleTime: 1 * 60 * 1000,
});

return result;
const hasNextPage = !result.data.pages.some((page) => page.reviews.length === 0);
Copy link
Contributor

@BadaHertz52 BadaHertz52 Oct 2, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

서버에서 내려주는 isLastPage를 사용하지 않은 이유가 있나요?
리뷰 데이터가 많아질 수록 배열 시간 복잡도가 올라가서 isLastPage를 사용하지 않고 배열을 순회해야하는 것에 의문이 들어서요

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image

서버에서 isLastPage를 true 주고 다시 한번 api 요청을 해서, 불필요한 api 요청이 추가로 가는 것이 아닌가 싶네요

Copy link
Contributor Author

@soosoo22 soosoo22 Oct 2, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

서버에서 내려주는 isLastPage를 사용하지 않은 이유가 있나요? 리뷰 데이터가 많아질 수록 배열 시간 복잡도가 올라가서 isLastPage를 사용하지 않고 배열을 순회해야하는 가라는 것에 의문이 들어서요

isLastPage를 사용해서 옵저버를 해제하면 무한스크롤로 받은 리뷰를 다 확인했을 때 살짝 올라갔다가 내려가면 다시 첫 리뷰를 불러오면서 리뷰가 무한 증식이 되어버립니다... 제가 보기엔 isLastReviewId가 0이면 첫 리뷰를 불러오고 있는데 거기서 문제가 있는게 아닌가... 싶네요..

⭐️해결 완료⭐️

콘솔로 isLastPage를 찍어서 확인해보니 false만을 넘겨주고 있더라고요. isLastPage가 false로만 넘어가던 이유는, 데이터를 받는 과정에서 가장 첫 번째 페이지의 isLastPage 값을 사용하고 있었어요. 실제로는 마지막으로 받은 페이지의 isLastPage 값을 넘겨줘야 했던 상황이었는데 말이죠...

수정한 방식인 data.pages[data.pages.length - 1].isLastPage;를 넘겨주는 방식으로 수정했습니다. 마지막 페이지의 데이터를 기준으로 isLastPage 값을 설정해주어서, 올바르게 동작하네요!

Copy link
Contributor Author

@soosoo22 soosoo22 Oct 2, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

서버에서 isLastPage를 true 주고 다시 한번 api 요청을 해서, 불필요한 api 요청이 추가로 가는 것이 아닌가 싶네요

맞습니다... 이 부분은 수정해야 겠네요.

⭐️ 해결 완료 ⭐️

isLastPage를 활용해서 true를 받으면 바로 옵저버에 fetchNextPage 함수가 호출되지 않게 수정했습니다. => 추가 api 요청이 가지 않습니다.


return { ...result, hasNextPage };
};

export default useGetReviewList;
2 changes: 1 addition & 1 deletion frontend/src/mocks/handlers/review.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ const getReviewList = (lastReviewId: number | null, size: number) => {
return HttpResponse.json({
revieweeName: REVIEW_LIST.revieweeName,
projectName: REVIEW_LIST.projectName,
lastReviewId: !isLastPage && lastReviewId !== null ? lastReviewId + size : null,
lastReviewId: paginatedReviews.length > 0 ? paginatedReviews[paginatedReviews.length - 1].reviewId : 0,
isLastPage: isLastPage,
reviews: paginatedReviews,
});
Expand Down
2 changes: 1 addition & 1 deletion frontend/src/mocks/mockData/reviewListMockData.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { ReviewList } from '@/types';
export const REVIEW_LIST: ReviewList = {
revieweeName: '쑤쑤',
projectName: 'review-me',
lastReviewId: 12,
lastReviewId: 24,
isLastPage: false,
reviews: [
{
Expand Down
1 change: 0 additions & 1 deletion frontend/src/pages/ReviewListPage/components/PageContents/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ const PageContents = () => {
fetchNextPage,
hasNextPage,
isLoading,
isLastPage: data.pages[0].isLastPage,
});

const handleReviewClick = (id: number) => {
Expand Down
7 changes: 3 additions & 4 deletions frontend/src/pages/ReviewListPage/hooks/useInfiniteScroll.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,14 @@ export interface InfiniteScrollProps {
fetchNextPage: () => void;
hasNextPage: boolean;
isLoading: boolean;
isLastPage: boolean;
}

const useInfiniteScroll = ({ fetchNextPage, hasNextPage, isLoading, isLastPage }: InfiniteScrollProps) => {
const useInfiniteScroll = ({ fetchNextPage, hasNextPage, isLoading }: InfiniteScrollProps) => {
const observer = useRef<IntersectionObserver | null>(null);

const lastElementRef = useCallback(
(node: HTMLElement | null) => {
if (isLoading || isLastPage) return;
if (isLoading) return;
if (observer.current) observer.current.disconnect();

observer.current = new IntersectionObserver((entries) => {
Expand All @@ -23,7 +22,7 @@ const useInfiniteScroll = ({ fetchNextPage, hasNextPage, isLoading, isLastPage }

if (node) observer.current.observe(node);
},
[isLoading, fetchNextPage, hasNextPage, isLastPage],
[isLoading, fetchNextPage, hasNextPage],
);

return lastElementRef;
Expand Down