Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthrough비디오 플레이어의 준비 상태 확인을 추가하고, 새로운 마퀴 효과 훅과 스타일을 구현하며, 피드 및 큐레이션 페이지의 캐러셀 컴포넌트를 재구성하고, 로그인 모달 기능을 추가하고, 라우팅 설정을 업데이트했습니다. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
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 docstrings
🧪 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 |
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은 홈, 검색, 피드, 큐레이션 등 여러 핵심 기능에 걸쳐 발견된 QA 이슈들을 해결하고 사용자 경험을 개선하는 데 중점을 두었습니다. 특히, 플레이어 관련 컴포넌트들을 통합하고 재사용 가능한 위젯으로 분리하여 코드의 유지보수성과 확장성을 향상시켰습니다. 또한, 검색 결과 페이지의 무한 스크롤 기능을 최적화하고, 비로그인 사용자를 위한 좋아요 기능의 흐름을 명확히 하는 등 전반적인 안정성과 편의성을 높였습니다. 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. 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. Footnotes
|
|
🎵 Storybook Link 🎵 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request significantly refactors and consolidates playlist and carousel functionalities across the application, moving towards a more centralized PlaylistCarousel widget. Key changes include enhanced player state handling in PlayerProvider, improved usePlaylistDetail and useCarouselCdList hooks, and the introduction of a login modal for like actions. Routing has been updated to reflect these structural changes, and infinite scrolling in SearchResultPage now uses react-intersection-observer. A new useMarquee hook and corresponding styles were added for animated text overflow in titles. Review comments highlight potential issues with NaN values in routePlaylistId conversion, a lingering console.log statement, a hardcoded z-index value that violates style guidelines, and unnecessary optional chaining in PlaylistCarousel components.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/entities/playlist/model/usePlaylists.ts (1)
67-75:⚠️ Potential issue | 🟡 Minor
playlistId를undefined로 넓힌 만큼 내부에서도 방어가 필요합니다.Line 73은 여전히 강제 캐스팅 후 호출해서, 허용된 입력 조합 중
enabled: true+undefined에서 바로 잘못된 요청이 나갑니다.🛠️ 예시 수정안
export const usePlaylistDetail = ( playlistId: number | null | undefined, // TODO: mycd 레거시 정리 시 null 타입도 삭제 options?: { enabled?: boolean } ) => { + const hasPlaylistId = playlistId != null + return useQuery({ queryKey: ['playlistDetail', playlistId], - queryFn: () => getPlaylistDetail(playlistId as number), - enabled: options?.enabled ?? !!playlistId, + queryFn: () => { + if (playlistId == null) throw new Error('playlistId is required') + return getPlaylistDetail(playlistId) + }, + enabled: hasPlaylistId && (options?.enabled ?? true), }) }🤖 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 67 - 75, The hook usePlaylistDetail allows playlistId to be undefined but still calls getPlaylistDetail via queryFn, so when options.enabled is true and playlistId is undefined it triggers an invalid request; modify usePlaylistDetail so enabled is computed as options?.enabled ?? (playlistId != null) and guard the queryFn by checking playlistId (e.g., if playlistId == null throw or return a rejected promise) before calling getPlaylistDetail(playlistId as number) to ensure getPlaylistDetail is never invoked with undefined.src/widgets/playlist/PlayerLayout.tsx (1)
11-17:⚠️ Potential issue | 🟠 Major
PlaylistProviderkey prop 제거로 인한 상태 초기화 누락PlayerLayout에서 key prop을 제거하면, 다른 큐레이션 번들 간 이동 시 컴포넌트가 리마운트되지 않아
currentPlaylist,currentTrackIndex등의 상태가 유지됩니다. 특히 같은 playlistId가 여러 번들에 존재할 경우, playlistDetail 쿼리가 새로 실행되지 않아 이전 플레이리스트 데이터가 노출될 수 있습니다.MyCdLayout은 의도적으로
key="my-cd"를 사용하여 상태를 격리하고 있으므로, PlayerLayout도 번들 변경 시 상태를 초기화하는 메커니즘이 필요합니다. bundleId 변경을 감지하여 PlaylistProvider를 리마운트하거나, 명시적으로 상태를 초기화하는 로직을 추가하세요.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/widgets/playlist/PlayerLayout.tsx` around lines 11 - 17, PlayerLayout currently renders <PlaylistProvider> without a key so provider state (currentPlaylist, currentTrackIndex) persists across bundle changes; make the provider remount when bundleId changes by passing a stable unique key (e.g., key={bundleId}) to PlaylistProvider or by subscribing to bundleId changes and calling the provider's reset method; ensure the key is derived from the bundle identifier used to fetch playlistDetail so navigating between bundles forces a remount and clears state.
🧹 Nitpick comments (7)
src/pages/home/index.tsx (1)
57-60: 콘텐츠 노출 갯수 변경은 PR 목적에 부합합니다.변경된 limit 값들(7, 5, 7)이 API에 올바르게 전달되며, 홈 화면 추천 콘텐츠 수를 조정하는 의도대로 동작합니다.
다만, 변수명이 PascalCase(
AdminRecommendData,WeeklyRecommendData,UserRecommendData)로 작성되어 있습니다. 코딩 가이드라인에 따르면 변수는 camelCase를 사용해야 합니다.,
♻️ 변수명 camelCase 적용 제안
- const { data: AdminRecommendData } = useAdminRecommendation(7) - const { data: WeeklyRecommendData } = useWeeklyRecommendation(5) + const { data: adminRecommendData } = useAdminRecommendation(7) + const { data: weeklyRecommendData } = useWeeklyRecommendation(5) const { data: GenreData } = useRecommendedGenres() - const { data: UserRecommendData } = useUserRecommendation(7) + const { data: genreData } = useRecommendedGenres() + const { data: userRecommendData } = useUserRecommendation(7)렌더링 부분에서도 변수명을 일괄 변경해야 합니다:
AdminRecommendData→adminRecommendData(Line 80)WeeklyRecommendData→weeklyRecommendData(Line 110)UserRecommendData→userRecommendData(Line 126)GenreData→genreData(Line 138)As per coding guidelines: "유틸리티/훅/변수: camelCase (예: getUserData, useUserInfo)"
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/home/index.tsx` around lines 57 - 60, Rename the PascalCase response variables to camelCase and update all usages: change AdminRecommendData→adminRecommendData for the result of useAdminRecommendation(7), WeeklyRecommendData→weeklyRecommendData for useWeeklyRecommendation(5), UserRecommendData→userRecommendData for useUserRecommendation(7), and GenreData→genreData for useRecommendedGenres(); then update every render/reference site where those variables are used (the JSX render blocks) to the new camelCase names so imports/hooks remain unchanged and only the local variable names are updated.src/pages/search/SearchResultPage.tsx (1)
56-63: UX 개선 제안: 미리 로딩을 위한rootMargin추가 고려현재
LoadingTrigger가 1px 높이이고threshold: 0이므로, 사용자가 목록의 맨 끝까지 스크롤해야 다음 페이지가 로드됩니다.rootMargin을 추가하면 목록 끝에 도달하기 전에 미리 데이터를 불러와 더 부드러운 스크롤 경험을 제공할 수 있습니다.💡 rootMargin 추가 예시
const { ref: targetRef } = useInView({ threshold: 0, + rootMargin: '100px', onChange: (inView) => { if (inView && current.hasNextPage && !current.isFetchingNextPage) { current.fetchNextPage() } }, })Also applies to: 187-189
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/search/SearchResultPage.tsx` around lines 56 - 63, The IntersectionObserver used in useInView (ref: targetRef) currently has threshold: 0 which requires the trigger element (LoadingTrigger) to be visible; update the hook options to include a rootMargin (e.g. "200px 0px" or "100px 0px") and optionally a small threshold so next-page fetching (current.fetchNextPage) starts before the user reaches the absolute bottom, improving perceived performance; apply the same change to the other useInView instance that also controls LoadingTrigger/fetchNextPage.src/pages/curation/play/index.tsx (1)
17-17:routePlaylistId가undefined일 때NaN이 됩니다.
Number(undefined)는NaN을 반환합니다. 현재 코드는NaN이 falsy라서 동작하지만, 의도가 명확하지 않습니다.♻️ 명시적인 null 처리 제안
- const routeId = Number(routePlaylistId) + const routeId = routePlaylistId ? Number(routePlaylistId) : null🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/curation/play/index.tsx` at line 17, routePlaylistId is converted with Number(routePlaylistId) which yields NaN when routePlaylistId is undefined; change the assignment of routeId to explicitly handle undefined/null (e.g., return null or undefined when routePlaylistId is absent, otherwise convert to a number) and update any downstream checks that currently rely on falsy NaN behavior to instead test for a defined routeId (or use Number.isFinite/Number.isNaN) so logic using routeId is clear and robust; refer to the routePlaylistId variable and the routeId constant in this file when making the change.src/pages/feed/ui/FeedCarousel.tsx (2)
141-158:togglePublicMutation캐시 무효화 중복
useMyCdActions훅 내부에서togglePublicMutation.onSuccess가 이미['playlistDetail', cdId],['getTracklist', cdId],['myCdList'],['feedCdList']를 무효화하고 있습니다. 여기서 Line 145의queryClient.invalidateQueries({ queryKey: ['playlistDetail', anchorId] })는 중복 호출입니다.♻️ 중복 무효화 제거
if (optionType === 'toggleVisibility') { togglePublicMutation.mutate(undefined, { onSuccess: () => { toast(playlistDetail?.isPublic ? 'PRIVATE' : 'PUBLIC') - queryClient.invalidateQueries({ queryKey: ['playlistDetail', anchorId] }) }, onError: () => {🤖 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 141 - 158, The code redundantly invalidates the same cache key: remove the extra invalidateQueries call inside the FeedCarousel handler—specifically delete or omit the queryClient.invalidateQueries({ queryKey: ['playlistDetail', anchorId] }) line in the togglePublicMutation.mutate success callback because useMyCdActions.togglePublicMutation already invalidates ['playlistDetail', cdId] (and related keys); keep the toast and modal behavior but rely on the hook's onSuccess cache invalidation to avoid duplicate invalidation.
101-112:onModalClose참조 타이밍 문제
onModalClose함수가 Line 101에서 정의되고 Line 102-112의useState초기값에서 참조됩니다. JavaScript 호이스팅으로 인해 현재는 동작하지만, 코드 순서상 가독성이 떨어지고 유지보수 시 혼란을 줄 수 있습니다.♻️ 순서 재배치 또는 인라인 처리
- const onModalClose = () => setModal((prev) => ({ ...prev, isOpen: false })) const [modal, setModal] = useState<ModalProps>({ isOpen: false, title: '', description: '', ctaType: 'single', confirmText: '확인', cancelText: '취소', - onClose: onModalClose, - onConfirm: onModalClose, - onCancel: onModalClose, + onClose: () => {}, + onConfirm: () => {}, + onCancel: () => {}, }) + + const onModalClose = () => setModal((prev) => ({ ...prev, isOpen: false }))🤖 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 101 - 112, The modal close handler reference timing is fragile: ensure onModalClose is defined before it's used in the useState initial value or avoid referencing external handlers in the initial state; specifically, move the onModalClose function declaration to precede the useState call (or inline simple handlers directly into the ModalProps initial object), and/or wrap onModalClose in useCallback, then update the useState initialization to reference the already-declared onModalClose so ModalProps' onClose/onConfirm/onCancel reliably point to the correct function (references: onModalClose, setModal, useState, ModalProps).src/widgets/playlist/PlaylistCarousel.tsx (1)
30-30:window.innerHeight직접 접근은 반응형이 아님
window.innerHeight는 컴포넌트 마운트 시점의 값만 캡처하며, 화면 크기 변경 시 업데이트되지 않습니다. 사용자가 브라우저 높이를 조절하거나 모바일에서 키보드가 열릴 때isSmall값이 갱신되지 않습니다.♻️ resize 이벤트 반응형 처리
+import { useEffect, useCallback, useState, useMemo } from 'react' + +const useWindowHeight = () => { + const [height, setHeight] = useState(window.innerHeight) + useEffect(() => { + const handler = () => setHeight(window.innerHeight) + window.addEventListener('resize', handler) + return () => window.removeEventListener('resize', handler) + }, []) + return height +} + const PlaylistCarousel = ({ ... }) => { const { isMobile } = useDevice() - const isSmall = isMobile && window.innerHeight < 633 + const windowHeight = useWindowHeight() + const isSmall = isMobile && windowHeight < 633현재 기능에 큰 영향이 없다면 추후 개선으로 미뤄도 됩니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/widgets/playlist/PlaylistCarousel.tsx` at line 30, The code reads window.innerHeight directly into the isSmall constant which only captures height at render time; make isSmall reactive by storing height in state and updating it on resize/visual viewport changes: in PlaylistCarousel (where isSmall is defined) create a height state (e.g., viewportHeight), initialize from window.innerHeight (or visualViewport.height when available), add a useEffect that subscribes to window.resize and visualViewport.onresize to setViewportHeight, derive isSmall = isMobile && viewportHeight < 633, and clean up listeners on unmount to avoid leaks.src/widgets/playlist/PlayerLayout.tsx (1)
31-39: Outlet context 타입 정보 손실로 타입 안전성 저하
useOutletContext()가 타입 파라미터 없이 호출되어unknown타입을 반환합니다. 하위 라우트인src/pages/curation/index.tsx와src/pages/curation/play/index.tsx에서BundleInfo타입을 기대하고 있어, 타입 불일치 시 런타임 에러가 발생할 수 있습니다.♻️ 타입 안전성을 위한 제네릭 타입 추가
+import type { BundleInfo } from '@/entities/bundle' + const Content = () => { // ... - const context = useOutletContext() + const context = useOutletContext<BundleInfo>() // ... return ( <> - <Outlet context={context} /> + <Outlet context={context} />코딩 가이드라인에 따르면 "타입 명시적으로 작성 (any 사용 지양)" 원칙을 준수해야 합니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/widgets/playlist/PlayerLayout.tsx` around lines 31 - 39, The call to useOutletContext in PlayerLayout is untyped and returns unknown, reducing type safety for child routes expecting BundleInfo; update useOutletContext to pass the correct generic (e.g., useOutletContext<BundleInfo>()) and ensure the Outlet is rendered with the typed context so downstream components (such as those in src/pages/curation/index.tsx and src/pages/curation/play/index.tsx) receive a BundleInfo rather than unknown; also import or reference the BundleInfo type where PlayerLayout is defined and adjust any local variables (context) to the typed version.
🤖 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/features/like/ui/LikeButton.tsx`:
- Around line 75-79: Remove the leftover console.log from the onConfirm handler
in the LikeButton component; locate the onConfirm block (inside the modal
confirm callback in LikeButton.tsx) and delete the line console.log('누름') so the
handler only calls navigate('/login') and setIsModalOpen(false), keeping
behavior unchanged and satisfying the linter/code guidelines that only
console.warn/error are allowed.
In `@src/pages/customize/step3/index.tsx`:
- Around line 25-27: The refetch call uses an incorrect key shape for the
feedCdList cache (queryClient.refetchQueries with ['feedCdList',
userInfo.shareCode, 'cds']) which doesn't match useCarouselCdList's key
structure; replace this with a proper invalidateQueries call or the exact
matching key shape. Either call queryClient.invalidateQueries(['feedCdList']) to
broadly invalidate all feed lists (consistent with step2 and useMyCd), or build
the exact key used by useCarouselCdList (['feedCdList', type, shareCode,
params.sort, params.anchorId]) and call queryClient.invalidateQueries(...) so
prefix matching works correctly; update the code around
queryClient.refetchQueries and reference
useCarouselCdList/queryClient.invalidateQueries accordingly.
In `@src/pages/feed/ui/FeedCarousel.tsx`:
- Around line 56-59: The hook is being called with a default 0 when anchorId is
undefined which can cause accidental API calls to cdId 0 if deleteMutation or
togglePublicMutation are invoked; update the call to useMyCdActions so it only
receives a valid id (pass undefined/null or skip the hook when anchorId is
falsy) and add a defensive check in the callers (e.g., in handleOptionClick) to
verify anchorId is a valid number before calling deleteMutation.mutate or
togglePublicMutation.mutate; reference the useMyCdActions call site and the
deleteMutation/togglePublicMutation usage to implement both the hook parameter
change and the guard in the event handler.
- Around line 125-134: The delete flow in onConfirm uses deleteMutation.mutate
with only onSuccess; add an onError handler to handle deletion failures: inside
the mutate call (deleteMutation.mutate(undefined, { ... })) add onError that
closes or keeps the modal consistent (use onModalClose as appropriate), shows an
error toast/message via toast (e.g., 'CD_DELETE_FAILED' or the error message),
and refrains from navigating or invalidating queries; keep using getNextId,
navigate, and queryClient.invalidateQueries only in onSuccess. Ensure you
reference deleteMutation.mutate, onConfirm, onModalClose, toast, getNextId,
navigate, and queryClient.invalidateQueries when implementing the handler.
In `@src/shared/config/routesConfig.ts`:
- Line 39: The /mycd route was removed from routesConfig.ts but related
references remain and cause 404s; either fully remove the mycd feature or
restore the route. To fix: (A) If deleting the feature, remove the entire
src/pages/mycd directory and delete any imports/usages: clear
navigate('/mycd/${playlistId}') calls in the file that contains those calls
(src/pages/mycd/index.tsx), remove basePath="/mycd" in PlaylistCarousel
(src/pages/mycd/ui/PlaylistCarousel.tsx), and delete the
pathname.startsWith('/mycd') check in App.tsx; or (B) if restoring the route,
add the /mycd route back into routesConfig.ts (the routing entry that previously
referenced PlayerLayout or the playlist UI) so the existing navigate calls,
PlaylistCarousel basePath, and App.tsx check resolve correctly. Ensure you
update or remove any imports that reference src/pages/mycd to avoid dead-code.
In `@src/shared/lib/useMarquee.ts`:
- Around line 35-43: The useEffect that triggers auto-run (inside useMarquee)
currently depends only on isPlaying and isOverflow and never resets
hasAutoPlayedRef, so when the component is reused with a new text the auto-run
won’t restart; update the effect to include text (or a title identifier) in its
dependency array and reset hasAutoPlayedRef.current = false when the text
changes (or on effect cleanup) so that setIsAutoRunning and the auto-run logic
can run again for new titles; locate the useEffect, hasAutoPlayedRef, and
setIsAutoRunning in useMarquee to implement these dependency and reset changes.
- Around line 17-33: The effect that sets isOverflow (useEffect containing
checkOverflow and timer) only runs when text or isMobile changes, so it never
recalculates after parent/viewport size changes; update this by wiring
checkOverflow to resize/orientation events or, preferably, a ResizeObserver on
the element's parent (or titleRef.current) so any layout change triggers
recomputation, ensure the observer/listeners are created on mount and cleaned up
in the effect return, and keep references to titleRef, checkOverflow, and
isOverflow handling (the useEffect, titleRef, checkOverflow, and setIsOverflow)
so the state updates correctly when the parent width changes.
In `@src/widgets/playlist/PlaylistCarousel.tsx`:
- Around line 82-89: handleProgressClick sets the playlist and calls play() when
starting playback but misses the mobile unmute step; update handleProgressClick
(the useCallback defined as handleProgressClick) to call unmuteOnce() before
invoking play() when !isPlaying (mirror the behavior in handleTogglePlay),
ensuring you reference currentPlaylist, setPlaylist, isPlaying, play, and
unmuteOnce so progress-click-initiated playback will unmute on mobile.
- Around line 63-67: The effect in PlaylistCarousel is being retriggered because
setPlaylist is not memoized (PlayerProvider recreates its value each render), so
either memoize setPlaylist where it’s defined (wrap the setter in useCallback
inside PlayerProvider so the function identity is stable) or remove setPlaylist
from the useEffect dependency list and access the stable setter via a useRef;
update the provider to export the memoized setPlaylist or change
PlaylistCarousel to read a ref to the setter so the useEffect([playlistDetail,
currentPlaylist, isMobile]) no longer reruns due to a changing function
reference.
In `@src/widgets/playlist/PlaylistInfo.tsx`:
- Around line 76-79: The Wrapper uses the flexColCenter mixin (which contains
justify-content: center) together with height: 100dvh causing the header+content
to be vertically centered; override that by setting justify-content: flex-start
on the Wrapper (or stop using flexColCenter and reapply only needed styles) so
the page content is aligned to the top — update the styled component named
Wrapper to explicitly set justify-content: flex-start (or replace flexColCenter
with a mixin that does align-start) to restore top alignment.
---
Outside diff comments:
In `@src/entities/playlist/model/usePlaylists.ts`:
- Around line 67-75: The hook usePlaylistDetail allows playlistId to be
undefined but still calls getPlaylistDetail via queryFn, so when options.enabled
is true and playlistId is undefined it triggers an invalid request; modify
usePlaylistDetail so enabled is computed as options?.enabled ?? (playlistId !=
null) and guard the queryFn by checking playlistId (e.g., if playlistId == null
throw or return a rejected promise) before calling getPlaylistDetail(playlistId
as number) to ensure getPlaylistDetail is never invoked with undefined.
In `@src/widgets/playlist/PlayerLayout.tsx`:
- Around line 11-17: PlayerLayout currently renders <PlaylistProvider> without a
key so provider state (currentPlaylist, currentTrackIndex) persists across
bundle changes; make the provider remount when bundleId changes by passing a
stable unique key (e.g., key={bundleId}) to PlaylistProvider or by subscribing
to bundleId changes and calling the provider's reset method; ensure the key is
derived from the bundle identifier used to fetch playlistDetail so navigating
between bundles forces a remount and clears state.
---
Nitpick comments:
In `@src/pages/curation/play/index.tsx`:
- Line 17: routePlaylistId is converted with Number(routePlaylistId) which
yields NaN when routePlaylistId is undefined; change the assignment of routeId
to explicitly handle undefined/null (e.g., return null or undefined when
routePlaylistId is absent, otherwise convert to a number) and update any
downstream checks that currently rely on falsy NaN behavior to instead test for
a defined routeId (or use Number.isFinite/Number.isNaN) so logic using routeId
is clear and robust; refer to the routePlaylistId variable and the routeId
constant in this file when making the change.
In `@src/pages/feed/ui/FeedCarousel.tsx`:
- Around line 141-158: The code redundantly invalidates the same cache key:
remove the extra invalidateQueries call inside the FeedCarousel
handler—specifically delete or omit the queryClient.invalidateQueries({
queryKey: ['playlistDetail', anchorId] }) line in the
togglePublicMutation.mutate success callback because
useMyCdActions.togglePublicMutation already invalidates ['playlistDetail', cdId]
(and related keys); keep the toast and modal behavior but rely on the hook's
onSuccess cache invalidation to avoid duplicate invalidation.
- Around line 101-112: The modal close handler reference timing is fragile:
ensure onModalClose is defined before it's used in the useState initial value or
avoid referencing external handlers in the initial state; specifically, move the
onModalClose function declaration to precede the useState call (or inline simple
handlers directly into the ModalProps initial object), and/or wrap onModalClose
in useCallback, then update the useState initialization to reference the
already-declared onModalClose so ModalProps' onClose/onConfirm/onCancel reliably
point to the correct function (references: onModalClose, setModal, useState,
ModalProps).
In `@src/pages/home/index.tsx`:
- Around line 57-60: Rename the PascalCase response variables to camelCase and
update all usages: change AdminRecommendData→adminRecommendData for the result
of useAdminRecommendation(7), WeeklyRecommendData→weeklyRecommendData for
useWeeklyRecommendation(5), UserRecommendData→userRecommendData for
useUserRecommendation(7), and GenreData→genreData for useRecommendedGenres();
then update every render/reference site where those variables are used (the JSX
render blocks) to the new camelCase names so imports/hooks remain unchanged and
only the local variable names are updated.
In `@src/pages/search/SearchResultPage.tsx`:
- Around line 56-63: The IntersectionObserver used in useInView (ref: targetRef)
currently has threshold: 0 which requires the trigger element (LoadingTrigger)
to be visible; update the hook options to include a rootMargin (e.g. "200px 0px"
or "100px 0px") and optionally a small threshold so next-page fetching
(current.fetchNextPage) starts before the user reaches the absolute bottom,
improving perceived performance; apply the same change to the other useInView
instance that also controls LoadingTrigger/fetchNextPage.
In `@src/widgets/playlist/PlayerLayout.tsx`:
- Around line 31-39: The call to useOutletContext in PlayerLayout is untyped and
returns unknown, reducing type safety for child routes expecting BundleInfo;
update useOutletContext to pass the correct generic (e.g.,
useOutletContext<BundleInfo>()) and ensure the Outlet is rendered with the typed
context so downstream components (such as those in src/pages/curation/index.tsx
and src/pages/curation/play/index.tsx) receive a BundleInfo rather than unknown;
also import or reference the BundleInfo type where PlayerLayout is defined and
adjust any local variables (context) to the typed version.
In `@src/widgets/playlist/PlaylistCarousel.tsx`:
- Line 30: The code reads window.innerHeight directly into the isSmall constant
which only captures height at render time; make isSmall reactive by storing
height in state and updating it on resize/visual viewport changes: in
PlaylistCarousel (where isSmall is defined) create a height state (e.g.,
viewportHeight), initialize from window.innerHeight (or visualViewport.height
when available), add a useEffect that subscribes to window.resize and
visualViewport.onresize to setViewportHeight, derive isSmall = isMobile &&
viewportHeight < 633, and clean up listeners on unmount to avoid leaks.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: f23bf09f-6740-4320-9e5f-defe21004388
📒 Files selected for processing (27)
src/app/providers/PlayerProvider.tsxsrc/entities/playlist/model/usePlaylists.tssrc/features/like/model/useLike.tssrc/features/like/ui/LikeButton.tsxsrc/pages/curation/CurationLayout.tsxsrc/pages/curation/index.tsxsrc/pages/curation/play/index.tsxsrc/pages/curation/ui/CurationCarousel.tsxsrc/pages/curation/ui/index.tssrc/pages/customize/step3/index.tsxsrc/pages/feed/ui/CarouselItem.tsxsrc/pages/feed/ui/FeedCarousel.tsxsrc/pages/feed/ui/index.tssrc/pages/feed/ui/layout/CdPlayerLayout.tsxsrc/pages/home/index.tsxsrc/pages/search/SearchResultPage.tsxsrc/shared/config/routesConfig.tssrc/shared/lib/index.tssrc/shared/lib/useMarquee.tssrc/shared/styles/mixins.tssrc/shared/ui/ContentHeader.tsxsrc/shared/ui/Modal.tsxsrc/widgets/playlist/PlayerLayout.tsxsrc/widgets/playlist/PlaylistCarousel.tsxsrc/widgets/playlist/PlaylistInfo.tsxsrc/widgets/playlist/PlaylistLayout.tsxsrc/widgets/playlist/index.ts
💤 Files with no reviewable changes (5)
- src/pages/curation/ui/index.ts
- src/pages/feed/ui/index.ts
- src/pages/feed/ui/layout/CdPlayerLayout.tsx
- src/pages/curation/ui/CurationCarousel.tsx
- src/pages/feed/ui/CarouselItem.tsx
src/pages/customize/step3/index.tsx
Outdated
| await queryClient.refetchQueries({ | ||
| queryKey: ['feedCdList', userInfo.shareCode, 'cds'], // 피드 리스트만 타겟팅 | ||
| }) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find and examine the target file
find . -name "index.tsx" -path "*/customize/step3/*" | head -5Repository: dnd-side-project/dnd-13th-8-frontend
Length of output: 117
🏁 Script executed:
# Look for usePlaylists.ts file
find . -name "usePlaylists.ts" -o -name "*Playlist*" -type f | grep -E "\.(ts|tsx)$" | head -10Repository: dnd-side-project/dnd-13th-8-frontend
Length of output: 495
🏁 Script executed:
# Search for useCarouselCdList hook definition
rg "useCarouselCdList" --type ts --type tsx -B 2 -A 10 | head -50Repository: dnd-side-project/dnd-13th-8-frontend
Length of output: 107
🏁 Script executed:
# Read the target file to see lines 25-27
cat -n ./src/pages/customize/step3/index.tsx | head -40Repository: dnd-side-project/dnd-13th-8-frontend
Length of output: 1676
🏁 Script executed:
# Read usePlaylists.ts to find useCarouselCdList definition
cat -n ./src/entities/playlist/model/usePlaylists.ts | head -100Repository: dnd-side-project/dnd-13th-8-frontend
Length of output: 3459
🏁 Script executed:
# Search for useCarouselCdList in usePlaylists.ts
grep -n "useCarouselCdList" ./src/entities/playlist/model/usePlaylists.ts -A 20Repository: dnd-side-project/dnd-13th-8-frontend
Length of output: 836
🏁 Script executed:
# Also search for useFeedCdList since that might be related
grep -n "useFeedCdList\|feedCdList" ./src/entities/playlist/model/usePlaylists.ts -B 2 -A 15Repository: dnd-side-project/dnd-13th-8-frontend
Length of output: 1558
🏁 Script executed:
# Check what page is navigated to (line 28 shows the navigation)
cat -n ./src/pages/customize/step3/index.tsx | sed -n '28p'Repository: dnd-side-project/dnd-13th-8-frontend
Length of output: 144
🏁 Script executed:
# Search for what queries are used on the CD detail page (the destination)
find ./src/pages -name "*.tsx" -path "*cds*" | head -10Repository: dnd-side-project/dnd-13th-8-frontend
Length of output: 110
🏁 Script executed:
# Look for the CD detail page that might use carousel
find ./src/pages -type d -name "cds"Repository: dnd-side-project/dnd-13th-8-frontend
Length of output: 100
🏁 Script executed:
# Read the feed/cds page to see what queries it uses
cat -n ./src/pages/feed/cds/index.tsxRepository: dnd-side-project/dnd-13th-8-frontend
Length of output: 242
🏁 Script executed:
# Find FeedCarousel component
find ./src -name "*FeedCarousel*" -o -name "FeedCarousel.tsx"Repository: dnd-side-project/dnd-13th-8-frontend
Length of output: 116
🏁 Script executed:
# Search for FeedCarousel in feed/ui directory
ls -la ./src/pages/feed/ui/Repository: dnd-side-project/dnd-13th-8-frontend
Length of output: 692
🏁 Script executed:
# Read FeedCarousel component
cat -n ./src/pages/feed/ui/FeedCarousel.tsx | head -100Repository: dnd-side-project/dnd-13th-8-frontend
Length of output: 3809
🏁 Script executed:
# Check if there are other refetch/invalidate calls nearby to understand the pattern
grep -n "refetchQueries\|invalidateQueries" ./src/pages/customize/step3/index.tsxRepository: dnd-side-project/dnd-13th-8-frontend
Length of output: 286
🏁 Script executed:
# Also check if there's any other place where feedCdList is being refetched/invalidated
rg "feedCdList" --type js --type ts -B 2 -A 2 | head -60Repository: dnd-side-project/dnd-13th-8-frontend
Length of output: 3329
🏁 Script executed:
# Check the TanStack Query documentation or examples in the codebase about prefix matching
# Look at how invalidateQueries with partial keys is used
rg "invalidateQueries.*queryKey.*\['[^,]+'\]" --type js --type ts -A 1 | head -40Repository: dnd-side-project/dnd-13th-8-frontend
Length of output: 1776
🏁 Script executed:
# Check if there are comments or documentation about query key structure
grep -r "queryKey\|query key" ./src --include="*.ts" --include="*.tsx" -B 1 | grep -i "comment\|//" | head -20Repository: dnd-side-project/dnd-13th-8-frontend
Length of output: 388
🏁 Script executed:
# Check TanStack Query version to understand behavior
cat ./package.json | grep -A 2 -B 2 "react-query\|@tanstack/react-query"Repository: dnd-side-project/dnd-13th-8-frontend
Length of output: 230
🏁 Script executed:
# Verify the actual destination page uses the carousel with correct params
cat -n ./src/pages/feed/ui/FeedCarousel.tsx | sed -n '70,82p'Repository: dnd-side-project/dnd-13th-8-frontend
Length of output: 391
🏁 Script executed:
# Let's verify by checking step2 page which also uses invalidateQueries on feedCdList
cat -n ./src/pages/customize/step2/index.tsx | grep -B 5 -A 2 "feedCdList"Repository: dnd-side-project/dnd-13th-8-frontend
Length of output: 444
🏁 Script executed:
# Also check the useMyCd to see the full context
cat -n ./src/entities/playlist/model/useMyCd.ts | grep -B 10 -A 5 "feedCdList"Repository: dnd-side-project/dnd-13th-8-frontend
Length of output: 689
feedCdList 캐시 갱신 전략을 수정하세요.
현재 refetch 키 ['feedCdList', userInfo.shareCode, 'cds']는 이동 후 화면에서 사용하는 useCarouselCdList의 실제 쿼리 키 구조 ['feedCdList', type, shareCode, params.sort, params.anchorId]와 맞지 않습니다. 위치 2-3의 순서가 반대이고, 정렬/anchorId 정보도 포함되지 않아 prefix 매칭이 제대로 작동하지 않을 수 있습니다.
코드베이스의 다른 곳(step2, useMyCd)에서는 invalidateQueries({ queryKey: ['feedCdList'] })로 간단히 처리하고 있습니다. 이 방식을 따르거나, 정확한 키 구조를 맞추어 invalidateQueries를 사용하세요.
🛠️ 수정 예시
- await queryClient.refetchQueries({
- queryKey: ['feedCdList', userInfo.shareCode, 'cds'],
+ await queryClient.invalidateQueries({
+ queryKey: ['feedCdList'],🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/pages/customize/step3/index.tsx` around lines 25 - 27, The refetch call
uses an incorrect key shape for the feedCdList cache (queryClient.refetchQueries
with ['feedCdList', userInfo.shareCode, 'cds']) which doesn't match
useCarouselCdList's key structure; replace this with a proper invalidateQueries
call or the exact matching key shape. Either call
queryClient.invalidateQueries(['feedCdList']) to broadly invalidate all feed
lists (consistent with step2 and useMyCd), or build the exact key used by
useCarouselCdList (['feedCdList', type, shareCode, params.sort,
params.anchorId]) and call queryClient.invalidateQueries(...) so prefix matching
works correctly; update the code around queryClient.refetchQueries and reference
useCarouselCdList/queryClient.invalidateQueries accordingly.
There was a problem hiding this comment.
@maylh 앗 useFeedCdList랑 useCarouselCdList랑 인피니트쿼리 쿼리키가 겹치네요🫢
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
There was a problem hiding this comment.
@maylh 앗 useFeedCdList랑 useCarouselCdList랑 인피니트쿼리 쿼리키가 겹치네요🫢
key 순서가 달라서 겹치진 않는데 혹시 문제가 되는 상황이 있나요 ??
There was a problem hiding this comment.
@maylh 말씀주신대로 키가 달라서 기능적으로는 문제 없을 것 같아요!
근데 쿼리키는 식별자라서 데이터가 다르면 구분해주는게 좋아서 구조적 관점으로는 분리해주는게 좋을 것 같아요🤔
(ex. 저희 코드 상으로는 큰 문제 없지만 queryClient.invalidateQueries({ queryKey: ['feedCdList'] }) 하면 둘 다 무효화됨)
급한건 아니라서 나중에 마이페이지 레거시 코드 삭제하면서 분리해둘게요!
| const handleProgressClick = useCallback( | ||
| (trackIndex: number, seconds: number) => { | ||
| if (!currentPlaylist) return | ||
| setPlaylist(currentPlaylist, trackIndex, seconds) | ||
| if (!isPlaying) play() | ||
| }, | ||
| [currentPlaylist, setPlaylist, isPlaying, play] | ||
| ) |
There was a problem hiding this comment.
handleProgressClick에서 play() 호출 시 모바일 unmute 처리 누락
handleTogglePlay에서는 모바일에서 재생 시작 시 unmuteOnce()를 호출하지만, handleProgressClick에서 play()를 호출할 때는 unmuteOnce()가 누락되어 있습니다. 사용자가 progress bar를 클릭해 재생을 시작하면 음소거 상태가 유지될 수 있습니다.
🐛 모바일 unmute 처리 추가
const handleProgressClick = useCallback(
(trackIndex: number, seconds: number) => {
if (!currentPlaylist) return
setPlaylist(currentPlaylist, trackIndex, seconds)
- if (!isPlaying) play()
+ if (!isPlaying) {
+ if (isMobile) unmuteOnce()
+ play()
+ }
},
- [currentPlaylist, setPlaylist, isPlaying, play]
+ [currentPlaylist, setPlaylist, isPlaying, play, isMobile, unmuteOnce]
)📝 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 handleProgressClick = useCallback( | |
| (trackIndex: number, seconds: number) => { | |
| if (!currentPlaylist) return | |
| setPlaylist(currentPlaylist, trackIndex, seconds) | |
| if (!isPlaying) play() | |
| }, | |
| [currentPlaylist, setPlaylist, isPlaying, play] | |
| ) | |
| const handleProgressClick = useCallback( | |
| (trackIndex: number, seconds: number) => { | |
| if (!currentPlaylist) return | |
| setPlaylist(currentPlaylist, trackIndex, seconds) | |
| if (!isPlaying) { | |
| if (isMobile) unmuteOnce() | |
| play() | |
| } | |
| }, | |
| [currentPlaylist, setPlaylist, isPlaying, play, isMobile, unmuteOnce] | |
| ) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/widgets/playlist/PlaylistCarousel.tsx` around lines 82 - 89,
handleProgressClick sets the playlist and calls play() when starting playback
but misses the mobile unmute step; update handleProgressClick (the useCallback
defined as handleProgressClick) to call unmuteOnce() before invoking play() when
!isPlaying (mirror the behavior in handleTogglePlay), ensuring you reference
currentPlaylist, setPlaylist, isPlaying, play, and unmuteOnce so
progress-click-initiated playback will unmute on mobile.
hansololiviakim
left a comment
hansololiviakim
left a comment
There was a problem hiding this comment.
고생하셨습니다 👏🍀👏🍀👏🍀
혹시 marquee 시작할 때 setTimeout처럼 한 2-3초 멈췄다가 흐르는 것도 가능한가요?!
둘러보기 스와이프하면서 아련+몽환 여돌플리(/discover/65) 보니까 바로 흘러서 앞부분 제목이 잘 안보이는 것 같아 여쭤봅니다..!
src/pages/customize/step3/index.tsx
Outdated
| await queryClient.refetchQueries({ | ||
| queryKey: ['feedCdList', userInfo.shareCode, 'cds'], // 피드 리스트만 타겟팅 | ||
| }) |
There was a problem hiding this comment.
@maylh 앗 useFeedCdList랑 useCarouselCdList랑 인피니트쿼리 쿼리키가 겹치네요🫢
| color: ${({ theme }) => theme.COLOR['gray-10']}; | ||
| flex-shrink: 0; | ||
| margin-right: 8px; | ||
| line-height: 1; |
There was a problem hiding this comment.
왼쪽이 개발계 / 오른쪽이 로컬인데 line-height: 1 적용되면서 글씨가 위로 올라와서 폭이 살짝 좁아진 것 같아요!👀
가능합니다 ! 근데 2-3초는 좀 긴 것 같아서 우선 1초로 해두겠습니다 |
2 participants
🛰️ 관련 이슈
✨ 주요 변경 사항
1️⃣ 메인
2️⃣ 검색
react-intersection-observer적용 -> 스크롤 이슈 해결3️⃣ 피드
cd 삭제 시 404 모달 출력)4️⃣ 큐레이션
5️⃣ 기타
/mycd라우트 제거🗯️ PR 포인트
🚀 알게된 점
📖 참고 자료 (선택)
Summary by CodeRabbit
릴리스 노트
새로운 기능
버그 수정
개선사항