fix: 피드 이미지 로딩 최적화 및 FollowerList에 의한 CLS 완화#315
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Walkthrough이미지 미리보기 기능 추가, FollowList 로딩 상태 및 스켈레톤 UI 통합, 여러 페이지에서 Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
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.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/components/feed/FollowList.styled.ts (1)
62-67:⚠️ Potential issue | 🟡 Minor중첩 스크롤바 셀렉터가 올바르지 않습니다.
.followerList내부에서.followerList::-webkit-scrollbar로 선언하면 실제로는.followerList .followerList::-webkit-scrollbar가 되어 의도한 대로 동작하지 않습니다.&::-webkit-scrollbar를 사용해야 합니다.🔧 수정 제안
/* ✅ 스크롤바 숨기기 */ scrollbar-width: none; /* Firefox */ -ms-overflow-style: none; /* IE, Edge */ - .followerList::-webkit-scrollbar { + &::-webkit-scrollbar { display: none; /* Chrome, Safari */ }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/feed/FollowList.styled.ts` around lines 62 - 67, The nested scrollbar selector is incorrect: using `.followerList { .followerList::-webkit-scrollbar { ... } }` results in `.followerList .followerList::-webkit-scrollbar`; update the rules for the `.followerList` component in FollowList.styled.ts to use the parent selector for WebKit, e.g. place the scrollbar rules inside the `.followerList` block and replace `.followerList::-webkit-scrollbar` with `&::-webkit-scrollbar`, keeping the existing `scrollbar-width: none` and `-ms-overflow-style: none` lines.src/pages/feed/FeedDetailPage.tsx (2)
46-55:⚠️ Potential issue | 🟡 Minor에러 경로에서 최소 500ms 로딩 시간이 보장되지 않습니다.
Line 47의
Promise.all(...)에서getFeedDetail(...)이 실패하면 즉시 reject되어 Line 55에서 로딩이 바로 내려갑니다. 실패 케이스에서도 스켈레톤 표시 타이밍을 일관되게 유지하려면 최소 지연을finally블록에서 기다려야 합니다.수정 예시
+ const minLoadingTime = new Promise<void>(resolve => setTimeout(resolve, 500)); try { setLoading(true); - const minLoadingTime = new Promise(resolve => setTimeout(resolve, 500)); - const [feedResponse] = await Promise.all([getFeedDetail(Number(feedId)), minLoadingTime]); + const feedResponse = await getFeedDetail(Number(feedId)); setFeedData(feedResponse.data); setError(null); } catch (err) { console.error('피드 상세 정보 로드 실패:', err); setError('피드 정보를 불러오는데 실패했습니다.'); } finally { + await minLoadingTime; setLoading(false); }🤖 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 46 - 55, The current Promise.all([...minLoadingTime]) rejects immediately on getFeedDetail failure so the 500ms skeleton guarantee is lost; change the flow to await getFeedDetail(Number(feedId)) (assigning its result) and move awaiting the minLoadingTime Promise into the finally block (e.g. create minLoadingTime before the try, and in finally await minLoadingTime before calling setLoading(false)) so that both success and error paths wait the minimum 500ms; keep using setFeedData, setError, and setLoading as before and ensure you only call setFeedData when the fetch succeeded (i.e., when the response exists).
36-60:⚠️ Potential issue | 🟠 MajorfeedId 변경 시 이전 요청이 상태를 덮어쓸 수 있습니다.
라인 36-60의 비동기 로딩은
feedId가 변경되거나 컴포넌트가 언마운트된 후에도setFeedData/setError/setLoading을 실행할 수 있습니다. 예를 들어, 피드 A를 로드하는 중 라우팅으로 피드 B로 이동하면 B의 응답이 먼저 도착한 후 A의 응답이 상태를 덮어쓸 수 있어 잘못된 피드가 화면에 표시됩니다.cleanup 함수에서
isActive플래그를 사용하거나AbortController를 도입하여 stale update를 방지하세요.수정 예시
useEffect(() => { + let isActive = true; + const loadFeedDetail = async () => { if (!feedId) { + if (!isActive) return; setError('피드 ID가 없습니다.'); setLoading(false); return; } try { setLoading(true); const minLoadingTime = new Promise(resolve => setTimeout(resolve, 500)); const [feedResponse] = await Promise.all([getFeedDetail(Number(feedId)), minLoadingTime]); + if (!isActive) return; setFeedData(feedResponse.data); setError(null); } catch (err) { + if (!isActive) return; console.error('피드 상세 정보 로드 실패:', err); setError('피드 정보를 불러오는데 실패했습니다.'); } finally { - setLoading(false); + if (isActive) setLoading(false); } }; loadFeedDetail(); + return () => { + isActive = false; + }; }, [feedId]);🤖 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 36 - 60, The loadFeedDetail async in the useEffect can update state after feedId changes or unmounts; modify it so stale responses are ignored by tracking activity: inside useEffect create an AbortController (or an isActive boolean) and pass the abort signal to getFeedDetail (or check isActive before calling setFeedData/setError/setLoading), add a cleanup that aborts the controller (or sets isActive = false) to prevent setFeedData/setError/setLoading from running for stale requests in loadFeedDetail.
🧹 Nitpick comments (5)
src/shared/ui/Skeleton/feed/OtherFeedSkeleton.styled.ts (1)
3-4: 주석 처리된 코드 및 사용되지 않는 prop 제거 권장
paddingTopprop이 타입에는 정의되어 있으나 실제로 사용되지 않습니다. 주석 처리된 코드를 제거하고 타입 정의도 함께 정리하는 것이 좋습니다.♻️ 정리 제안
-export const SkeletonContainer = styled.div<{ paddingTop?: number }>` - /* padding-top: ${({ paddingTop }) => (paddingTop !== undefined ? `${paddingTop}px` : '56px')}; */ +export const SkeletonContainer = styled.div` background-color: var(--color-black-main);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/shared/ui/Skeleton/feed/OtherFeedSkeleton.styled.ts` around lines 3 - 4, The SkeletonContainer styled component currently declares an unused paddingTop prop and contains commented-out styling; remove the commented line and drop the unused generic prop from the styled.div type (i.e., remove "{ paddingTop?: number }" from SkeletonContainer) and adjust any usages if they pass paddingTop to this component so there are no leftover props being forwarded.src/components/feed/TotalFeed.styled.ts (1)
4-9: 주석 처리된 코드 제거 권장
padding-top: 136px가 주석 처리되어 있습니다. Feed.styled.ts의 Container에서 이 패딩을 담당하도록 변경되었으므로, 주석 처리된 코드를 완전히 제거하는 것이 좋습니다.♻️ 정리 제안
export const Container = styled.div` min-height: 100vh; - /* padding-top: 136px; */ padding-bottom: 125px; //이전 76px background-color: var(--color-black-main); `;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/feed/TotalFeed.styled.ts` around lines 4 - 9, Remove the commented-out padding line inside the Container styled component: delete the commented `/* padding-top: 136px; */` so Container only contains active styles (min-height, padding-bottom, background-color) and ensure no leftover comment markers remain in the Container definition.src/components/feed/FollowList.tsx (2)
42-44:onLoadingChange호출 시 무한 루프 가능성을 확인하세요.
useEffect의 의존성 배열에onLoadingChange가 포함되어 있습니다. 부모 컴포넌트(Feed.tsx)에서setIsFollowListLoading을 직접 전달하므로 이 경우는 안전하지만, 만약 부모가 인라인 함수를 전달하면 매 렌더마다 effect가 재실행될 수 있습니다.현재 구현은 안전하지만, 방어적으로
useRef를 사용하거나 콜백 안정성에 대한 주석을 추가하는 것을 고려해 보세요.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/feed/FollowList.tsx` around lines 42 - 44, The effect in FollowList (useEffect) calls onLoadingChange and includes onLoadingChange in its dependency list, which can re-run every render if the parent (Feed.tsx) passes an unstable inline function like setIsFollowListLoading; to fix, stabilize the callback before invoking it by storing the latest onLoadingChange in a ref inside FollowList (useRef) and have the effect depend only on loading, then call ref.current?.(loading), or alternatively document that Feed.tsx must pass a stable callback (e.g., wrap setIsFollowListLoading in useCallback) so the useEffect in FollowList doesn't repeatedly retrigger.
8-8: 사용되지 않는 import를 제거하세요.
FollowListLoading이 import되었지만 컴포넌트에서 사용되지 않습니다. 스켈레톤 UI는FollowContainer를 직접 사용하여 렌더링되고 있습니다.♻️ 수정 제안
-import { Container, FollowContainer, EmptyFollowerContainer } from './FollowList.styled'; +import { Container, FollowContainer, EmptyFollowerContainer } from './FollowList.styled';
FollowListLoading을 사용하려면:-import { Container, FollowContainer, EmptyFollowerContainer } from './FollowList.styled'; +import { Container, FollowContainer, FollowListLoading, EmptyFollowerContainer } from './FollowList.styled';그리고 Lines 78-93의 스켈레톤 렌더링을
FollowListLoading으로 교체하거나, styled 파일에서FollowListLoadingexport를 제거하세요.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/feed/FollowList.tsx` at line 8, The import list includes FollowListLoading but the component is never used; either remove FollowListLoading from the import statement or replace the inline skeleton rendering (the current usage of FollowContainer for the skeleton) with the FollowListLoading component. Locate the import that mentions FollowListLoading and the render block that currently renders the skeleton using FollowContainer (the block around the existing skeleton UI) and either delete FollowListLoading from the import or swap that skeleton JSX to render <FollowListLoading /> so the import is used.src/pages/feed/Feed.tsx (1)
82-86:isFollowListLoading리셋 타이밍에 주의가 필요합니다.
activeTab이 변경될 때isFollowListLoading을true로 설정하지만,FollowList의onLoadingChange콜백과 경쟁 조건이 발생할 수 있습니다.FollowList가 이미 캐시된 데이터가 있거나 빠르게 로드되면false를 먼저 호출한 후 이 effect가true로 다시 설정할 수 있습니다.
FollowList내부에서 탭 전환 시 자체적으로 로딩 상태를 관리하거나, 이 effect에서 조건부로 처리하는 것을 고려해 보세요.🤖 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 82 - 86, The effect that sets setIsFollowListLoading(true) on activeTab === '피드' can race with FollowList's onLoadingChange; instead track the previous tab and only set loading true when switching into the feed (i.e., prevActiveTab !== '피드') or delegate loading entirely to the FollowList via its onLoadingChange. Concretely, update the useEffect (the one using activeTab and setIsFollowListLoading) to use a prevActiveTab ref and only call setIsFollowListLoading(true) when prevActiveTab !== '피드' (or remove this effect and rely on FollowList.onLoadingChange to control isFollowListLoading).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/feed/FollowList.tsx`:
- Line 84: FollowList.tsx uses a CSS class "skeletonUsername" that isn't defined
in FollowList.styled.ts (which only defines .skeletonItem); either remove the
unused class from the JSX or add a matching style rule in FollowList.styled.ts
(e.g., define .skeletonUsername with the intended sizing/animation and ensure
it's exported/available to the component), or change the JSX to use the existing
.skeletonItem class if that was the intended target (update the div className
accordingly in FollowList.tsx).
---
Outside diff comments:
In `@src/components/feed/FollowList.styled.ts`:
- Around line 62-67: The nested scrollbar selector is incorrect: using
`.followerList { .followerList::-webkit-scrollbar { ... } }` results in
`.followerList .followerList::-webkit-scrollbar`; update the rules for the
`.followerList` component in FollowList.styled.ts to use the parent selector for
WebKit, e.g. place the scrollbar rules inside the `.followerList` block and
replace `.followerList::-webkit-scrollbar` with `&::-webkit-scrollbar`, keeping
the existing `scrollbar-width: none` and `-ms-overflow-style: none` lines.
In `@src/pages/feed/FeedDetailPage.tsx`:
- Around line 46-55: The current Promise.all([...minLoadingTime]) rejects
immediately on getFeedDetail failure so the 500ms skeleton guarantee is lost;
change the flow to await getFeedDetail(Number(feedId)) (assigning its result)
and move awaiting the minLoadingTime Promise into the finally block (e.g. create
minLoadingTime before the try, and in finally await minLoadingTime before
calling setLoading(false)) so that both success and error paths wait the minimum
500ms; keep using setFeedData, setError, and setLoading as before and ensure you
only call setFeedData when the fetch succeeded (i.e., when the response exists).
- Around line 36-60: The loadFeedDetail async in the useEffect can update state
after feedId changes or unmounts; modify it so stale responses are ignored by
tracking activity: inside useEffect create an AbortController (or an isActive
boolean) and pass the abort signal to getFeedDetail (or check isActive before
calling setFeedData/setError/setLoading), add a cleanup that aborts the
controller (or sets isActive = false) to prevent setFeedData/setError/setLoading
from running for stale requests in loadFeedDetail.
---
Nitpick comments:
In `@src/components/feed/FollowList.tsx`:
- Around line 42-44: The effect in FollowList (useEffect) calls onLoadingChange
and includes onLoadingChange in its dependency list, which can re-run every
render if the parent (Feed.tsx) passes an unstable inline function like
setIsFollowListLoading; to fix, stabilize the callback before invoking it by
storing the latest onLoadingChange in a ref inside FollowList (useRef) and have
the effect depend only on loading, then call ref.current?.(loading), or
alternatively document that Feed.tsx must pass a stable callback (e.g., wrap
setIsFollowListLoading in useCallback) so the useEffect in FollowList doesn't
repeatedly retrigger.
- Line 8: The import list includes FollowListLoading but the component is never
used; either remove FollowListLoading from the import statement or replace the
inline skeleton rendering (the current usage of FollowContainer for the
skeleton) with the FollowListLoading component. Locate the import that mentions
FollowListLoading and the render block that currently renders the skeleton using
FollowContainer (the block around the existing skeleton UI) and either delete
FollowListLoading from the import or swap that skeleton JSX to render
<FollowListLoading /> so the import is used.
In `@src/components/feed/TotalFeed.styled.ts`:
- Around line 4-9: Remove the commented-out padding line inside the Container
styled component: delete the commented `/* padding-top: 136px; */` so Container
only contains active styles (min-height, padding-bottom, background-color) and
ensure no leftover comment markers remain in the Container definition.
In `@src/pages/feed/Feed.tsx`:
- Around line 82-86: The effect that sets setIsFollowListLoading(true) on
activeTab === '피드' can race with FollowList's onLoadingChange; instead track the
previous tab and only set loading true when switching into the feed (i.e.,
prevActiveTab !== '피드') or delegate loading entirely to the FollowList via its
onLoadingChange. Concretely, update the useEffect (the one using activeTab and
setIsFollowListLoading) to use a prevActiveTab ref and only call
setIsFollowListLoading(true) when prevActiveTab !== '피드' (or remove this effect
and rely on FollowList.onLoadingChange to control isFollowListLoading).
In `@src/shared/ui/Skeleton/feed/OtherFeedSkeleton.styled.ts`:
- Around line 3-4: The SkeletonContainer styled component currently declares an
unused paddingTop prop and contains commented-out styling; remove the commented
line and drop the unused generic prop from the styled.div type (i.e., remove "{
paddingTop?: number }" from SkeletonContainer) and adjust any usages if they
pass paddingTop to this component so there are no leftover props being
forwarded.
ℹ️ 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 (15)
src/components/common/Post/PostBody.tsxsrc/components/feed/FollowList.styled.tssrc/components/feed/FollowList.tsxsrc/components/feed/MyFeed.styled.tssrc/components/feed/TotalFeed.styled.tssrc/components/feed/TotalFeed.tsxsrc/pages/feed/Feed.styled.tssrc/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.tsxsrc/shared/ui/Skeleton/feed/OtherFeedSkeleton.styled.ts
💤 Files with no reviewable changes (2)
- src/components/feed/TotalFeed.tsx
- src/components/feed/MyFeed.styled.ts
| {Array.from({ length: 6 }).map((_, i) => ( | ||
| <div className="followers skeletonItem" key={i}> | ||
| <Skeleton.Circle width={36} /> | ||
| <div className="username skeletonUsername"> |
There was a problem hiding this comment.
skeletonUsername 클래스가 styled 파일에 정의되어 있지 않습니다.
skeletonUsername 클래스가 사용되었지만 FollowList.styled.ts에는 .skeletonItem만 정의되어 있습니다. 현재는 스타일링에 영향이 없지만, 의도된 스타일이 있다면 추가하거나 불필요하다면 클래스를 제거하세요.
🔧 수정 제안
<div className="followers skeletonItem" key={i}>
<Skeleton.Circle width={36} />
- <div className="username skeletonUsername">
+ <div className="username">
<Skeleton.Text width={30} height={10} />
</div>
</div>📝 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 className="username skeletonUsername"> | |
| <div className="followers skeletonItem" key={i}> | |
| <Skeleton.Circle width={36} /> | |
| <div className="username"> | |
| <Skeleton.Text width={30} height={10} /> | |
| </div> | |
| </div> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/feed/FollowList.tsx` at line 84, FollowList.tsx uses a CSS
class "skeletonUsername" that isn't defined in FollowList.styled.ts (which only
defines .skeletonItem); either remove the unused class from the JSX or add a
matching style rule in FollowList.styled.ts (e.g., define .skeletonUsername with
the intended sizing/animation and ensure it's exported/available to the
component), or change the JSX to use the existing .skeletonItem class if that
was the intended target (update the div className accordingly in
FollowList.tsx).
ho0010
left a comment
There was a problem hiding this comment.
고생 많으셨습니다!
실제로 UX측면에서 유의미한 개선이었는지가 궁금하네요. 관련해서 비교 이미지나 영상도 같이 보여주시면 어떨까요??
ljh130334
left a comment
There was a problem hiding this comment.
수고하셨습니다~! 이미지 3개 제한으로 불필요한 네트워크 요청 감소시킨게 인상적이네요!! 👍🏻 👍🏻 💯
위키에 마저 정리해두도록 하겠습니다~ |
📝작업 내용
피드 탭 초기 진입 시 FollowList 렌더 타이밍과 피드 미리보기 이미지 로딩 방식으로 인해 로딩 지연 체감과 레이아웃 시프트(CLS)가 발생했습니다.
변경 사항
적용 파일
기대 효과
위키
위키에서 자세한 내용 참고부탁드립니다.
Summary by CodeRabbit
릴리스 노트
새로운 기능
성능 개선
버그 수정