Skip to content

[refactor] 둘러보기, 나의 cd 코드 리팩토링 #106

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
maylh merged 7 commits into from
Sep 22, 2025
Merged

[refactor] 둘러보기, 나의 cd 코드 리팩토링 #106

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
0af3a3e
feat: PlaylistInfo 컴포넌트 생성 및 적용 (#104)
maylh Sep 16, 2025
e58de60
fix: Title width를 100%로 수정 (#104)
maylh Sep 16, 2025
610304d
chore: main 브랜치를 프로덕션 브랜치로 설정 (#104)
maylh Sep 16, 2025
5d6988a
refactor: 트랙 제어 로직을 Provider 내부로 통합 (#104)
maylh Sep 17, 2025
c9450ba
chore: mock data 파일 삭제 (#104)
maylh Sep 17, 2025
2ea1fc6
refactor: 누적 시간 계산 로직 유틸로 분리 (#104)
maylh Sep 18, 2025
7b6700c
refactor: 재생 시간 interval ProgressBar로 이동 (#104)
maylh Sep 18, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 41 additions & 18 deletions src/app/providers/PlayerProvider.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { createContext, useState, useContext, type ReactNode } from 'react'
import { createContext, useState, useContext, useRef, useCallback, type ReactNode } from 'react'

import type { PlaylistInfo } from '@/entities/playlist'

Expand All @@ -13,6 +13,8 @@
nextTrack: () => void
prevTrack: () => void
updateCurrentTime: (time: number) => void
playerRef: React.MutableRefObject<YT.Player | null>
handlePlayerStateChange: (event: YT.OnStateChangeEvent) => void
}

interface PlaylistProviderProps {
Expand All @@ -23,37 +25,58 @@

const PlaylistProvider = ({ children }: PlaylistProviderProps) => {
const [currentPlaylist, setCurrentPlaylist] = useState<PlaylistInfo | null>(null)
const [currentTrackIndex, setCurrentTrackIndex] = useState<number>(0)
const [isPlaying, setIsPlaying] = useState<boolean>(false)
const [currentTime, setCurrentTime] = useState<number>(0)
const [currentTrackIndex, setCurrentTrackIndex] = useState(0)
const [currentTime, setCurrentTime] = useState(0)
const [isPlaying, setIsPlaying] = useState(false)

const playerRef = useRef<YT.Player | null>(null)

const setPlaylist = (playlist: PlaylistInfo, trackIndex?: number, time?: number) => {
setCurrentPlaylist(playlist)
if (trackIndex !== undefined) setCurrentTrackIndex(trackIndex)
if (time !== undefined) setCurrentTime(time)
setIsPlaying(true)

if (playerRef.current) {
if (time !== undefined) playerRef.current.seekTo(time, true)
playerRef.current.playVideo()
}
}
Comment on lines 34 to 44
Copy link

@gemini-code-assist gemini-code-assist bot Sep 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

setPlaylist 함수를 useCallback으로 감싸서 불필요한 리렌더링을 방지하는 것이 좋습니다. 현재 구현에서는 PlaylistProvider가 리렌더링될 때마다 새로운 setPlaylist 함수가 생성되어, 이를 사용하는 자식 컴포넌트들도 함께 리렌더링될 수 있습니다. setState 함수와 ref는 항상 동일한 참조를 유지하므로 의존성 배열은 비워두어도 괜찮습니다. 1

Suggested change
const setPlaylist = (playlist: PlaylistInfo, trackIndex?: number, time?: number) => {
setCurrentPlaylist(playlist)
if (trackIndex !== undefined) setCurrentTrackIndex(trackIndex)
if (time !== undefined) setCurrentTime(time)
setIsPlaying(true)
if (playerRef.current) {
if (time !== undefined) playerRef.current.seekTo(time, true)
playerRef.current.playVideo()
}
}
const setPlaylist = useCallback((playlist: PlaylistInfo, trackIndex?: number, time?: number) => {
setCurrentPlaylist(playlist)
if (trackIndex !== undefined) setCurrentTrackIndex(trackIndex)
if (time !== undefined) setCurrentTime(time)
setIsPlaying(true)
if (playerRef.current) {
if (time !== undefined) playerRef.current.seekTo(time, true)
playerRef.current.playVideo()
}
}, [])

Style Guide References

Footnotes

  1. React의 useCallback을 사용하여 불필요한 함수 생성을 방지하고, 이를 통해 자식 컴포넌트의 불필요한 리렌더링을 줄여 성능을 최적화해야 합니다. (link) ↩


const play = () => setIsPlaying(true)
const pause = () => setIsPlaying(false)
const play = useCallback(() => {
if (playerRef.current) playerRef.current.playVideo()
setIsPlaying(true)
}, [])

const pause = useCallback(() => {
if (playerRef.current) playerRef.current.pauseVideo()
setIsPlaying(false)
}, [])

const nextTrack = () => {
const nextTrack = useCallback(() => {
if (currentPlaylist && currentTrackIndex < currentPlaylist.songs.length - 1) {
setCurrentTrackIndex((prevIndex) => prevIndex + 1)
setCurrentTrackIndex((prev) => prev + 1)
setCurrentTime(0)
if (playerRef.current) playerRef.current.seekTo(0, true)
}
}
}, [currentPlaylist, currentTrackIndex])

Comment on lines +56 to 63
Copy link

@coderabbitai coderabbitai bot Sep 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

다음 트랙 이동 시 seekTo(0)는 세그먼트 기반 모델에서 오동작 가능

앨범 1개 영상에 트랙 세그먼트를 매핑(누적 시간 모델)한다면, 트랙 시작 누적 시점으로 seek 해야 합니다. 0초로 이동하면 항상 영상 처음으로 돌아갑니다. getAccTime 유틸을 활용해 누적 시작 시점으로 이동하도록 수정 제안합니다.

-  const nextTrack = useCallback(() => {
-    if (currentPlaylist && currentTrackIndex < currentPlaylist.songs.length - 1) {
-      setCurrentTrackIndex((prev) => prev + 1)
-      setCurrentTime(0)
-      if (playerRef.current) playerRef.current.seekTo(0, true)
-    }
-  }, [currentPlaylist, currentTrackIndex])
+  const nextTrack = useCallback(() => {
+    if (!currentPlaylist) return
+    if (currentTrackIndex >= currentPlaylist.songs.length - 1) return
+    const nextIndex = currentTrackIndex + 1
+    setCurrentTrackIndex(nextIndex)
+    setCurrentTime(0)
+    // FIXME: 아래 trackLengths는 실제 트랙 길이(초) 배열로 매핑하세요.
+    // 예: const trackLengths = currentPlaylist.songs.map((s) => s.lengthSec)
+    // 혹은 데이터 구조에 맞는 속성으로 교체
+    const trackLengths: number[] =
+      currentPlaylist.songs.map((/* s */) => /* lengthSec */ 0) // TODO: implement
+    if (playerRef.current) {
+      const acc = getAccTime(trackLengths, nextIndex, 0)
+      playerRef.current.seekTo(acc, true)
+    }
+  }, [currentPlaylist, currentTrackIndex])

추가: 상단 import

-import { createContext, useState, useContext, useRef, useCallback, type ReactNode } from 'react'
+import { createContext, useState, useContext, useRef, useCallback, type ReactNode } from 'react'
+import { getAccTime } from '@/shared/lib'
📝 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.

Suggested change
const nextTrack = useCallback(() => {
if (currentPlaylist && currentTrackIndex < currentPlaylist.songs.length - 1) {
setCurrentTrackIndex((prevIndex) => prevIndex + 1)
setCurrentTrackIndex((prev) => prev + 1)
setCurrentTime(0)
if (playerRef.current) playerRef.current.seekTo(0, true)
}
}
}, [currentPlaylist, currentTrackIndex])
import { createContext, useState, useContext, useRef, useCallback, type ReactNode } from 'react'
import { getAccTime } from '@/shared/lib'
const nextTrack = useCallback(() => {
if (!currentPlaylist) return
if (currentTrackIndex >= currentPlaylist.songs.length - 1) return
const nextIndex = currentTrackIndex + 1
setCurrentTrackIndex(nextIndex)
setCurrentTime(0)
// FIXME: 아래 trackLengths는 실제 트랙 길이(초) 배열로 매핑하세요.
// 예: const trackLengths = currentPlaylist.songs.map((s) => s.lengthSec)
// 혹은 데이터 구조에 맞는 속성으로 교체
const trackLengths: number[] =
currentPlaylist.songs.map((/* s */) => /* lengthSec */ 0) // TODO: implement
if (playerRef.current) {
const acc = getAccTime(trackLengths, nextIndex, 0)
playerRef.current.seekTo(acc, true)
}
}, [currentPlaylist, currentTrackIndex])
🤖 Prompt for AI Agents 
In src/app/providers/PlayerProvider.tsx around lines 56 to 63, nextTrack
currently seeks to 0 which breaks segment-based (accumulated time) playlists;
instead compute the track's accumulated start time with the getAccTime util and
seek there. Change the seek call to
playerRef.current.seekTo(getAccTime(currentPlaylist.songs, currentTrackIndex +
1), true) (use the new index after incrementing), keep setCurrentTime(0) for
track-relative position, and add an import for getAccTime at the top of the
file.

const prevTrack = () => {
const prevTrack = useCallback(() => {
if (currentTrackIndex > 0) {
setCurrentTrackIndex((prevIndex) => prevIndex - 1)
setCurrentTrackIndex((prev) => prev - 1)
setCurrentTime(0)
if (playerRef.current) playerRef.current.seekTo(0, true)
}
}
}, [currentTrackIndex])

Comment on lines +64 to 71
Copy link

@coderabbitai coderabbitai bot Sep 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

이전 트랙 이동도 누적 시작 시점으로 seek 필요

prevTrack도 동일하게 누적 시작 시점으로 이동해야 합니다.

-  const prevTrack = useCallback(() => {
-    if (currentTrackIndex > 0) {
-      setCurrentTrackIndex((prev) => prev - 1)
-      setCurrentTime(0)
-      if (playerRef.current) playerRef.current.seekTo(0, true)
-    }
-  }, [currentTrackIndex])
+  const prevTrack = useCallback(() => {
+    if (!currentPlaylist) return
+    if (currentTrackIndex <= 0) return
+    const prevIndex = currentTrackIndex - 1
+    setCurrentTrackIndex(prevIndex)
+    setCurrentTime(0)
+    const trackLengths: number[] =
+      currentPlaylist.songs.map((/* s */) => /* lengthSec */ 0) // TODO: implement
+    if (playerRef.current) {
+      const acc = getAccTime(trackLengths, prevIndex, 0)
+      playerRef.current.seekTo(acc, true)
+    }
+  }, [currentPlaylist, currentTrackIndex])

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents 
In src/app/providers/PlayerProvider.tsx around lines 64–71, prevTrack currently
seeks to 0 when moving to the previous track; it must instead seek to the
cumulative start time for that track. Replace the fixed 0 seek with calculation
of the track's accumulated start (e.g., compute the sum of durations/start
offsets for all tracks before the new index or use the existing helper used by
nextTrack), setCurrentTime to that accumulatedStart, and call
playerRef.current.seekTo(accumulatedStart, true). Also ensure the useCallback
dependency list includes any values used to compute the accumulated start
(tracks or helper).

const updateCurrentTime = (time: number) => {
setCurrentTime(time)
}
const updateCurrentTime = (time: number) => setCurrentTime(time)
Copy link

@gemini-code-assist gemini-code-assist bot Sep 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

updateCurrentTime 함수도 useCallback으로 감싸서 매 렌더링마다 함수가 새로 생성되는 것을 방지하는 것이 좋습니다. 이는 setPlaylist와 마찬가지로 성능 최적화에 도움이 됩니다. 1

Suggested change
const updateCurrentTime = (time: number) => setCurrentTime(time)
const updateCurrentTime = useCallback((time: number) => setCurrentTime(time), [])

Style Guide References

Footnotes

  1. React의 useCallback을 사용하여 불필요한 함수 생성을 방지하고, 이를 통해 자식 컴포넌트의 불필요한 리렌더링을 줄여 성능을 최적화해야 합니다. (link) ↩


const handlePlayerStateChange = useCallback(
(event: YT.OnStateChangeEvent) => {
if (event.data === window.YT.PlayerState.ENDED) nextTrack()
},
[nextTrack]
)

const value = {
currentPlaylist,
Expand All @@ -66,6 +89,8 @@
nextTrack,
prevTrack,
updateCurrentTime,
playerRef,
handlePlayerStateChange,
}

return <PlaylistContext.Provider value={value}>{children}</PlaylistContext.Provider>
Expand All @@ -73,10 +98,8 @@

export default PlaylistProvider

export const usePlaylist = () => {

Check warning on line 101 in src/app/providers/PlayerProvider.tsx

View workflow job for this annotation

GitHub Actions / Build and Lint 

Fast refresh only works when a file only exports components. Use a new file to share constants or functions between components
const context = useContext(PlaylistContext)
if (context === undefined) {
throw new Error('usePlaylist must be used within a PlaylistProvider')
}
if (!context) throw new Error('usePlaylist must be used within a PlaylistProvider')
return context
}
50 changes: 0 additions & 50 deletions src/pages/discover/commentData.json
View file
Open in desktop

This file was deleted.

86 changes: 20 additions & 66 deletions src/pages/discover/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import { useCallback, useEffect, useMemo, useRef, useState } from 'react'
import { useCallback, useEffect, useMemo, useState } from 'react'
import { useParams } from 'react-router-dom'
import type { YouTubeEvent } from 'react-youtube'

import styled from 'styled-components'

Expand All @@ -20,18 +19,19 @@
const DiscoverPage = () => {
const { id: playlistId } = useParams()
const {
setPlaylist,
isPlaying,
currentPlaylist,
currentTrackIndex,
nextTrack,
prevTrack,
currentTime,
isPlaying,
playerRef,
setPlaylist,
play,
pause,
currentTime,
updateCurrentTime,
nextTrack,
prevTrack,
handlePlayerStateChange,
} = usePlaylist()
const playerRef = useRef<YT.Player | null>(null)

const [showCoachmark, setShowCoachmark] = useState(false)
const [isMuted, setIsMuted] = useState<boolean | null>(null)

Expand Down Expand Up @@ -60,7 +60,7 @@
} = useShufflePlaylists()

// shuffleData에서 실제 playlist 배열만 추출
const shufflePlaylists = shuffleData?.pages.flatMap((page) => page.content) ?? []

Check warning on line 63 in src/pages/discover/index.tsx

View workflow job for this annotation

GitHub Actions / Build and Lint 

The 'shufflePlaylists' logical expression could make the dependencies of useMemo Hook (at line 97) change on every render. Move it inside the useMemo callback. Alternatively, wrap the initialization of 'shufflePlaylists' in its own useMemo() Hook

// PlaylistDetailResponse → PlaylistInfo 변환
const playlistAsInfo = useMemo(() => {
Expand Down Expand Up @@ -96,8 +96,6 @@
return [playlistAsInfo, ...shufflePlaylists]
}, [playlistAsInfo, shufflePlaylists])

console.log(playlistsData)

const videoId = currentPlaylist
? getVideoId(currentPlaylist.songs[currentTrackIndex]?.youtubeUrl)
: null
Expand All @@ -109,11 +107,11 @@
if (!currentPlaylist && playlistsData.length > 0 && isReady) {
const initialPlaylist =
playlistsData.find((p) => p.playlistId === Number(playlistId)) || playlistsData[0]

setPlaylist(initialPlaylist, 0, 0)
}
}, [playlistsData, currentPlaylist, playlistId, setPlaylist])
}, [playlistsData, currentPlaylist, playlistId, setPlaylist, isReady])

// 재생, 확인, 조회수 refetch
useEffect(() => {
if (!currentPlaylist || !isPlaying) return
startPlaylist(currentPlaylist.playlistId)
Expand All @@ -135,21 +133,6 @@
}
}, [currentPlaylist, isPlaying, startPlaylist, confirmPlaylist, refetchViewCounts])

// 현재 재생 시간 업데이트
useEffect(() => {
const intervalId = setInterval(() => {
if (playerRef.current) updateCurrentTime(playerRef.current.getCurrentTime())
}, 1000)
return () => clearInterval(intervalId)
}, [updateCurrentTime])

// 재생, 일시정지
useEffect(() => {
if (!playerRef.current) return
if (isPlaying) playerRef.current.playVideo()
else playerRef.current.pauseVideo()
}, [isPlaying])

// 캐러셀 스와이프 시 현재 플레이리스트 업데이트
const handleSelectPlaylist = useCallback(
(index: number) => {
Expand All @@ -162,48 +145,26 @@
fetchNextPage()
}
},
[setPlaylist, currentPlaylist, playlistsData, fetchNextPage, hasNextPage, isFetchingNextPage]
[playlistsData, currentPlaylist, setPlaylist, hasNextPage, isFetchingNextPage, fetchNextPage]
)

const handlePlayerStateChange = useCallback(
(event: YouTubeEvent) => {
if (event.data === window.YT.PlayerState.ENDED) nextTrack()
},
[nextTrack]
)

const handlePlayPause = () => {
if (isPlaying) pause()
else play()
}

const isCurrentlyPlaying = (() => {
if (!window.YT || !playerRef.current) return false
return isPlaying && playerRef.current.getPlayerState() === window.YT.PlayerState.PLAYING
})()

return (
<Page>
{showCoachmark && <DiscoverCoachMark onClick={handleCloseCoachmark} />}
<SwipeCarousel data={playlistsData} onSelectIndexChange={handleSelectPlaylist}>
{playlistsData.map((data) => (
<Slide key={data.playlistId}>
<PlaylistLayout
key={data.playlistId}
data={data}
currentPlaylist={currentPlaylist}
currentTrackIndex={currentTrackIndex}
currentTime={currentTime}
isPlaying={isCurrentlyPlaying}
onPlayPause={handlePlayPause}
isPlaying={isPlaying}
onPlayPause={() => (isPlaying ? pause() : play())}
onNext={nextTrack}
onPrev={prevTrack}
onSelectTrack={(trackIndex, time) => {
if (currentPlaylist) {
setPlaylist(currentPlaylist, trackIndex)
if (time !== undefined) playerRef.current?.seekTo(time, true)
if (!isPlaying) play()
}
if (currentPlaylist) setPlaylist(currentPlaylist, trackIndex, time)
}}
playerRef={playerRef}
isMuted={isMuted}
Expand All @@ -212,22 +173,16 @@
</Slide>
))}
</SwipeCarousel>
{!showCoachmark && videoId && (

{videoId && (
<YoutubePlayer
videoId={videoId}
onReady={(event) => {
playerRef.current = event.target

// 현 상태 참조해서 동기화
playerRef.current?.seekTo(currentTime, true)
if (isPlaying) playerRef.current?.playVideo()
else playerRef.current?.pauseVideo()
setIsMuted(playerRef.current?.isMuted() ?? null)

if (isPlaying) {
playerRef.current?.seekTo(currentTime, true)
playerRef.current?.playVideo()
} else {
playerRef.current?.seekTo(currentTime, true)
playerRef.current?.pauseVideo()
}
}}
onStateChange={handlePlayerStateChange}
/>
Expand All @@ -241,7 +196,6 @@
const Slide = styled.div`
flex: 0 0 100%;
`

const Page = styled.div`
position: relative;
`
Loading
Loading