Feat(client): article intersection observer로 infinite scroll 구현#158
Feat(client): article intersection observer로 infinite scroll 구현#158constantly-dev merged 7 commits intodevelopfrom
Conversation
Walkthrough리마인드와 나의 북마크 목록을 단일페이지 쿼리에서 useInfiniteQuery 기반 페이지네이션으로 전환하고, IntersectionObserver 기반의 공용 useInfiniteScroll 훅을 도입해 스크롤 센티넬로 추가 페이지 자동 로드를 구현. UI는 pages 병합, 배지/빈상태 카운트를 첫 페이지 데이터로 계산하도록 변경. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant UI as MyBookmark/Remind Page
participant Hook as useInfiniteScroll
participant RQ as react-query useInfiniteQuery
participant API as Backend API
rect rgba(230,243,255,0.6)
note over UI,RQ: 초기 로드
UI->>RQ: useInfiniteQuery(init: pageParam=0)
RQ->>API: GET page=0,size=20
API-->>RQ: { articles[], counts, ... }
RQ-->>UI: data.pages[0]
end
rect rgba(235,255,235,0.6)
note over UI,Hook: 무한 스크롤 관찰 등록
UI->>Hook: root(containerRef), hasNextPage, fetchNextPage
Hook-->>UI: observerRef
UI->>Hook: 센티넬에 ref 연결
Hook->>Hook: IntersectionObserver 등록
end
alt 센티넬 가시
Hook->>RQ: fetchNextPage()
RQ->>API: GET page=n,size=20
API-->>RQ: { articles[] } or {}
RQ-->>UI: data.pages 업데이트
else 더 없음
Hook->>Hook: hasNextPage=false → 관찰 중단
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests
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. 🧪 Early access (Sonnet 4.5): enabledWe are currently testing the Sonnet 4.5 model, which is expected to improve code review quality. However, this model may lead to increased noise levels in the review comments. Please disable the early access features if the noise level causes any inconvenience. Note:
Comment |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
✅ Storybook chromatic 배포 확인: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/client/src/pages/remind/Remind.tsx (1)
121-154: 스크롤 컨테이너 높이 설정 필요스크롤 컨테이너에 명시적인 높이를 설정하지 않으면 무한 스크롤이 제대로 작동하지 않을 수 있습니다. 현재
overflow-y-auto만 설정되어 있어 컨테이너가 뷰포트를 초과할 수 있습니다.<div ref={scrollContainerRef} - className="scrollbar-hide mt-[2.6rem] flex flex-wrap gap-[1.6rem] overflow-y-auto scroll-smooth" + className="scrollbar-hide mt-[2.6rem] flex h-[calc(100vh-20rem)] flex-wrap gap-[1.6rem] overflow-y-auto scroll-smooth" >
🧹 Nitpick comments (8)
apps/client/src/pages/remind/apis/queries.ts (1)
5-15: 빈 페이지 추가 호출 방지: PAGE_SIZE 기준으로 next page 계산 제안마지막 페이지가 "비어있지 않지만 더 이상 다음 페이지가 없는" 경우에도 한 번 더 빈 페이지 요청이 발생할 수 있습니다. 현재는 length === 0에서만 종료하기 때문입니다. 요청 크기(20)에 맞춰 length < PAGE_SIZE이면 종료하도록 바꾸면 불필요한 네트워크 호출 1회를 줄일 수 있습니다. 또한 매직넘버 20은 상수화하는 편이 좋습니다.
아래 변경을 권장합니다:
export const useGetRemindArticles = (nowDate: string, readStatus: boolean) => { return useInfiniteQuery({ queryKey: ['remindArticles', nowDate, readStatus], - queryFn: ({ pageParam = 0 }) => - getRemindArticles(nowDate, readStatus, pageParam, 20), + queryFn: ({ pageParam = 0 }) => + getRemindArticles(nowDate, readStatus, pageParam, PAGE_SIZE), initialPageParam: 0, getNextPageParam: (lastPage, allPages) => { - if (lastPage.articles.length === 0) { - return undefined; - } - return allPages.length; + return lastPage.articles.length < PAGE_SIZE ? undefined : allPages.length; }, }); };상단 임포트 아래에 상수 추가:
const PAGE_SIZE = 20;apps/client/src/pages/myBookmark/apis/queries.ts (2)
8-18: 불필요한 마지막 빈 페이지 요청 방지 및 페이지 크기 상수화세 훅 모두 마지막 페이지에서 length === 0일 때만 중단되어, 실제 마지막 페이지가 꽉 차지 않았을 때 불필요한 추가 호출 1회가 발생할 수 있습니다. 요청 크기(20)에 맞춰 length < PAGE_SIZE 시 종료하도록 변경하고, 매직넘버를 상수화하는 것을 권장합니다.
아래와 같이 적용하면 됩니다:
export const useGetBookmarkArticles = () => { return useInfiniteQuery({ queryKey: ['bookmarkReadArticles'], - queryFn: ({ pageParam = 0 }) => getBookmarkArticles(pageParam, 20), + queryFn: ({ pageParam = 0 }) => getBookmarkArticles(pageParam, PAGE_SIZE), initialPageParam: 0, getNextPageParam: (lastPage, allPages) => { - if (lastPage.articles.length === 0) { - return undefined; - } - return allPages.length; + return lastPage.articles.length < PAGE_SIZE ? undefined : allPages.length; }, }); } export const useGetBookmarkUnreadArticles = () => { return useInfiniteQuery({ queryKey: ['bookmarkUnreadArticles'], - queryFn: ({ pageParam = 0 }) => getBookmarkUnreadArticles(pageParam, 20), + queryFn: ({ pageParam = 0 }) => + getBookmarkUnreadArticles(pageParam, PAGE_SIZE), initialPageParam: 0, getNextPageParam: (lastPage, allPages) => { - if (lastPage.articles.length === 0) { - return undefined; - } - return allPages.length; + return lastPage.articles.length < PAGE_SIZE ? undefined : allPages.length; }, }); } export const useGetCategoryBookmarkArticles = ( categoryId: string | null, readStatus: boolean | null ) => { return useInfiniteQuery({ queryKey: ['categoryBookmarkArticles', readStatus, categoryId], queryFn: ({ pageParam = 0 }) => - getCategoryBookmarkArticles(categoryId, readStatus, pageParam, 20), + getCategoryBookmarkArticles(categoryId, readStatus, pageParam, PAGE_SIZE), initialPageParam: 0, getNextPageParam: (lastPage, allPages) => { - if (lastPage.articles.length === 0) { - return undefined; - } - return allPages.length; + return lastPage.articles.length < PAGE_SIZE ? undefined : allPages.length; }, enabled: !!categoryId, }); }파일 상단에 상수 추가:
const PAGE_SIZE = 20;추가로, 앱 전역에서 재사용한다면 shared/constants 등으로 분리하는 것도 고려해 주세요.
Also applies to: 22-33, 40-50
51-51: enabled 조건 확인 요청
enabled: !!categoryId는 categoryId가 빈 문자열('')이면 비활성화됩니다. 라우팅/상태에서 ''가 유효한 값으로 올 수 있는지 확인 부탁드립니다. 만약 null만 "없음"으로 간주한다면enabled: categoryId !== null이 더 안전합니다.apps/client/src/shared/hooks/useInfiniteScroll.ts (2)
3-8: fetchNextPage 중복 트리거 가능성 완화 및 옵저버 정리 강화관찰 영역에 오래 머물면 콜백이 반복 호출될 수 있어 fetchNextPage가 중복 트리거될 여지가 있습니다. React Query가 내부적으로 어느 정도 방어하더라도,
isFetchingNextPage가드로 중복 호출을 막는 편이 안전합니다. 또한 cleanup에서disconnect()로 옵저버 리소스를 확실히 해제하는 것을 권장합니다. 프리패치를 원하면 rootMargin 옵션도 유용합니다.interface UseInfiniteScrollProps { fetchNextPage: () => void; hasNextPage?: boolean; - root?: React.RefObject<HTMLElement | null>; - threshold?: number; + root?: React.RefObject<HTMLElement | null>; + threshold?: number; + isFetchingNextPage?: boolean; + rootMargin?: string; } export const useInfiniteScroll = ({ fetchNextPage, hasNextPage, root, - threshold = 0.5, + threshold = 0.5, + isFetchingNextPage = false, + rootMargin = '0px 0px 300px 0px', }: UseInfiniteScrollProps) => { const targetRef = useRef<HTMLDivElement>(null); useEffect(() => { - if (!hasNextPage) return; + if (!hasNextPage || isFetchingNextPage) return; const observer = new IntersectionObserver( ([entry]) => { - if (entry.isIntersecting) { + if (entry.isIntersecting && !isFetchingNextPage) { fetchNextPage(); } }, { root: root?.current, - threshold, + threshold, + rootMargin, } ); const currentTarget = targetRef.current; if (currentTarget) { observer.observe(currentTarget); } return () => { if (currentTarget) { observer.unobserve(currentTarget); } + observer.disconnect(); }; - }, [fetchNextPage, hasNextPage, root, threshold]); + }, [fetchNextPage, hasNextPage, isFetchingNextPage, root, threshold, rootMargin]); return targetRef; };Also applies to: 15-21, 23-31, 33-35, 43-49, 48-48
6-7: root 파라미터 타입 범위 완화(선택)재사용성을 높이려면
root?: Element | null을 직접 받는 방식도 고려해볼 수 있습니다. 현재 RefObject만 허용되어 외부에서 직접 DOM 엘리먼트를 넘기기 어렵습니다.apps/client/src/pages/remind/Remind.tsx (2)
84-93: 빈 상태 컴포넌트 로직 개선 가능첫 페이지 데이터를 기반으로 빈 상태를 판단하는 로직은 적절하나, 데이터가 없을 때의 방어 로직을 추가하면 더 안전합니다.
const EmptyStateComponent = () => { const firstPageData = data?.pages[0]; + if (!firstPageData) { + return <NoRemindArticles />; + } if ( firstPageData?.readArticleCount === 0 && firstPageData?.unreadArticleCount === 0 ) { return <NoRemindArticles />; }
153-153: 센티넬 요소 스타일 개선 권장센티넬 요소를 인라인 스타일로 정의하는 대신 className을 사용하면 더 일관성 있는 코드가 됩니다.
- <div ref={observerRef} style={{ height: '1px', width: '100%' }} /> + <div ref={observerRef} className="h-[1px] w-full" />apps/client/src/pages/myBookmark/MyBookmark.tsx (1)
101-107: 쿼리키 팩토리 패턴 TODO 처리TODO 주석에 따르면 쿼리키 팩토리 패턴 적용이 필요합니다. 이는 쿼리키 관리를 일관성 있게 하고 타입 안정성을 높이는 중요한 리팩토링입니다.
쿼리키 팩토리 패턴 구현을 도와드릴까요? 중앙화된 쿼리키 관리 시스템을 생성하여 타입 안정성과 유지보수성을 개선할 수 있습니다.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
apps/client/src/pages/myBookmark/MyBookmark.tsx(8 hunks)apps/client/src/pages/myBookmark/apis/queries.ts(1 hunks)apps/client/src/pages/remind/Remind.tsx(7 hunks)apps/client/src/pages/remind/apis/queries.ts(1 hunks)apps/client/src/shared/hooks/useInfiniteScroll.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: constantly-dev
PR: Pinback-Team/pinback-client#2
File: pnpm-workspace.yaml:3-3
Timestamp: 2025-08-18T13:48:59.065Z
Learning: constantly-dev는 docs 디렉터리를 컨벤션 문서 추가용으로 사용할 예정이라고 명시했습니다.
🧬 Code graph analysis (4)
apps/client/src/pages/myBookmark/MyBookmark.tsx (3)
apps/client/src/shared/apis/queries.ts (2)
usePutArticleReadStatus(92-100)useDeleteRemindArticle(102-106)apps/client/src/pages/myBookmark/apis/queries.ts (3)
useGetBookmarkArticles(8-20)useGetBookmarkUnreadArticles(22-34)useGetCategoryBookmarkArticles(36-53)apps/client/src/shared/hooks/useInfiniteScroll.ts (1)
useInfiniteScroll(15-51)
apps/client/src/pages/remind/Remind.tsx (2)
apps/client/src/pages/remind/apis/queries.ts (1)
useGetRemindArticles(4-17)apps/client/src/shared/hooks/useInfiniteScroll.ts (1)
useInfiniteScroll(15-51)
apps/client/src/pages/myBookmark/apis/queries.ts (1)
apps/client/src/pages/myBookmark/apis/axios.ts (3)
getBookmarkArticles(3-8)getBookmarkUnreadArticles(10-15)getCategoryBookmarkArticles(17-34)
apps/client/src/pages/remind/apis/queries.ts (1)
apps/client/src/pages/remind/apis/axios.ts (1)
getRemindArticles(3-13)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: storybook
🔇 Additional comments (3)
apps/client/src/pages/remind/apis/queries.ts (1)
7-14: 응답 스키마/페이지 인덱스 확인 요청
- API 응답이 항상
{ articles: Article[] }형태인지 확인 부탁드립니다. 필드명이 달라질 경우 런타임 오류가 발생합니다.- 서버 페이지 인덱스가 0‑base(초깃값 0)인지 재확인 부탁드립니다. 1‑base라면 initialPageParam과 getNextPageParam 로직 조정이 필요합니다.
apps/client/src/pages/remind/Remind.tsx (1)
44-48: 무한 스크롤 훅 사용 방식 적절함
useInfiniteScroll훅을 사용하여 Intersection Observer 패턴을 깔끔하게 구현했습니다. root 컨테이너를 명시적으로 전달하는 것도 좋은 접근입니다.apps/client/src/pages/myBookmark/MyBookmark.tsx (1)
42-61: readStatus 파라미터 동작 검증 필요
activeBadge === 'notRead' ? false : null로 전달되는readStatus값이 API의read-status파라미터(true: 읽은 글, false: 읽지 않은 글, null: 전체)와 의도한 필터링(미읽음/전체) 결과를 반환하는지 확인해주세요.
| const articlesToDisplay = category | ||
| ? categoryArticles?.articles | ||
| ? (categoryArticlesData?.pages.flatMap((page) => page.articles) ?? []) | ||
| : activeBadge === 'all' | ||
| ? articles?.articles | ||
| : unreadArticles?.articles; | ||
| ? (articlesData?.pages.flatMap((page) => page.articles) ?? []) | ||
| : (unreadArticlesData?.pages.flatMap((page) => page.articles) ?? []); | ||
|
|
||
| const hasNextPage = category | ||
| ? hasNextCategoryArticles | ||
| : activeBadge === 'all' | ||
| ? hasNextArticles | ||
| : hasNextUnreadArticles; | ||
|
|
||
| const fetchNextPage = category | ||
| ? fetchNextCategoryArticles | ||
| : activeBadge === 'all' | ||
| ? fetchNextArticles | ||
| : fetchNextUnreadArticles; | ||
|
|
||
| const observerRef = useInfiniteScroll({ | ||
| fetchNextPage, | ||
| hasNextPage, | ||
| root: scrollContainerRef, | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
복잡한 조건 로직 리팩토링 권장
현재 3개의 삼항 연산자를 중첩하여 사용하고 있어 가독성이 떨어집니다. 헬퍼 함수나 useMemo를 활용한 리팩토링을 권장합니다.
+ const getActiveQueryData = () => {
+ if (category) return categoryArticlesData;
+ return activeBadge === 'all' ? articlesData : unreadArticlesData;
+ };
+
+ const activeData = getActiveQueryData();
+
const articlesToDisplay = category
? (categoryArticlesData?.pages.flatMap((page) => page.articles) ?? [])
: activeBadge === 'all'
? (articlesData?.pages.flatMap((page) => page.articles) ?? [])
: (unreadArticlesData?.pages.flatMap((page) => page.articles) ?? []);또는 더 간단하게:
- const articlesToDisplay = category
- ? (categoryArticlesData?.pages.flatMap((page) => page.articles) ?? [])
- : activeBadge === 'all'
- ? (articlesData?.pages.flatMap((page) => page.articles) ?? [])
- : (unreadArticlesData?.pages.flatMap((page) => page.articles) ?? []);
+ const activeData = category
+ ? categoryArticlesData
+ : activeBadge === 'all' ? articlesData : unreadArticlesData;
+
+ const articlesToDisplay = activeData?.pages.flatMap((page) => page.articles) ?? [];📝 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 articlesToDisplay = category | |
| ? categoryArticles?.articles | |
| ? (categoryArticlesData?.pages.flatMap((page) => page.articles) ?? []) | |
| : activeBadge === 'all' | |
| ? articles?.articles | |
| : unreadArticles?.articles; | |
| ? (articlesData?.pages.flatMap((page) => page.articles) ?? []) | |
| : (unreadArticlesData?.pages.flatMap((page) => page.articles) ?? []); | |
| const hasNextPage = category | |
| ? hasNextCategoryArticles | |
| : activeBadge === 'all' | |
| ? hasNextArticles | |
| : hasNextUnreadArticles; | |
| const fetchNextPage = category | |
| ? fetchNextCategoryArticles | |
| : activeBadge === 'all' | |
| ? fetchNextArticles | |
| : fetchNextUnreadArticles; | |
| const observerRef = useInfiniteScroll({ | |
| fetchNextPage, | |
| hasNextPage, | |
| root: scrollContainerRef, | |
| }); | |
| const activeData = category | |
| ? categoryArticlesData | |
| : activeBadge === 'all' ? articlesData : unreadArticlesData; | |
| const articlesToDisplay = activeData?.pages.flatMap((page) => page.articles) ?? []; | |
| const hasNextPage = category | |
| ? hasNextCategoryArticles | |
| : activeBadge === 'all' | |
| ? hasNextArticles | |
| : hasNextUnreadArticles; | |
| const fetchNextPage = category | |
| ? fetchNextCategoryArticles | |
| : activeBadge === 'all' | |
| ? fetchNextArticles | |
| : fetchNextUnreadArticles; | |
| const observerRef = useInfiniteScroll({ | |
| fetchNextPage, | |
| hasNextPage, | |
| root: scrollContainerRef, | |
| }); |
🤖 Prompt for AI Agents
In apps/client/src/pages/myBookmark/MyBookmark.tsx around lines 74 to 96, the
nested ternary logic selecting articlesToDisplay, hasNextPage, and fetchNextPage
is hard to read; refactor by extracting the selection into a small helper or
useMemo: create a useMemo that returns {articlesToDisplay, hasNextPage,
fetchNextPage} based on category and activeBadge, use safe null-coalescing for
pages.flatMap, and include all reactive variables (category, activeBadge and the
various data/flags/fetch functions) in the dependency array so values are
memoized and readable; then pass the memoized values into useInfiniteScroll.
| const EmptyStateComponent = () => { | ||
| if (articles?.totalArticle === 0) { | ||
| return <NoArticles />; | ||
| if (articlesToDisplay.length === 0) { | ||
| const totalArticlesInAllView = articlesData?.pages[0]?.totalArticle; | ||
| if (totalArticlesInAllView === 0) { | ||
| return <NoArticles />; | ||
| } | ||
| return <NoUnreadArticles />; | ||
| } | ||
| return <NoUnreadArticles />; | ||
| return null; | ||
| }; |
There was a problem hiding this comment.
EmptyState 로직 개선 필요
빈 상태 판단 로직이 복잡하고 일관성이 부족합니다. articlesToDisplay.length === 0 체크와 totalArticlesInAllView === 0 체크가 중복될 수 있습니다.
const EmptyStateComponent = () => {
- if (articlesToDisplay.length === 0) {
- const totalArticlesInAllView = articlesData?.pages[0]?.totalArticle;
- if (totalArticlesInAllView === 0) {
- return <NoArticles />;
- }
- return <NoUnreadArticles />;
+ const firstPageData = activeData?.pages[0];
+ if (!firstPageData || firstPageData.totalArticle === 0) {
+ return <NoArticles />;
+ }
+ if (articlesToDisplay.length === 0 && activeBadge === 'notRead') {
+ return <NoUnreadArticles />;
}
return null;
};Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In apps/client/src/pages/myBookmark/MyBookmark.tsx around lines 125 to 134, the
EmptyStateComponent mixes two overlapping checks (articlesToDisplay.length and
totalArticle from pages) making the logic redundant and hard to maintain;
compute a single definitive value for totalArticles (e.g., derive
totalArticlesInAllView once from articlesData?.pages — or sum pages'
totalArticle if needed) and then branch clearly: if totalArticlesInAllView === 0
return <NoArticles />, else if articlesToDisplay.length === 0 return
<NoUnreadArticles />, otherwise return null; move the totalArticles computation
outside the render body and remove nested/duplicated checks so the component
reads as a simple three-way decision.
| <div | ||
| ref={scrollContainerRef} | ||
| className="scrollbar-hide mt-[2.6rem] flex h-screen flex-wrap content-start gap-[1.6rem] overflow-y-auto scroll-smooth" | ||
| > |
There was a problem hiding this comment.
스크롤 컨테이너 높이 중복 설정
h-screen이 두 번 설정되어 있습니다 (Line 147과 182). Line 182의 h-screen은 제거하고 적절한 높이 계산이 필요합니다.
<div
ref={scrollContainerRef}
- className="scrollbar-hide mt-[2.6rem] flex h-screen flex-wrap content-start gap-[1.6rem] overflow-y-auto scroll-smooth"
+ className="scrollbar-hide mt-[2.6rem] flex h-[calc(100vh-16rem)] flex-wrap content-start gap-[1.6rem] overflow-y-auto scroll-smooth"
>📝 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.
| <div | |
| ref={scrollContainerRef} | |
| className="scrollbar-hide mt-[2.6rem] flex h-screen flex-wrap content-start gap-[1.6rem] overflow-y-auto scroll-smooth" | |
| > | |
| <div | |
| ref={scrollContainerRef} | |
| className="scrollbar-hide mt-[2.6rem] flex h-[calc(100vh-16rem)] flex-wrap content-start gap-[1.6rem] overflow-y-auto scroll-smooth" | |
| > |
🤖 Prompt for AI Agents
In apps/client/src/pages/myBookmark/MyBookmark.tsx around lines 180-183 (and
also note the earlier instance at line 147), the scroll container has a
duplicate h-screen class; remove the h-screen on the element at ~line 182 and
replace it with a calculated/relative height (e.g., min-h-full,
h-[calc(100vh-<headerHeight>)] or a container-specific class) so the layout
relies on one definitive full-screen height (the original at line 147) and the
scrollable area computes its height correctly.
| const { data, isPending, fetchNextPage, hasNextPage } = useGetRemindArticles( | ||
| formattedDate, | ||
| activeBadge === 'read', | ||
| 0, | ||
| 10 | ||
| activeBadge === 'read' | ||
| ); |
There was a problem hiding this comment.
데이터 접근 시 옵셔널 체이닝 필요
useGetRemindArticles 훅이 데이터를 반환하지 않을 수 있으므로, 안전한 데이터 접근을 위해 옵셔널 체이닝을 사용해야 합니다.
- const { data, isPending, fetchNextPage, hasNextPage } = useGetRemindArticles(
+ const { data, isPending, fetchNextPage, hasNextPage = false } = useGetRemindArticles(
formattedDate,
activeBadge === 'read'
);Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In apps/client/src/pages/remind/Remind.tsx around lines 39 to 42, the call to
useGetRemindArticles can return undefined data so any subsequent accesses must
use optional chaining; update all places that read from data (e.g., data.pages,
data.map, data.flat, data.length, etc.) to use data?., provide safe fallbacks
where appropriate (e.g., [] for arrays or 0 for lengths), and ensure conditional
rendering checks like if (!data) or data?.pages?.length before rendering to
avoid runtime errors.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (3)
apps/client/src/pages/myBookmark/MyBookmark.tsx (3)
74-90: 중첩 삼항 연산자 리팩토링 필요 (이전 리뷰 지적사항 미반영)이전 리뷰에서 지적된 중첩 삼항 연산자의 가독성 문제가 여전히 남아있습니다.
articlesToDisplay,hasNextPage,fetchNextPage선택 로직을 헬퍼 함수나useMemo로 추출하는 것을 권장합니다.예시:
const activeQueryData = useMemo(() => { if (category) { return { data: categoryArticlesData, fetchNextPage: fetchNextCategoryArticles, hasNextPage: hasNextCategoryArticles, }; } return activeBadge === 'all' ? { data: articlesData, fetchNextPage: fetchNextArticles, hasNextPage: hasNextArticles, } : { data: unreadArticlesData, fetchNextPage: fetchNextUnreadArticles, hasNextPage: hasNextUnreadArticles, }; }, [category, activeBadge, categoryArticlesData, articlesData, unreadArticlesData, /* ... */]); const articlesToDisplay = activeQueryData.data?.pages.flatMap((page) => page.articles) ?? [];
180-183: 중복 h-screen 클래스 제거 필요 (이전 리뷰 지적사항 미반영)이전 리뷰에서 지적된
h-screen중복 문제가 여전히 존재합니다. Line 147의 외부 컨테이너와 Line 182의 스크롤 컨테이너 모두h-screen을 사용하고 있어 레이아웃 문제가 발생할 수 있습니다.Line 182의 스크롤 컨테이너는 계산된 높이를 사용해야 합니다:
<div ref={scrollContainerRef} - className="scrollbar-hide mt-[2.6rem] flex h-screen flex-wrap content-start gap-[1.6rem] overflow-y-auto scroll-smooth" + className="scrollbar-hide mt-[2.6rem] flex h-[calc(100vh-16rem)] flex-wrap content-start gap-[1.6rem] overflow-y-auto scroll-smooth" >
125-134: EmptyState 로직 오류: 활성 뷰를 고려하지 않음
EmptyStateComponent가 항상articlesData(전체보기)의totalArticle만 확인하고 있어, 카테고리 뷰나 안 읽음 필터 활성화 시 잘못된 빈 상태를 표시할 수 있습니다.예를 들어:
- 카테고리에 아티클이 없지만 전체 북마크에는 있을 때:
NoUnreadArticles표시 (잘못됨)- 안 읽음 필터에서 아티클이 없지만 전체에는 있을 때: 의도대로 작동하지만 로직이 불명확
활성 뷰의 데이터를 기반으로 판단하도록 수정이 필요합니다:
const EmptyStateComponent = () => { if (articlesToDisplay.length === 0) { - const totalArticlesInAllView = articlesData?.pages[0]?.totalArticle; - if (totalArticlesInAllView === 0) { + const activeViewTotalArticles = category + ? categoryArticlesData?.pages[0]?.totalArticle + : articlesData?.pages[0]?.totalArticle; + + if (activeViewTotalArticles === 0) { return <NoArticles />; } return <NoUnreadArticles />; } return null; };
🧹 Nitpick comments (2)
apps/client/src/pages/remind/Remind.tsx (2)
39-42: hasNextPage에 기본값 제공 권장
useInfiniteQuery에서 반환되는hasNextPage는 초기에undefined일 수 있습니다.useInfiniteScroll훅에서if (!hasNextPage)체크로 처리되지만, 명시적인 기본값 제공이 더 안전합니다.다음 diff를 적용하여 기본값을 추가하세요:
- const { data, isPending, fetchNextPage, hasNextPage } = useGetRemindArticles( + const { data, isPending, fetchNextPage, hasNextPage = false } = useGetRemindArticles( formattedDate, activeBadge === 'read' );
121-154: 옵저버 센티넬 div의 높이 증가 고려무한 스크롤 구현이 올바르게 되어 있으나, 옵저버 div의 높이가
1px로 매우 작습니다. IntersectionObserver의 threshold 값(기본 0.5)과 결합될 때 트리거 타이밍이 불안정할 수 있습니다.다음과 같이 높이를 늘리는 것을 고려하세요:
- <div ref={observerRef} style={{ height: '1px', width: '100%' }} /> + <div ref={observerRef} className="h-[10px] w-full" />이렇게 하면:
- 더 안정적인 intersection 감지
- Tailwind 클래스로 일관된 스타일링
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/client/src/pages/myBookmark/MyBookmark.tsx(8 hunks)apps/client/src/pages/remind/Remind.tsx(7 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
apps/client/src/pages/remind/Remind.tsx (2)
apps/client/src/pages/remind/apis/queries.ts (1)
useGetRemindArticles(4-17)apps/client/src/shared/hooks/useInfiniteScroll.ts (1)
useInfiniteScroll(15-51)
apps/client/src/pages/myBookmark/MyBookmark.tsx (3)
apps/client/src/shared/apis/queries.ts (2)
usePutArticleReadStatus(92-100)useDeleteRemindArticle(102-106)apps/client/src/pages/myBookmark/apis/queries.ts (3)
useGetBookmarkArticles(8-20)useGetBookmarkUnreadArticles(22-34)useGetCategoryBookmarkArticles(36-53)apps/client/src/shared/hooks/useInfiniteScroll.ts (1)
useInfiniteScroll(15-51)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: storybook
🔇 Additional comments (13)
apps/client/src/pages/remind/Remind.tsx (6)
1-1: LGTM: 필요한 임포트가 적절히 추가되었습니다.무한 스크롤 구현을 위한
useRef와useInfiniteScroll훅 임포트가 올바르게 추가되었습니다.Also applies to: 20-20
27-27: LGTM: 스크롤 컨테이너 ref가 적절히 선언되었습니다.
scrollContainerRef가 올바르게 선언되어 무한 스크롤의 루트 컨테이너로 사용됩니다.
44-48: LGTM: useInfiniteScroll 훅 사용이 적절합니다.무한 스크롤 훅에 필요한 파라미터들이 올바르게 전달되었습니다.
58-58: LGTM: 페이지네이션 데이터 병합이 올바릅니다.
data.pages를 올바르게 병합하여 전체 아티클 배열을 생성했으며, 옵셔널 체이닝과 널 병합 연산자로 안전하게 처리했습니다.
83-93: LGTM: 빈 상태 처리 로직이 안전합니다.첫 페이지 데이터를 기반으로 빈 상태를 판단하는 로직이 올바르며, 옵셔널 체이닝으로 안전하게 처리했습니다.
100-101: LGTM: 배지 카운트 계산이 안전합니다.첫 페이지 데이터에서 카운트를 추출하는 로직이 안전하게 구현되었습니다.
apps/client/src/pages/myBookmark/MyBookmark.tsx (7)
2-2: LGTM: Import 변경사항 적절
useRef,useInfiniteScroll추가 및FetchCard경로를 alias로 변경한 것이 무한 스크롤 기능 구현에 적합합니다.Also applies to: 23-24
37-37: LGTM: scrollContainerRef 선언 적절IntersectionObserver의 root로 사용될 스크롤 컨테이너 ref가 올바르게 선언되었습니다.
42-61: LGTM: useInfiniteQuery 훅 사용 적절세 가지 뷰(전체, 안 읽음, 카테고리)에 대한 페이지네이션 훅이 일관되게 구성되었으며,
categoryArticlesData가activeBadge에 따라readStatus를 올바르게 전달합니다.
92-96: LGTM: useInfiniteScroll 통합 적절IntersectionObserver 기반 무한 스크롤이 올바르게 통합되었습니다.
category나activeBadge변경 시fetchNextPage/hasNextPage가 업데이트되어 observer가 재생성되는 것은 예상된 동작입니다.
136-144: LGTM: 배지 카운트 계산 적절카테고리 뷰와 전체 뷰를 구분하여 첫 페이지의
totalArticle및totalUnreadArticle을 올바르게 사용하고 있습니다.
215-215: LGTM: Sentinel 요소 배치 적절IntersectionObserver의 타겟 sentinel 요소가 스크롤 컨테이너 내부의 리스트 끝에 올바르게 배치되었습니다. 최소 높이(1px) 사용이 적절합니다.
102-107: LGTM: 쿼리 무효화 로직 적절삭제 및 읽음 상태 변경 후 관련된 모든 쿼리(
bookmarkReadArticles,bookmarkUnreadArticles,categoryBookmarkArticles,arcons)를 무효화하는 것이 적절합니다.arcons무효화는 아카이브/카테고리 트리의 카운트 업데이트를 위해 필요한 것으로 보입니다.TODO 코멘트에 언급된 쿼리키 팩토리 패턴 적용은 향후 코드 중복을 줄이고 유지보수성을 높이는 좋은 개선 방향입니다.
Also applies to: 193-202
jllee000
left a comment
jllee000
left a comment
There was a problem hiding this comment.
인터섹션 옵서버! 저도 언젠간 써봐야지 했는데 이런식으로 활용하면 되겠군요!
고민하신 지점은, 저도 생각해봤는데 !!
저희 서비스 특성에 맞게 고려해봤을때, 이미지 여러개 보여지는 구간이 대시보드 쪽에서만 있고, 있더라도 무한스크롤 기능이랑 엮여 있는 형태라,, 따로 훅 분리 없이 이런식으로 가도 괜찮지 않을까 싶어요!
|
|
||
| const observer = new IntersectionObserver( | ||
| ([entry]) => { | ||
| if (entry.isIntersecting) { | ||
| fetchNextPage(); | ||
| } | ||
| }, | ||
| { | ||
| root: root?.current, |
There was a problem hiding this comment.
observer 활용해서, 다음 스크롤 페이지 연결되는 지점인걸까요? 와우! 이런식으로 쓸 수 있군요 배워갑니다,,!
| (category | ||
| ? categoryArticlesData?.pages[0]?.totalArticle | ||
| : articlesData?.pages[0]?.totalArticle) ?? 0; | ||
|
|
||
| const totalUnreadArticleCount = | ||
| (category | ||
| ? categoryArticlesData?.pages[0]?.totalUnreadArticle | ||
| : articlesData?.pages[0]?.totalUnreadArticle) ?? 0; |
There was a problem hiding this comment.
가독성이 떨어지는 것 같기도 한데, 변수로 따로 빼는건 불필요한 메모리만 쓰게 되는걸까요? (just 질문)
2 participants
📌 Related Issues
📄 Tasks
⭐ PR Point (To Reviewer)
intersection observer를 이용해서 무한 스크롤 추가했습니다.remind와 mybookmark 페이지에서 재사용돼서
useInfiniteScroll.ts에 커스텀 훅으로 분리했어요.저희 필터링이 다 다른 API로 분리되어 있어서
data나fetchNextPage같은 상태 분기 처리를 하다보니 코드가 너무 지저분하고 복잡하네요...차라리 이
badge값에 따른 분기 처리 관련 로직을 커스텀 훅으로 분리해서 역할 분리를 해주는 것이 더 좋을 것 같다는 생각도 들어서 리팩토링 할 때 다시 고쳐볼게요.그리고 지금
useInfiniteScroll훅이intersection observer와 무한 스크롤의 기능을 모두 합친 훅이어서 재사용을 할 때 조금 활용도가 떨어질 수 있을 것이라고 생각해요. 아예toss simplekit에 있는useIntersectionObserver처럼 observer 관련 훅을 추상화하고, infinite scroll은 사용하는 곳에서 정의하도록 하는 것은 어떨까 생각이 드는데 팀원 분들은 어떻게 생각하시나요??toss simplekit
📷 Screenshot
2025-09-27.6.22.55.mov
Summary by CodeRabbit