-
Notifications
You must be signed in to change notification settings - Fork 0
[Feat] Cursor Pagination 기반 무한 스크롤 구현 #177
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.
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
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughConverts the group list page from client-side hook-based data fetching to a server-side async component that pre-fetches initial data. Implements infinite scroll functionality with a new GroupList component that uses IntersectionObserver and a custom hook to load additional pages on demand. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Server
participant HomePage as HomePage<br/>(Server)
participant API as API Service
participant Browser
participant GroupList
participant useInfiniteGroupList as useInfiniteGroupList<br/>Hook
participant IntersectionObserver
User->>Browser: Navigate to group list page
Browser->>HomePage: Request page
HomePage->>API: Call groupService.getGroups<br/>(size=10, cursor=null)
API-->>HomePage: Return {items, nextCursor}
HomePage->>GroupList: Render with initialItems<br/>& initialCursor
GroupList->>Browser: Render Card list + sentinel
Browser-->>User: Display initial groups
rect rgb(200, 220, 255)
Note over User,IntersectionObserver: Infinite Scroll Flow
User->>Browser: Scroll down
IntersectionObserver->>IntersectionObserver: Sentinel becomes visible
IntersectionObserver->>useInfiniteGroupList: Trigger onIntersect callback
useInfiniteGroupList->>API: fetchNext()<br/>Call groupService.getGroups<br/>(size=10, cursor=nextCursor)
API-->>useInfiniteGroupList: Return {items, nextCursor}
useInfiniteGroupList->>useInfiniteGroupList: Append items to state<br/>Update nextCursor
useInfiniteGroupList->>GroupList: Update state
GroupList->>Browser: Re-render with additional Cards
Browser-->>User: Display more groups
end
alt Error Occurs
useInfiniteGroupList->>GroupList: Store error in state
GroupList->>ErrorMessage: Display error + retry button
User->>ErrorMessage: Click retry
ErrorMessage->>useInfiniteGroupList: Call handleRetry()
useInfiniteGroupList->>API: Retry fetchNext()
end
alt No More Data
API-->>useInfiniteGroupList: Return {items: [], nextCursor: null}
useInfiniteGroupList->>GroupList: nextCursor = null
GroupList->>Browser: Show "모든 모임을 불러왔습니다" message
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Areas requiring extra attention:
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
🎭 Playwright Report✨ E2E Test가 성공적으로 완료되었습니다. Test 요약 내용을 확인해주세요.
📊 Test Summary
📜 Test Details✅ Passed Tests (9)
|
🎨 Storybook Report✅ 변경 사항이 없습니다 모든 Story가 이전 빌드와 동일합니다.
|
📊 Coverage Report
📉 #177을 main에 병합하면 coverage가 Coverage 요약@@ Coverage Diff @@
## main #177 +/- ##
===========================================
- Coverage 37.72% 35.89% -1.83%
===========================================
Files 138 143 +5
Lines 5567 5850 +283
Branches 226 231 +5
===========================================
Hits 2100 2100 0
+ Misses 3467 3750 +283 영향받은 파일✅ 이 PR로 영향받은 파일이 없습니다 수정된 모든 파일이 현재 coverage를 유지했습니다. |
🚀 PR Preview Report✨ Build가 성공적으로 완료되었습니다. Preview에서 변경사항을 확인하세요.
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🧹 Nitpick comments (3)
src/components/shared/error-message/index.tsx (1)
7-17: Consider using conditional className concatenation.The current template literal concatenation can produce trailing whitespace when
classNameis an empty string. Consider using a utility likeclsxor conditional logic for cleaner class merging.Apply this diff to improve className handling:
-export const ErrorMessage = ({ className = '', message, onRetry }: ErrorMessageProps) => ( - <div className={`flex flex-col items-center justify-center gap-4 ${className}`}> +export const ErrorMessage = ({ className, message, onRetry }: ErrorMessageProps) => ( + <div className={`flex flex-col items-center justify-center gap-4${className ? ` ${className}` : ''}`}>Alternatively, if
clsxis available in the project, use it for cleaner merging:import clsx from 'clsx'; export const ErrorMessage = ({ className, message, onRetry }: ErrorMessageProps) => ( <div className={clsx('flex flex-col items-center justify-center gap-4', className)}>src/components/pages/group-list/index.tsx (1)
31-67: Consider addressing the TODO comment.Line 66 contains a Korean comment "// 이후 수정 예정" (to be modified later), indicating planned future changes to the "all data loaded" message.
Would you like me to open a tracking issue for this planned UI refinement, or would you prefer to address it in this PR?
src/hooks/use-group/use-group-infinite-list/index.ts (1)
46-86: Consider removing or conditionalizing extensive console logging.The code contains extensive
console.logstatements (lines 55-58, 66-72, 79, 105-110, 122-129, 132-135, 143, 188-194) with a comment requesting they not be deleted (line 46: "콘솔은 지우지 말아주세요 🙏🏻").While useful for debugging, these logs will run in production and clutter the console. Consider:
- Removing them before production deployment
- Wrapping them in a debug flag check
- Using a proper logging library with configurable levels
Example with a debug flag:
const DEBUG = process.env.NODE_ENV === 'development'; // Then wrap logs: if (DEBUG) { console.log('다음 페이지 요청 시작', { '요청 크기': GROUP_LIST_PAGE_SIZE, // ... }); }Or define a debug utility:
const debug = (...args: unknown[]) => { if (process.env.NODE_ENV === 'development') { console.log(...args); } };Also applies to: 98-148
📜 Review details
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
src/app/page.tsx(1 hunks)src/components/pages/group-list/index.tsx(1 hunks)src/components/shared/error-message/index.tsx(1 hunks)src/components/shared/index.ts(1 hunks)src/hooks/use-group/use-group-infinite-list/index.ts(1 hunks)src/hooks/use-intersection-observer/index.ts(1 hunks)src/lib/constants/group-list.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
src/components/shared/error-message/index.tsx (1)
src/components/shared/index.ts (1)
ErrorMessage(3-3)
src/app/page.tsx (3)
src/api/index.ts (1)
API(22-22)src/lib/constants/group-list.ts (1)
GROUP_LIST_PAGE_SIZE(1-1)src/components/pages/group-list/index.tsx (1)
GroupList(17-71)
src/hooks/use-group/use-group-infinite-list/index.ts (3)
src/types/service/group.ts (1)
GroupListItemResponse(2-21)src/lib/constants/group-list.ts (1)
GROUP_LIST_PAGE_SIZE(1-1)src/api/index.ts (1)
API(22-22)
src/hooks/use-intersection-observer/index.ts (1)
src/lib/constants/group-list.ts (1)
INTERSECTION_OBSERVER_THRESHOLD(2-2)
🔇 Additional comments (5)
src/components/shared/index.ts (1)
3-3: LGTM!The export follows the existing pattern and correctly re-exports the new ErrorMessage component.
src/lib/constants/group-list.ts (1)
1-2: LGTM!The constants are well-defined with appropriate values. A page size of 10 and an intersection threshold of 0.1 (10% visibility) are sensible defaults for infinite scrolling.
src/app/page.tsx (1)
7-16: Consider adding error handling for the server-side API call.The API call on line 8 lacks explicit error handling. If
API.groupService.getGroupsthrows an error, it will propagate to Next.js's error boundary. While this is functional, consider whether you want to add explicit error handling for a more controlled user experience (e.g., displaying a custom error page or fallback UI).Verify whether the current error propagation behavior aligns with your intended UX. If explicit error handling is needed, consider wrapping the API call in a try-catch block and rendering an error state.
src/components/pages/group-list/index.tsx (1)
17-29: LGTM! Well-structured infinite scroll implementation.The component correctly separates concerns:
- Uses
useInfiniteGroupListfor data management- Uses
useIntersectionObserverfor scroll detection- Properly disables the observer when there's no more data or an error
The enabled condition
nextCursor !== null && error === nullensures the observer only triggers when appropriate.src/hooks/use-group/use-group-infinite-list/index.ts (1)
187-196: LGTM! Theexhaustive-depsdisable is correct here.The empty dependency array for the initial logging effect is intentional—it should only run once on mount. The
eslint-disablecomment on line 195 is appropriate.
| const fetchNext = useCallback(async (): Promise<void> => { | ||
| if (!canFetchNext()) { | ||
| return; | ||
| } | ||
|
|
||
| isFetchingRef.current = true; | ||
|
|
||
| console.log('다음 페이지 요청 시작', { | ||
| '요청 크기': GROUP_LIST_PAGE_SIZE, | ||
| '현재 커서': nextCursor, | ||
| '현재 누적 데이터 개수': items.length, | ||
| 키워드: keyword || '없음', | ||
| }); | ||
|
|
||
| try { | ||
| const response = await API.groupService.getGroups({ | ||
| keyword, | ||
| cursor: nextCursor as number, | ||
| size: GROUP_LIST_PAGE_SIZE, | ||
| }); | ||
|
|
||
| const previousItemsCount = items.length; | ||
| const newItemsCount = previousItemsCount + response.items.length; | ||
|
|
||
| console.log('다음 페이지 요청 완료', { | ||
| '요청 크기': GROUP_LIST_PAGE_SIZE, | ||
| '받은 데이터 개수': response.items.length, | ||
| '이전 누적 데이터 개수': previousItemsCount, | ||
| '새로운 누적 데이터 개수': newItemsCount, | ||
| '다음 커서': response.nextCursor, | ||
| 키워드: keyword || '없음', | ||
| }); | ||
|
|
||
| if (response.nextCursor === null) { | ||
| console.log('모든 데이터 로드 완료', { | ||
| '총 데이터 개수': newItemsCount, | ||
| 키워드: keyword || '없음', | ||
| }); | ||
| } | ||
|
|
||
| setItems((prevItems) => [...prevItems, ...response.items]); | ||
| setNextCursor(response.nextCursor); | ||
| setError(null); | ||
| } catch (err) { | ||
| const error = createError(err, '다음 페이지를 불러오는데 실패했습니다.'); | ||
| console.error('다음 페이지 조회 실패:', error); | ||
| setError(error); | ||
| } finally { | ||
| isFetchingRef.current = false; | ||
| } | ||
| }, [canFetchNext, nextCursor, keyword, items.length, createError]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Dependency on items.length causes fetchNext to recreate frequently.
The fetchNext function includes items.length in its dependencies (line 148), causing it to be recreated every time items are added. This is used only for console logging (lines 108, 119-120, 125).
This frequent recreation has cascading performance implications:
fetchNextis passed asonIntersecttouseIntersectionObserver- The observer recreates on every
onIntersectchange - Observer disconnect/reconnect is expensive and happens after every data fetch
Solution: Remove items.length from dependencies or use a ref pattern.
Option 1 - Use a ref for logging (recommended):
export const useInfiniteGroupList = ({
initialCursor,
initialItems,
initialKeyword,
}: UseInfiniteGroupListParams): UseInfiniteGroupListReturn => {
const [keyword, setKeyword] = useState<string | undefined>(initialKeyword);
const [items, setItems] = useState<GroupListItemResponse[]>(initialItems);
const [nextCursor, setNextCursor] = useState<number | null>(initialCursor);
const [error, setError] = useState<Error | null>(null);
const isFetchingRef = useRef(false);
const prevKeywordRef = useRef(initialKeyword);
+ const itemsRef = useRef(items);
+
+ useEffect(() => {
+ itemsRef.current = items;
+ }, [items]);Then update the fetchNext function to use the ref:
const fetchNext = useCallback(async (): Promise<void> => {
if (!canFetchNext()) {
return;
}
isFetchingRef.current = true;
console.log('다음 페이지 요청 시작', {
'요청 크기': GROUP_LIST_PAGE_SIZE,
'현재 커서': nextCursor,
- '현재 누적 데이터 개수': items.length,
+ '현재 누적 데이터 개수': itemsRef.current.length,
키워드: keyword || '없음',
});
try {
const response = await API.groupService.getGroups({
keyword,
cursor: nextCursor as number,
size: GROUP_LIST_PAGE_SIZE,
});
- const previousItemsCount = items.length;
+ const previousItemsCount = itemsRef.current.length;
const newItemsCount = previousItemsCount + response.items.length;
// ... rest of the function
}
- }, [canFetchNext, nextCursor, keyword, items.length, createError]);
+ }, [canFetchNext, nextCursor, keyword, createError]);Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/hooks/use-group/use-group-infinite-list/index.ts around lines 98 to 148,
fetchNext currently lists items.length in its dependency array which forces
recreation on every item append and causes the intersection observer to
reconnect; change to track items length with a ref instead of depending on
items.length for logging: create an itemsCountRef that you update whenever items
change (e.g., in the setItems updater or an effect) and use that ref inside
fetchNext for console messages, then remove items.length from the useCallback
dependency array so fetchNext only depends on stable values (canFetchNext,
nextCursor, keyword, createError).
| const handleRetry = useCallback(() => { | ||
| setError(null); | ||
| if (items.length === 0) { | ||
| fetchFirstPage(initialKeyword); | ||
| } else { | ||
| fetchNext(); | ||
| } | ||
| }, [items.length, initialKeyword, fetchFirstPage, fetchNext]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Potential keyword inconsistency in handleRetry.
Line 156 calls fetchFirstPage(initialKeyword), but the user may have updated the keyword since initialization. The current keyword state might differ from initialKeyword, leading to unexpected behavior where retry uses a stale keyword.
Consider using the current keyword state instead:
const handleRetry = useCallback(() => {
setError(null);
if (items.length === 0) {
- fetchFirstPage(initialKeyword);
+ fetchFirstPage(keyword);
} else {
fetchNext();
}
-}, [items.length, initialKeyword, fetchFirstPage, fetchNext]);
+}, [items.length, keyword, fetchFirstPage, fetchNext]);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const handleRetry = useCallback(() => { | |
| setError(null); | |
| if (items.length === 0) { | |
| fetchFirstPage(initialKeyword); | |
| } else { | |
| fetchNext(); | |
| } | |
| }, [items.length, initialKeyword, fetchFirstPage, fetchNext]); | |
| const handleRetry = useCallback(() => { | |
| setError(null); | |
| if (items.length === 0) { | |
| fetchFirstPage(keyword); | |
| } else { | |
| fetchNext(); | |
| } | |
| }, [items.length, keyword, fetchFirstPage, fetchNext]); |
🤖 Prompt for AI Agents
In src/hooks/use-group/use-group-infinite-list/index.ts around lines 153 to 160,
handleRetry currently calls fetchFirstPage(initialKeyword) which can use a stale
value if the user changed the keyword; change it to call fetchFirstPage(keyword)
so the retry uses the current keyword state, and update the useCallback
dependency array to include keyword (and any other needed stable refs) to avoid
stale closures.
| useEffect(() => { | ||
| const target = targetRef.current; | ||
| if (!target || !enabled) return; | ||
|
|
||
| const observer = new IntersectionObserver( | ||
| (entries) => { | ||
| if (entries[0]?.isIntersecting) { | ||
| onIntersect(); | ||
| } | ||
| }, | ||
| { | ||
| threshold, | ||
| }, | ||
| ); | ||
|
|
||
| observer.observe(target); | ||
|
|
||
| return () => { | ||
| observer.disconnect(); | ||
| }; | ||
| }, [onIntersect, enabled, threshold]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IntersectionObserver recreates on every onIntersect change.
The effect depends on onIntersect (line 38), which will be a new function reference on each parent re-render if not memoized. This causes the observer to disconnect and reconnect frequently, which is inefficient.
In the current usage (GroupList passes fetchNext from useInfiniteGroupList), fetchNext is recreated whenever items.length changes, leading to observer recreation on every data fetch.
Solution: Use a ref to store the latest callback and avoid including it in dependencies.
Apply this diff:
export const useIntersectionObserver = ({
onIntersect,
enabled = true,
threshold = INTERSECTION_OBSERVER_THRESHOLD,
}: UseIntersectionObserverParams) => {
const targetRef = useRef<HTMLDivElement>(null);
+ const callbackRef = useRef(onIntersect);
+
+ // Update callback ref without triggering effect
+ useEffect(() => {
+ callbackRef.current = onIntersect;
+ }, [onIntersect]);
useEffect(() => {
const target = targetRef.current;
if (!target || !enabled) return;
const observer = new IntersectionObserver(
(entries) => {
if (entries[0]?.isIntersecting) {
- onIntersect();
+ callbackRef.current();
}
},
{
threshold,
},
);
observer.observe(target);
return () => {
observer.disconnect();
};
- }, [onIntersect, enabled, threshold]);
+ }, [enabled, threshold]);
return targetRef;
};Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/hooks/use-intersection-observer/index.ts around lines 18 to 38, the
effect currently depends on the onIntersect callback causing the
IntersectionObserver to be recreated whenever the parent supplies a new function
reference; change this by creating a ref (e.g., latestCallbackRef) to hold the
latest onIntersect, update that ref in a separate useEffect when onIntersect
changes, and modify the observer effect to read from latestCallbackRef.current
instead of depending on onIntersect; remove onIntersect from the observer effect
dependencies (keep enabled and threshold), and if necessary add an ESLint
comment to suppress the exhaustive-deps warning.
Chiman2937
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
수고하셨습니다!
HopeFullee
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
굿또 잡
📝 변경 사항
Cursor Pagination 기반 무한 스크롤 구현
핵심 설계 원칙
nextCursor값 유무로만 다음 요청 여부 판단하여 중복 호출 방지구현 세부 사항
1. Server Component (
src/app/page.tsx)initialItems)와 커서 값(initialCursor)을 Client Component로 전달2. Client Component (
src/components/pages/group-list/index.tsx)useInfiniteGroupList훅으로 무한 스크롤 상태 관리useIntersectionObserver훅으로 sentinel 요소 감지3. 무한 스크롤 로직 훅 (
src/hooks/use-group/use-group-infinite-list/index.ts)items(현재까지 로드된 모임 목록),nextCursor(다음 페이지 커서),error(에러 상태)fetchFirstPage: 키워드 변경 시 첫 페이지 재요청fetchNext: 다음 페이지 요청 (중복 요청 방지 로직 포함)canFetchNext: 요청 가능 여부 판단 (nextCursor !== null && !isFetching)handleRetry: 에러 발생 시 재시도 기능reset: 상태 초기화initialKeyword변경 시 자동으로 첫 페이지 재요청4. IntersectionObserver 훅 (
src/hooks/use-intersection-observer/index.ts)threshold기본값 0.1 (10%)로 설정하여 적절한 시점에 다음 페이지 요청 (이후 수정 가능)5. 에러 메시지 컴포넌트 (
src/components/shared/error-message/index.tsx)6. 상수 정의 (
src/lib/constants/group-list.ts)GROUP_LIST_PAGE_SIZE: 페이지당 로드할 모임 개수 (10개)INTERSECTION_OBSERVER_THRESHOLD: IntersectionObserver 감지 임계값 (0.1 = 10%)컴포넌트
src/app/page.tsx: 모임 목록 초기 데이터 SSR 구현src/components/pages/group-list/index.tsx: 모임 목록 무한 스크롤 UI 컴포넌트 구현src/components/shared/error-message/index.tsx: 에러 메시지 공유 컴포넌트 추가커스텀 훅
src/hooks/use-group/use-group-infinite-list/index.ts: 모임 목록 무한 스크롤 로직 커스텀 훅 구현src/hooks/use-intersection-observer/index.ts: IntersectionObserver 커스텀 훅 구현상수 및 유틸리티
src/lib/constants/group-list.ts: 모임 목록 관련 상수 정의개선 사항
🔗 관련 이슈
Closes #96
🧪 테스트 방법
📸 스크린샷 (선택)
📋 체크리스트
💬 추가 코멘트
변경 사항
루트 페이지(
app/page.tsx)의 모임 목록 UI를components/pages/group-list/index.tsx로 분리했습니다.변경 이유
1. Next.js App Router 아키텍처 요구사항
무한 스크롤은
useState,useEffect,IntersectionObserver등 클라이언트 전용 기능이 필요하므로 Client Component여야 합니다.2. 성능 최적화
page.tsx): 초기 데이터를 서버에서 로드 → 빠른 첫 화면 렌더링GroupList): 이후 스크롤 시 클라이언트에서 추가 데이터 로드3. 재사용성
GroupList컴포넌트와useInfiniteGroupList훅을 다른 페이지에서도 재사용 가능합니다. (이후 스케줄러 페이지 예정)4. 설계 원칙
구조 (결론)
CodeRabbit Review는 자동으로 실행되지 않습니다.
Review를 실행하려면 comment에 아래와 같이 작성해주세요
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.