Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Walkthrough여러 페이지에서 데이터 로딩 및 무한 스크롤 로직을 Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/pages/mypage/SavePage.tsx (1)
38-66:⚠️ Potential issue | 🟡 Minor
Feed.tsx와 동일하게minLoadingTime패턴이 빠져 있습니다.FollowerListPage,SearchBook,Memory등 다른 페이지에서는 첫 페이지 로딩 시 500ms 최소 지연을 적용하고 있으나, 여기에는 없습니다. 빠른 네트워크에서 스켈레톤 깜빡임이 발생할 수 있습니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/mypage/SavePage.tsx` around lines 38 - 66, The savedFeeds and savedBooks useInifinieScroll calls are missing the minLoadingTime pattern which causes skeleton flicker on fast networks; update both useInifinieScroll invocations (the ones that define fetchPage for getSavedFeedsInMy and getSavedBooksInMy) to include a minLoadingTime option of 500 (apply it so it affects initial load — i.e., when cursor is null/undefined) following the same pattern used in Feed.tsx/Followers/SearchBook/Memory to ensure the first-page loading shows at least 500ms.src/pages/feed/FeedDetailPage.tsx (1)
137-145:⚠️ Potential issue | 🟡 Minor
window.close()가 중복 호출됩니다.Line 138에서
window.close()가 무조건 호출된 후, Line 141에서 다시 호출됩니다. 첫 번째 호출 이후if/else블록은 사실상 도달할 수 없는 코드입니다. 이 PR에서 도입된 변경은 아니지만, 의도한 동작은window.opener가 있을 때만window.close()하고 그렇지 않으면navigate(-1)하는 것으로 보입니다.🐛 수정 제안
const handleBackClick = () => { - window.close(); - if (window.opener) { window.close(); } else { navigate(-1); } };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/feed/FeedDetailPage.tsx` around lines 137 - 145, The handleBackClick function currently calls window.close() unconditionally and again inside the if block, making the first call redundant; update handleBackClick so it only checks window.opener and calls window.close() when window.opener is truthy, otherwise call navigate(-1). Locate the function by name handleBackClick and adjust the conditional logic (referencing window.opener and navigate) to remove the duplicate unconditional window.close() call.src/pages/feed/Feed.tsx (1)
43-74:⚠️ Potential issue | 🟡 Minor
fetchPage에 500ms 최소 로딩 시간이 없습니다.다른 페이지들(
FollowerListPage,SearchBook,Memory등)은 첫 페이지 로딩 시minLoadingTime(500ms)을 적용하여 스켈레톤이 너무 짧게 깜빡이는 것을 방지하고 있습니다.Feed.tsx의totalFeed와myFeed의fetchPage에는 이 패턴이 빠져 있어, 빠른 네트워크 환경에서 스켈레톤이 순간적으로 깜빡일 수 있습니다.Based on learnings, Feed 페이지 초기 로딩 지연은 500ms여야 합니다.
🔧 수정 제안 (totalFeed 예시, myFeed에도 동일 적용)
const totalFeed = useInifinieScroll<PostData>({ enabled: activeTab === '피드', reloadKey: activeTab, fetchPage: async cursor => { await waitForToken(); + const minLoadingTime = cursor ? null : new Promise(resolve => setTimeout(resolve, 500)); const response = await getTotalFeeds(cursor ? { cursor } : undefined); + if (minLoadingTime) await minLoadingTime; return { items: response.data.feedList, nextCursor: response.data.nextCursor || null, isLast: response.data.isLast, }; },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/feed/Feed.tsx` around lines 43 - 74, The fetchPage implementations in totalFeed and myFeed must enforce a 500ms minimum loading time for the initial page load to prevent skeleton flicker: inside each fetchPage (in the useInifinieScroll calls for totalFeed and myFeed) record start time before awaiting waitForToken(), run the network request (getTotalFeeds/getMyFeeds) and in parallel await a delay so that when cursor is falsy (initial load) the total elapsed time is at least 500ms, then return the response as before; implement the delay by computing remaining = 500 - (Date.now() - start) and awaiting a sleep/timeout only if remaining > 0, leaving non-initial pages unchanged.
🧹 Nitpick comments (5)
src/pages/groupSearch/GroupSearch.tsx (2)
44-48:fetchRecentSearches가 useEffect 의존성 배열에 누락되었습니다.Line 41의 마운트 effect에는 eslint-disable 주석이 있지만, 이 effect(Line 44-48)에서는
fetchRecentSearches를 호출하면서 의존성 배열에 포함하지 않았습니다.useCallback으로 감싸져 있어 현재는 안정적인 참조이지만,fetchRecentSearches를 의존성에 추가하는 것이 좋습니다.♻️ 수정 제안
useEffect(() => { if (searchStatus === 'idle') { fetchRecentSearches(); } - }, [searchStatus]); + }, [searchStatus, fetchRecentSearches]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/groupSearch/GroupSearch.tsx` around lines 44 - 48, The effect that calls fetchRecentSearches when searchStatus === 'idle' is missing fetchRecentSearches from its dependency array; update the useEffect to include fetchRecentSearches (i.e., useEffect(..., [searchStatus, fetchRecentSearches])) and ensure the fetchRecentSearches function is memoized with useCallback (or remains so) to provide a stable reference; also remove any eslint-disable comment masking this dependency if present so the linter can validate the dependency list.
139-141:reloadKey에searchStatus가 포함되어 있어 불필요한 재요청이 발생할 수 있습니다.
searchStatus가'searching'→'searched'로 변경될 때queryTerm도 함께 변경되면,reloadKey의searchStatus와queryTerm두 값이 모두 바뀌면서 effect가 두 번 트리거될 수 있습니다. 또한searchStatus가isFinalized파라미터에도 영향을 주므로,reloadKey에서searchStatus를 제거하면 실제 검색 파라미터 변화에만 반응하도록 할 수 있습니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/groupSearch/GroupSearch.tsx` around lines 139 - 141, The reloadKey currently includes searchStatus which causes redundant reloads when status flips (e.g., 'searching'→'searched'); update the invocation of useInifinieScroll by removing searchStatus from the reloadKey (keep reloadKey=`${queryTerm}-${selectedFilter}-${category}`) so the effect only reacts to actual search parameters, but leave the enabled logic (enabled: searchStatus !== 'idle' && (searchStatus === 'searched' || !!queryTerm)) and any isFinalized handling unchanged.src/pages/mypage/SavePage.tsx (1)
118-191: 렌더링 로직의 중첩 삼항 연산자가 깊습니다.
showInitialLoading ? (...) : activeTab === '피드' ? (...) : savedBooks.items.length > 0 ? (...) : (...)처럼 4단계 깊이의 삼항 연산자 체인은 가독성을 떨어뜨립니다. 각 조건별 렌더링을 별도 컴포넌트나 함수로 추출하면 유지보수성이 개선될 수 있습니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/mypage/SavePage.tsx` around lines 118 - 191, The JSX uses a deeply nested ternary (showInitialLoading ? ... : activeTab === '피드' ? ... : savedBooks.items.length > 0 ? ... : ...) which hurts readability; refactor by extracting each branch into small components or functions (e.g., LoadingSkeletons using FeedPostSkeleton and BookSkeletonItem, SavedFeedList rendering savedFeeds.items with FeedPost and handleFeedSaveToggle, and SavedBookList rendering savedBooks.items with Cover, BookInfo and handleSaveToggle) and replace the ternary chain in SavePage's render with a clear conditional (if/switch or direct component returns) that returns LoadingSkeletons, SavedFeedList, SavedBookList, or EmptyState accordingly; ensure you keep sentinelRef, isLoadingMore, keys (feed.feedId, book.bookId) and props like isLast/isMyFeed when moving logic.src/pages/memory/Memory.tsx (2)
207-221: 서버 필터링과 클라이언트 필터링이 중복됩니다.
fetchPage에서 이미isOverview,pageStart,pageEnd,isPageFilter파라미터를 API에 전달하여 서버 측에서 필터링하고 있습니다(Lines 114-120). 그런데filteredRecords(Lines 207-221)에서 동일한 조건으로 다시 클라이언트 필터링을 수행하고 있습니다. 서버 응답이 이미 필터링된 데이터를 반환한다면 이 메모이제이션은 불필요한 연산입니다.서버 필터링만으로 충분하다면
filteredRecords를 제거하고recordsList.items를 직접 사용하는 것을 고려해주세요.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/memory/Memory.tsx` around lines 207 - 221, The client-side filtering in the useMemo named filteredRecords duplicates server-side filtering done in fetchPage (parameters isOverview, pageStart, pageEnd, isPageFilter); remove filteredRecords and replace its usages with recordsList.items (or change the memo to simply return recordsList.items) so we rely on server-filtered results; ensure any references to activeFilter/selectedPageRange that expected client-side filtering are updated to trigger fetchPage instead and that UI logic still behaves correctly.
143-151: Axios 에러 타입 체크가 취약합니다.덕 타이핑으로
'response' in error를 확인하는 대신axios의isAxiosError유틸리티나instanceof AxiosError를 사용하면 타입 안전성이 향상됩니다. 관련 코드 스니펫(getRoomPlaying.ts)에서도instanceof AxiosError를 사용하고 있습니다.♻️ 수정 제안
+ import { AxiosError } from 'axios'; // ... } catch (error) { - if (error && typeof error === 'object' && 'response' in error) { - const axiosError = error as { response?: { data?: { code?: number } } }; - if (axiosError.response?.data?.code === 40002) { + if (error instanceof AxiosError && error.response?.data?.code === 40002) { setActiveFilter(null); throw new Error('독서 진행률이 80% 이상이어야 총평을 볼 수 있습니다.'); - } } throw new Error('기록을 불러오는 중 오류가 발생했습니다.'); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/memory/Memory.tsx` around lines 143 - 151, Replace the duck-typing error check in the catch block of Memory.tsx with axios' type-safe check: import and use axios.isAxiosError (or AxiosError instanceof) to determine if the thrown error is an AxiosError, then inspect axiosError.response?.data?.code === 40002 and call setActiveFilter(null) and rethrow the specific 80% message; otherwise rethrow the generic "기록을 불러오는 중 오류가 발생했습니다." error—update the catch branch that currently uses "'response' in error" and the axiosError type assertion to use the axios utility or AxiosError type for proper type safety.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src/pages/feed/Feed.tsx`:
- Around line 43-74: The fetchPage implementations in totalFeed and myFeed must
enforce a 500ms minimum loading time for the initial page load to prevent
skeleton flicker: inside each fetchPage (in the useInifinieScroll calls for
totalFeed and myFeed) record start time before awaiting waitForToken(), run the
network request (getTotalFeeds/getMyFeeds) and in parallel await a delay so that
when cursor is falsy (initial load) the total elapsed time is at least 500ms,
then return the response as before; implement the delay by computing remaining =
500 - (Date.now() - start) and awaiting a sleep/timeout only if remaining > 0,
leaving non-initial pages unchanged.
In `@src/pages/feed/FeedDetailPage.tsx`:
- Around line 137-145: The handleBackClick function currently calls
window.close() unconditionally and again inside the if block, making the first
call redundant; update handleBackClick so it only checks window.opener and calls
window.close() when window.opener is truthy, otherwise call navigate(-1). Locate
the function by name handleBackClick and adjust the conditional logic
(referencing window.opener and navigate) to remove the duplicate unconditional
window.close() call.
In `@src/pages/mypage/SavePage.tsx`:
- Around line 38-66: The savedFeeds and savedBooks useInifinieScroll calls are
missing the minLoadingTime pattern which causes skeleton flicker on fast
networks; update both useInifinieScroll invocations (the ones that define
fetchPage for getSavedFeedsInMy and getSavedBooksInMy) to include a
minLoadingTime option of 500 (apply it so it affects initial load — i.e., when
cursor is null/undefined) following the same pattern used in
Feed.tsx/Followers/SearchBook/Memory to ensure the first-page loading shows at
least 500ms.
---
Nitpick comments:
In `@src/pages/groupSearch/GroupSearch.tsx`:
- Around line 44-48: The effect that calls fetchRecentSearches when searchStatus
=== 'idle' is missing fetchRecentSearches from its dependency array; update the
useEffect to include fetchRecentSearches (i.e., useEffect(..., [searchStatus,
fetchRecentSearches])) and ensure the fetchRecentSearches function is memoized
with useCallback (or remains so) to provide a stable reference; also remove any
eslint-disable comment masking this dependency if present so the linter can
validate the dependency list.
- Around line 139-141: The reloadKey currently includes searchStatus which
causes redundant reloads when status flips (e.g., 'searching'→'searched');
update the invocation of useInifinieScroll by removing searchStatus from the
reloadKey (keep reloadKey=`${queryTerm}-${selectedFilter}-${category}`) so the
effect only reacts to actual search parameters, but leave the enabled logic
(enabled: searchStatus !== 'idle' && (searchStatus === 'searched' ||
!!queryTerm)) and any isFinalized handling unchanged.
In `@src/pages/memory/Memory.tsx`:
- Around line 207-221: The client-side filtering in the useMemo named
filteredRecords duplicates server-side filtering done in fetchPage (parameters
isOverview, pageStart, pageEnd, isPageFilter); remove filteredRecords and
replace its usages with recordsList.items (or change the memo to simply return
recordsList.items) so we rely on server-filtered results; ensure any references
to activeFilter/selectedPageRange that expected client-side filtering are
updated to trigger fetchPage instead and that UI logic still behaves correctly.
- Around line 143-151: Replace the duck-typing error check in the catch block of
Memory.tsx with axios' type-safe check: import and use axios.isAxiosError (or
AxiosError instanceof) to determine if the thrown error is an AxiosError, then
inspect axiosError.response?.data?.code === 40002 and call setActiveFilter(null)
and rethrow the specific 80% message; otherwise rethrow the generic "기록을 불러오는 중
오류가 발생했습니다." error—update the catch branch that currently uses "'response' in
error" and the axiosError type assertion to use the axios utility or AxiosError
type for proper type safety.
In `@src/pages/mypage/SavePage.tsx`:
- Around line 118-191: The JSX uses a deeply nested ternary (showInitialLoading
? ... : activeTab === '피드' ? ... : savedBooks.items.length > 0 ? ... : ...)
which hurts readability; refactor by extracting each branch into small
components or functions (e.g., LoadingSkeletons using FeedPostSkeleton and
BookSkeletonItem, SavedFeedList rendering savedFeeds.items with FeedPost and
handleFeedSaveToggle, and SavedBookList rendering savedBooks.items with Cover,
BookInfo and handleSaveToggle) and replace the ternary chain in SavePage's
render with a clear conditional (if/switch or direct component returns) that
returns LoadingSkeletons, SavedFeedList, SavedBookList, or EmptyState
accordingly; ensure you keep sentinelRef, isLoadingMore, keys (feed.feedId,
book.bookId) and props like isLast/isMyFeed when moving logic.
ℹ️ Review info
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (7)
src/pages/feed/Feed.tsxsrc/pages/feed/FeedDetailPage.tsxsrc/pages/feed/FollowerListPage.tsxsrc/pages/groupSearch/GroupSearch.tsxsrc/pages/memory/Memory.tsxsrc/pages/mypage/SavePage.tsxsrc/pages/searchBook/SearchBook.tsx
ljh130334
left a comment
There was a problem hiding this comment.
충돌 많았을텐데 깔끔하게 작업해주셔서 감사합니다!! 넘 수고하셨습니다 👍🏻 🥇
ho0010
left a comment
There was a problem hiding this comment.
고생 많으셨습니다!
확실히 로직을 분리하니 가독성 측면에서 크게 개선된 것 같네요~
📝작업 내용
Memory, SearchBook, SavePage, GroupSearch, FollowerListPage, Feed, FeedDetailPage에서 훅 기반 무한스크롤, 기존 수동 페이징, 스켈레톤 로직이 혼재되어 있어 아래 원칙에 따라 코드를 정리했습니다.
수정 파일
Summary by CodeRabbit
릴리스 노트
성능 개선
버그 수정