Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! 이 PR은 피드 페이지의 CD 재생 기능을 위한 API 연결을 구현하고, 관련 컴포넌트 구조를 리팩토링하여 코드의 재사용성과 유지보수성을 높이는 데 중점을 둡니다. 특히, isOwner 판단 로직을 상위 컴포넌트에서 관리하도록 변경하고, CD 캐러셀의 데이터 로딩 및 뷰 로직을 분리하여 더욱 견고하고 확장 가능한 아키텍처를 구축했습니다. 이를 통해 피드 페이지에서 CD 플레이리스트를 효율적으로 탐색하고 재생할 수 있게 됩니다. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
📝 WalkthroughWalkthrough피드 CD 재생용 서버측 캐러셀 엔드포인트와 타입을 추가하고, 클라이언트에서 캐러셀 페이지네이션 훅과 FeedCarousel/CarouselItem으로 UI를 재구성하여 라우터 컨텍스트 기반 상태 전달로 전환했습니다. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant FeedLayout as FeedLayout\n(outlet context)
participant FeedCarousel as FeedCarousel\n(useCarouselCdList)
participant API as Playlist API
participant Detail as PlaylistDetail\n(usePlaylistDetail)
participant CarouselItem as CarouselItem
User->>FeedLayout: 피드 진입 (shareCode)
FeedLayout->>FeedCarousel: isOwner 컨텍스트 전달
User->>FeedCarousel: 캐러셀 초기 로드
FeedCarousel->>API: getCdCarousel / getLikedCdCarousel (params)
API-->>FeedCarousel: paginated carousel data
FeedCarousel->>Detail: usePlaylistDetail(centerPlaylistId)
Detail-->>FeedCarousel: playlistDetail
FeedCarousel->>CarouselItem: playlistData + playlistDetail + centerItem
User->>CarouselItem: 스와이프 / 선택
CarouselItem->>FeedCarousel: handleCenterChange(newId)
FeedCarousel->>API: next/prev page 요청 (필요시)
API-->>FeedCarousel: 추가 페이지 데이터
FeedCarousel->>Detail: 새 center에 대한 상세 재요청
Detail-->>FeedCarousel: 업데이트된 playlistDetail
FeedCarousel->>CarouselItem: 업데이트 전달
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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.
Code Review
이 PR은 피드에서 CD를 재생하는 기능을 구현하고, 관련 API를 연동하는 작업을 포함합니다. 전반적으로 isOwner 상태를 FeedLayout에서 관리하여 하위 컴포넌트로 전달하는 구조 개선과, FeedCarousel 컴포넌트를 도입하여 cds와 likes 페이지의 중복 로직을 제거한 점이 인상적입니다. 이는 코드의 재사용성과 유지보수성을 크게 향상시킬 것입니다. 또한, useInfiniteQuery를 활용한 양방향 캐러셀 구현과 cdSpinner 스타일 믹스인 추출 등 좋은 리팩토링이 많이 이루어졌습니다. 다만, ActionBar 컴포넌트에서 자신의 CD에 '좋아요' 버튼이 표시될 수 있는 가능성이 있어 보이는 부분을 발견하여 리뷰 코멘트를 남겼습니다. 이 부분을 확인해주시면 좋을 것 같습니다.
|
🎵 Storybook Link 🎵 |
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/widgets/playlist/ActionBar.tsx (1)
32-37:⚠️ Potential issue | 🟠 Major
onClick핸들러를SvgButton에 직접 전달하세요.현재
onClick={handleMovePlaylist}가 wrapperdiv(DetailButton)에 있지만, 내부의<SvgButton>은 네이티브<button>을 렌더링합니다.SvgButton은onClickprop을 지원하므로, 핸들러를 정확한 버튼 요소로 옮겨야 합니다.<DetailButton $isMy={type === 'MY'}> <SvgButton icon={Playlist} onClick={handleMovePlaylist} width={type === 'MY' ? 16 : 24} height={type === 'MY' ? 16 : 24} /> </DetailButton>그리고
DetailButton에서onClick제거하세요. 현재 방식은 의미론적으로 올바르지 않은 div에 click handler가 있어 스크린리더 및 키보드 접근성 문제가 발생합니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/widgets/playlist/ActionBar.tsx` around lines 32 - 37, The click handler is attached to the wrapper DetailButton div instead of the actual button element; move the onClick from DetailButton to the inner SvgButton by removing onClick={handleMovePlaylist} from DetailButton and adding onClick={handleMovePlaylist} to the SvgButton (keep width/height/ icon logic intact) so the native button rendered by SvgButton receives the event (references: DetailButton, SvgButton, handleMovePlaylist); this also removes the non-semantic click on a div to restore proper keyboard/ARIA behavior.
🤖 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/entities/playlist/model/usePlaylists.ts`:
- Around line 156-163: The query key for useInfiniteQuery in useCarouselCdList
currently includes params.anchorId which causes the infinite query to reset on
every carousel center-card change; remove params.anchorId from the queryKey
(keep ['feedCdList', type, shareCode, params.sort]) and instead pass
params.anchorId only into the fetch function as an initial seed; when the anchor
truly needs to trigger a reload, call the queryClient.invalidateQueries or
queryClient.resetQueries / refetch manually from the component (or expose a
reset/refetch function from useCarouselCdList) so anchor changes do not recreate
the whole infinite query state.
In `@src/pages/feed/FeedLayout.tsx`:
- Around line 9-13: The current FeedLayout renders <Outlet context={{ isOwner:
data?.isOwner }} /> even when useOwnerStatus failed, causing undefined isOwner
in child routes; update the component to check useOwnerStatus's isError (and
!data) after isLoading and render an error/fallback (or null) instead of the
Outlet, and only pass data.isOwner into <Outlet context={{ isOwner: ... }}> when
the query succeeded; locate the useOwnerStatus call and the Outlet return in
FeedLayout (symbols: useOwnerStatus, isLoading, isError, data, Outlet) and
implement the early error branch so children never receive undefined isOwner.
In `@src/pages/feed/ui/CarouselItem.tsx`:
- Around line 243-247: The ActionBar is always receiving type="MY" which breaks
UI differences; update the ActionBar invocation in CarouselItem (the ActionBar
component near centerItem/playlistId/currentPlaylist usage) to pass a dynamic
type based on pageType (e.g., type={pageType === 'MY' ? 'MY' : 'DISCOVER'} or
simply type={pageType} if values align) so LikeButton, ShareButton, ChatButton
and layout behave correctly for LIKE/DISCOVER vs MY pages.
In `@src/pages/feed/ui/FeedCarousel.tsx`:
- Around line 23-24: The carousel can hang when routePlaylistId exists but
cannot be resolved (NaN, not present in the fetched list, or list is empty), so
update FeedCarousel to defensively handle that: validate routePlaylistId before
using it (ensure Number(routePlaylistId) is finite), and if the playlist cannot
be found set anchorId to undefined or explicitly set a fallback/empty state so
centerItem is initialized (or trigger the component's error/fallback path).
Specifically, adjust the logic around currentSort and anchorId initialization
and add a guard where centerItem is computed (the code that currently relies on
!centerItem.playlistId at runtime) to short-circuit into an empty-state or
isError path when the list is empty or the anchor lookup fails. Ensure this uses
existing error handling (onError/try-catch or state flag) rather than letting
the spinner run indefinitely.
In `@src/pages/feed/ui/layout/CdPlayerLayout.tsx`:
- Around line 31-39: CdPlayerLayout currently calls useOutletContext() without a
type so undefined/any flows into children; change the hook to
useOutletContext<ShareCodeOwnerResponse | null>() in CdPlayerLayout (and
validate consumers) and update the provider (FeedLayout) to never pass undefined
by returning null or a safe default for data?.isOwner (or explicitly provide a
ShareCodeOwnerResponse|null) so child components like FeedCarousel and
downstream routes receive a typed value and handle the null case instead of
undefined.
---
Outside diff comments:
In `@src/widgets/playlist/ActionBar.tsx`:
- Around line 32-37: The click handler is attached to the wrapper DetailButton
div instead of the actual button element; move the onClick from DetailButton to
the inner SvgButton by removing onClick={handleMovePlaylist} from DetailButton
and adding onClick={handleMovePlaylist} to the SvgButton (keep width/height/
icon logic intact) so the native button rendered by SvgButton receives the event
(references: DetailButton, SvgButton, handleMovePlaylist); this also removes the
non-semantic click on a div to restore proper keyboard/ARIA behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 370e73e0-de20-4fab-b409-1b49a554875a
📒 Files selected for processing (18)
src/entities/playlist/api/playlist.tssrc/entities/playlist/model/useMyCd.tssrc/entities/playlist/model/usePlaylists.tssrc/entities/playlist/types/playlist.tssrc/pages/feed/FeedLayout.tsxsrc/pages/feed/cds/index.tsxsrc/pages/feed/index.tsxsrc/pages/feed/likes/index.tsxsrc/pages/feed/ui/CarouselItem.tsxsrc/pages/feed/ui/CdCarousel.tsxsrc/pages/feed/ui/FeedCarousel.tsxsrc/pages/feed/ui/FeedCdList.tsxsrc/pages/feed/ui/index.tssrc/pages/feed/ui/layout/CdPlayerLayout.tsxsrc/pages/mycd/index.tsxsrc/shared/styles/mixins.tssrc/widgets/playlist/ActionBar.tsxsrc/widgets/playlist/PlaylistLayout.tsx
💤 Files with no reviewable changes (2)
- src/pages/mycd/index.tsx
- src/pages/feed/ui/CdCarousel.tsx
| export const useCarouselCdList = ( | ||
| type: FEED_CD_LIST_TAB_TYPE, // cds or likes | ||
| shareCode: string, | ||
| params: CarouselParams | ||
| ) => { | ||
| return useInfiniteQuery({ | ||
| queryKey: ['feedCdList', type, shareCode, params.sort, params.anchorId], | ||
|
|
There was a problem hiding this comment.
anchorId가 cache key에 들어가서 카드 이동마다 infinite query가 리셋됩니다.
FeedCarousel은 중심 카드가 바뀔 때마다 route id를 갱신합니다. 지금 키에 params.anchorId가 포함돼 있어서 스와이프 한 번마다 새 쿼리가 생성되고, 이미 받아온 pages를 버린 채 다시 로딩하게 됩니다. 이 훅의 양방향 프리패칭 이점이 거의 사라집니다.
♻️ 수정 방향 예시
return useInfiniteQuery({
- queryKey: ['feedCdList', type, shareCode, params.sort, params.anchorId],
+ queryKey: ['feedCdList', type, shareCode, params.sort, params.limit],anchorId는 초기 진입 seed로만 사용하고, 실제 재-anchor가 필요할 때만 별도로 reset/refetch 하는 편이 안전합니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/entities/playlist/model/usePlaylists.ts` around lines 156 - 163, The
query key for useInfiniteQuery in useCarouselCdList currently includes
params.anchorId which causes the infinite query to reset on every carousel
center-card change; remove params.anchorId from the queryKey (keep
['feedCdList', type, shareCode, params.sort]) and instead pass params.anchorId
only into the fetch function as an initial seed; when the anchor truly needs to
trigger a reload, call the queryClient.invalidateQueries or
queryClient.resetQueries / refetch manually from the component (or expose a
reset/refetch function from useCarouselCdList) so anchor changes do not recreate
the whole infinite query state.
| const { data, isLoading } = useOwnerStatus(shareCode || '') | ||
|
|
||
| if (isLoading) return <Loading isLoading /> | ||
|
|
||
| return <Outlet context={{ isOwner: data?.isOwner }} /> |
There was a problem hiding this comment.
오너 상태 조회 실패를 정상 렌더로 넘기지 마세요.
여기서는 useOwnerStatus가 실패해도 그대로 Outlet을 렌더링해서 isOwner: undefined가 하위 라우트로 내려갑니다. 그러면 실제 소유자여도 편집/삭제 UI가 숨겨지고 헤더 문구도 잘못 나옵니다. 로딩이 끝난 뒤에는 isError || !data를 먼저 처리하고, 성공한 경우에만 data를 context로 넘기는 쪽이 안전합니다.
🐛 수정 예시
-import { Outlet, useParams } from 'react-router-dom'
+import { Navigate, Outlet, useParams } from 'react-router-dom'
...
- const { data, isLoading } = useOwnerStatus(shareCode || '')
+ const { data, isLoading, isError } = useOwnerStatus(shareCode)
...
- return <Outlet context={{ isOwner: data?.isOwner }} />
+ if (isError || !data) return <Navigate to="/error" replace />
+
+ return <Outlet context={data} />📝 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 { data, isLoading } = useOwnerStatus(shareCode || '') | |
| if (isLoading) return <Loading isLoading /> | |
| return <Outlet context={{ isOwner: data?.isOwner }} /> | |
| import { Navigate, Outlet, useParams } from 'react-router-dom' | |
| const { data, isLoading, isError } = useOwnerStatus(shareCode) | |
| if (isLoading) return <Loading isLoading /> | |
| if (isError || !data) return <Navigate to="/error" replace /> | |
| return <Outlet context={data} /> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/pages/feed/FeedLayout.tsx` around lines 9 - 13, The current FeedLayout
renders <Outlet context={{ isOwner: data?.isOwner }} /> even when useOwnerStatus
failed, causing undefined isOwner in child routes; update the component to check
useOwnerStatus's isError (and !data) after isLoading and render an
error/fallback (or null) instead of the Outlet, and only pass data.isOwner into
<Outlet context={{ isOwner: ... }}> when the query succeeded; locate the
useOwnerStatus call and the Outlet return in FeedLayout (symbols:
useOwnerStatus, isLoading, isError, data, Outlet) and implement the early error
branch so children never receive undefined isOwner.
| <ActionBar | ||
| playlistId={centerItem.playlistId ?? 0} | ||
| creatorId={currentPlaylist.creatorId} | ||
| stickers={playlistDetail?.cdResponse?.cdItems || []} | ||
| stickers={playlistDetail.cdResponse.cdItems} | ||
| type="MY" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== ActionBar 관련 선언 ==="
fd 'ActionBar' src -t f -x sed -n '1,240p' {}
echo
echo "=== ActionBar 사용처 ==="
rg -nP --type=ts --type=tsx '<ActionBar\b' src -C 3Repository: dnd-side-project/dnd-13th-8-frontend
Length of output: 1766
🏁 Script executed:
# CarouselItem.tsx 전체 구조와 pageType 정의 확인
cat -n src/pages/feed/ui/CarouselItem.tsx | head -280 | tail -100Repository: dnd-side-project/dnd-13th-8-frontend
Length of output: 4128
🏁 Script executed:
# ActionBar를 사용하는 모든 위치 (grep 사용, rg 대신)
grep -rn "ActionBar" src --include="*.ts" --include="*.tsx" -A 5 | head -100Repository: dnd-side-project/dnd-13th-8-frontend
Length of output: 5532
🏁 Script executed:
# CarouselItem.tsx에서 pageType이 어떻게 정의되고 사용되는지 확인
grep -n "pageType" src/pages/feed/ui/CarouselItem.tsxRepository: dnd-side-project/dnd-13th-8-frontend
Length of output: 400
🏁 Script executed:
# LikeButton, ChatButton, ShareButton의 type 핸들링 확인
find src -name "LikeButton.tsx" -o -name "ChatButton.tsx" -o -name "ShareButton.tsx" | head -10Repository: dnd-side-project/dnd-13th-8-frontend
Length of output: 185
🏁 Script executed:
# 각 컴포넌트의 type prop 사용 방식
grep -rn "type\s*=" src/features/like src/features/share src/widgets/chat --include="*.tsx" -A 2 | grep -E "(LikeButton|ChatButton|ShareButton|interface|type\s*[?:])" | head -50Repository: dnd-side-project/dnd-13th-8-frontend
Length of output: 2794
pageType이 'LIKE'일 때도 ActionBar에 'MY'를 고정으로 전달하고 있습니다.
자식 컴포넌트들이 type prop을 받아 다르게 처리하므로:
LikeButton은type='MY'일 때 "좋아요" 라벨을 표시 (좋아요 탭에서 불필요)ShareButton과ChatButton은type='MY'일 때 아이콘이 16×16으로 축소 (DISCOVER 시 24×24)ActionBar레이아웃도type='MY'일 때flexRowCenter,type='DISCOVER'일 때flexColCenter로 다름
pageType에 따라 ActionBar의 타입을 동적으로 전달해야 합니다 (예: type={pageType === 'MY' ? 'MY' : 'DISCOVER'}).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/pages/feed/ui/CarouselItem.tsx` around lines 243 - 247, The ActionBar is
always receiving type="MY" which breaks UI differences; update the ActionBar
invocation in CarouselItem (the ActionBar component near
centerItem/playlistId/currentPlaylist usage) to pass a dynamic type based on
pageType (e.g., type={pageType === 'MY' ? 'MY' : 'DISCOVER'} or simply
type={pageType} if values align) so LikeButton, ShareButton, ChatButton and
layout behave correctly for LIKE/DISCOVER vs MY pages.
| const [currentSort] = useState(() => state?.currentSort ?? 'RECENT') | ||
| const anchorId = routePlaylistId ? Number(routePlaylistId) : undefined |
There was a problem hiding this comment.
anchor를 못 찾는 케이스를 처리하지 않으면 여기서 spinner가 끝나지 않습니다.
routePlaylistId가 있지만 응답에 해당 CD가 없거나, Number(routePlaylistId)가 NaN이거나, 목록이 빈 배열이면 centerItem이 끝까지 설정되지 않습니다. 그러면 Line 109의 !centerItem.playlistId가 계속 참이어서 isError/fallback 분기로 절대 내려가지 못합니다. 삭제·언라이크 직후나 잘못된 deep link에서 화면이 무한 로딩 상태로 남습니다.
🐛 최소 수정 예시
- if (isLoading || isDetailLoading || !centerItem.playlistId) {
+ if (isLoading || isDetailLoading) {
return <Loading isLoading />
}
- if (!data || !playlistDetail || isError) {
+ if (isError || !playlistData.length || !centerItem.playlistId || !playlistDetail) {
return <Navigate to="/error" replace />
}빈 목록을 에러로 보내고 싶지 않다면, 같은 위치에 별도 empty state를 두세요.
Also applies to: 51-75, 109-115
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/pages/feed/ui/FeedCarousel.tsx` around lines 23 - 24, The carousel can
hang when routePlaylistId exists but cannot be resolved (NaN, not present in the
fetched list, or list is empty), so update FeedCarousel to defensively handle
that: validate routePlaylistId before using it (ensure Number(routePlaylistId)
is finite), and if the playlist cannot be found set anchorId to undefined or
explicitly set a fallback/empty state so centerItem is initialized (or trigger
the component's error/fallback path). Specifically, adjust the logic around
currentSort and anchorId initialization and add a guard where centerItem is
computed (the code that currently relies on !centerItem.playlistId at runtime)
to short-circuit into an empty-state or isError path when the list is empty or
the anchor lookup fails. Ensure this uses existing error handling
(onError/try-catch or state flag) rather than letting the spinner run
indefinitely.
| const isOwner = useOutletContext() | ||
|
|
||
| const videoId = currentPlaylist | ||
| ? getVideoId(currentPlaylist.songs[currentTrackIndex]?.youtubeUrl) | ||
| : null | ||
|
|
||
| return ( | ||
| <> | ||
| <Outlet /> | ||
| <Outlet context={isOwner} /> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -e
for file in \
src/pages/feed/FeedLayout.tsx \
src/pages/feed/ui/layout/CdPlayerLayout.tsx \
src/pages/feed/index.tsx
do
echo "### $file"
rg -n -C2 'Outlet context|useOutletContext|isOwner' "$file"
doneRepository: dnd-side-project/dnd-13th-8-frontend
Length of output: 1079
🏁 Script executed:
# ShareCodeOwnerResponse 타입 정의 확인
rg -n 'type ShareCodeOwnerResponse|interface ShareCodeOwnerResponse' -A3Repository: dnd-side-project/dnd-13th-8-frontend
Length of output: 588
🏁 Script executed:
# FeedLayout.tsx 전체 내용으로 data 상태와 로딩 처리 확인
cat -n src/pages/feed/FeedLayout.tsxRepository: dnd-side-project/dnd-13th-8-frontend
Length of output: 598
🏁 Script executed:
# CdPlayerLayout.tsx의 하위 라우트 구조 확인
rg -n '<Outlet|useOutletContext' -B3 -A3 src/pages/feed/Repository: dnd-side-project/dnd-13th-8-frontend
Length of output: 4028
CdPlayerLayout에서 useOutletContext에 제네릭 타입을 지정하고, provider의 undefined 상태를 처리하세요.
useOutletContext()는 타입이 없어서 any와 동일하며, 상위 FeedLayout에서 data?.isOwner(undefined 가능)를 그대로 넘기고 있습니다. 현재 코드는 하위 라우트(src/pages/feed/index.tsx, src/pages/feed/ui/FeedCarousel.tsx)가 기대하는 ShareCodeOwnerResponse 타입을 검증하지 못하므로, owner-status 조회 실패 시 undefined가 모든 하위 라우트로 퍼져 타입 에러가 발생할 수 있습니다.
수정 방안:
const isOwner = useOutletContext<ShareCodeOwnerResponse>()로 명시적 타입 지정- 또는
FeedLayout에서data가 없으면null을 반환하거나 기본값을 제공하여 중간 계층부터 undefined 상태 차단
스타일 가이드 "타입스크립트: strict 모드 사용, 타입 명시적으로 작성 (any 사용 지양)" 위반입니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/pages/feed/ui/layout/CdPlayerLayout.tsx` around lines 31 - 39,
CdPlayerLayout currently calls useOutletContext() without a type so
undefined/any flows into children; change the hook to
useOutletContext<ShareCodeOwnerResponse | null>() in CdPlayerLayout (and
validate consumers) and update the provider (FeedLayout) to never pass undefined
by returning null or a safe default for data?.isOwner (or explicitly provide a
ShareCodeOwnerResponse|null) so child components like FeedCarousel and
downstream routes receive a typed value and handle the null case instead of
undefined.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/pages/feed/index.tsx (1)
29-29: outlet context가 없을 경우 방어 코드 고려
useOutletContext는 outlet context가 없을 경우undefined를 반환할 수 있습니다.FeedLayout에서 항상 context를 제공한다고 가정하지만, 라우트 구조 변경 시 런타임 에러가 발생할 수 있습니다.🛡️ 방어 코드 제안
- const { isOwner: isMyFeed } = useOutletContext<ShareCodeOwnerResponse>() + const outletContext = useOutletContext<ShareCodeOwnerResponse>() + const isMyFeed = outletContext?.isOwner ?? false🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/feed/index.tsx` at line 29, useOutletContext<ShareCodeOwnerResponse>() can return undefined if no outlet context is provided; add a defensive guard around the call in the feed page to avoid runtime errors (e.g., check the returned value before destructuring or provide a safe default). Locate the usage of useOutletContext and the isMyFeed variable in src/pages/feed/index.tsx and update it so that it safely handles undefined (for example, call useOutletContext into a variable like outletContext and then derive isMyFeed = outletContext?.isOwner ?? false), and ensure this remains consistent with FeedLayout which normally provides the context.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/pages/feed/index.tsx`:
- Line 29: useOutletContext<ShareCodeOwnerResponse>() can return undefined if no
outlet context is provided; add a defensive guard around the call in the feed
page to avoid runtime errors (e.g., check the returned value before
destructuring or provide a safe default). Locate the usage of useOutletContext
and the isMyFeed variable in src/pages/feed/index.tsx and update it so that it
safely handles undefined (for example, call useOutletContext into a variable
like outletContext and then derive isMyFeed = outletContext?.isOwner ?? false),
and ensure this remains consistent with FeedLayout which normally provides the
context.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 2cd72372-f02e-4716-8bd1-bd4e0226a9da
📒 Files selected for processing (4)
src/entities/playlist/types/playlist.tssrc/pages/feed/index.tsxsrc/pages/feed/ui/FeedCdList.tsxsrc/pages/feed/ui/index.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- src/pages/feed/ui/FeedCdList.tsx
- src/entities/playlist/types/playlist.ts
hansololiviakim
left a comment
There was a problem hiding this comment.
isOwner 판단 로직을 FeedLayout에서 담당하도록 변경
feed 하위 여러 라우트에서 동일하게 사용하는 isOwner 판단 로직을 중복 없이 관리하기 위해 공통 부모인 FeedLayout에서 계산하고 Outlet context로 전달하도록 구조를 변경했습니다. 일부 라우트에서 사용하지 않더라도 공통 로직을 상위 레벨에 두는 것이 유지보수성과 상태 일관성 측면에서 더 적합하다고 판단했습니다.
너무 좋습니다~! 이제 2차 스프린트도 끝이 보이네요 ㅎ.ㅎ 고생하셨습니다!
🛰️ 관련 이슈
✨ 주요 변경 사항
피드 CD 재생 API 연결
🗯️ PR 포인트
isOwner판단 로직을FeedLayout에서 담당하도록 변경feed 하위 여러 라우트에서 동일하게 사용하는 isOwner 판단 로직을 중복 없이 관리하기 위해 공통 부모인 FeedLayout에서 계산하고 Outlet context로 전달하도록 구조를 변경했습니다. 일부 라우트에서 사용하지 않더라도 공통 로직을 상위 레벨에 두는 것이 유지보수성과 상태 일관성 측면에서 더 적합하다고 판단했습니다.
테스트 시 좋아요 해제 및 내 플리 삭제하는 과정에서 데이터 갱신과 관련된 이슈가 있습니다. 서버 이슈로 판단되어서 현재 수정 요청 드린 상태입니다 ! (디코 참고) 수정 후에도 동일 이슈 계속해서 발생하면 QA 진행하면서 추가로 확인해보겠습니다
🚀 알게된 점
📖 참고 자료 (선택)
Summary by CodeRabbit
릴리스 노트
새로운 기능
개선 사항