20260102 #121 토스트 전역 핸들링을 위한 커스텀 훅 use toast 추가#125
Conversation
|
Caution Review failedThe pull request is closed. Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. Walkthrough버전 정보(CHANGELOG, package.json, 상수)가 0.4.1로 업데이트되었고, 중앙화된 Changes
Sequence Diagram(s)sequenceDiagram
participant UI as MenuPageContainer
participant Hook as useToast
participant Toast as CartToast
Note over UI,Hook: 사용자 액션: 장바구니에 추가
UI->>Hook: showToast("장바구니에 쏙 담았어요!")
alt 기존 토스트 표시 중
Hook->>Hook: hide current toast (clear timers)
Hook-->>Hook: wait animationDuration
end
Hook->>Hook: set message, visible = true
Hook->>Toast: render with visible/message
Hook->>Hook: schedule auto-hide timer (duration)
Note over Hook,Toast: auto-hide 또는 추가 showToast로 대체 가능
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Possibly related PRs
Suggested labels
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
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.
Pull request overview
This PR adds a custom useToast hook to centralize toast message handling across the application, replacing manual state management in components. The version is bumped from 0.4.0 to 0.4.1.
- Introduces a reusable
useToasthook with timer conflict prevention and toast replacement logic - Refactors
MenuPageContainerto use the new hook, eliminating duplicate toast state management code - Updates application version to 0.4.1 across package files and changelog
Reviewed changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/shared/hooks/useToast.ts | New custom hook for managing toast visibility, messages, and timer lifecycle |
| src/features/menu/components/MenuPageContainer.tsx | Refactored to use the new useToast hook instead of manual state management |
| src/constants/version.ts | Version bumped to 0.4.1 |
| package.json | Version bumped to 0.4.1 |
| package-lock.json | Version bumped to 0.4.1 |
| CHANGELOG.md | Added entry for version 0.4.1 |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const showToast = useCallback((message: string) => { | ||
| // 기존 타이머 정리 | ||
| if (timerRef.current) { | ||
| clearTimeout(timerRef.current); | ||
| } | ||
| if (hideTimerRef.current) { | ||
| clearTimeout(hideTimerRef.current); | ||
| } | ||
|
|
||
| // 이미 토스트가 표시중이면 먼저 제거 | ||
| if (visibleRef.current) { | ||
| // 기존 토스트 숨김 | ||
| setVisible(false); | ||
| visibleRef.current = false; | ||
|
|
||
| // 애니메이션 완료 대기 | ||
| hideTimerRef.current = setTimeout(() => { | ||
| setMessage(message); | ||
| setVisible(true); | ||
| visibleRef.current = true; | ||
|
|
||
| // duration 후 토스트 숨김 | ||
| timerRef.current = setTimeout(() => { | ||
| setVisible(false); | ||
| visibleRef.current = false; | ||
| timerRef.current = null; | ||
| }, duration); | ||
|
|
||
| hideTimerRef.current = null; | ||
| }, animationDuration); | ||
| } else { | ||
| // 기존 토스트가 없는 경우 | ||
| setMessage(message); | ||
| setVisible(true); | ||
| visibleRef.current = true; | ||
|
|
||
| timerRef.current = setTimeout(() => { | ||
| setVisible(false); | ||
| visibleRef.current = false; | ||
| timerRef.current = null; | ||
| }, duration); | ||
| } | ||
| }, [duration, animationDuration]); |
There was a problem hiding this comment.
The showToast callback function depends on duration and animationDuration, which means it will be recreated whenever these values change. However, since duration and animationDuration are hook parameters that could change between renders, this could cause unnecessary recreations of the callback. Consider removing duration and animationDuration from the dependency array and using refs instead, or add a comment explaining why they should trigger callback recreation.
| if (visibleRef.current) { | ||
| // 기존 토스트 숨김 | ||
| setVisible(false); | ||
| visibleRef.current = false; | ||
|
|
||
| // 애니메이션 완료 대기 | ||
| hideTimerRef.current = setTimeout(() => { | ||
| setMessage(message); | ||
| setVisible(true); | ||
| visibleRef.current = true; | ||
|
|
||
| // duration 후 토스트 숨김 | ||
| timerRef.current = setTimeout(() => { | ||
| setVisible(false); | ||
| visibleRef.current = false; | ||
| timerRef.current = null; | ||
| }, duration); | ||
|
|
||
| hideTimerRef.current = null; | ||
| }, animationDuration); | ||
| } else { | ||
| // 기존 토스트가 없는 경우 | ||
| setMessage(message); | ||
| setVisible(true); | ||
| visibleRef.current = true; | ||
|
|
||
| timerRef.current = setTimeout(() => { | ||
| setVisible(false); | ||
| visibleRef.current = false; | ||
| timerRef.current = null; | ||
| }, duration); | ||
| } |
There was a problem hiding this comment.
There's a potential race condition between the visible state and visibleRef. When showToast is called while a toast is already visible, the code sets both setVisible(false) and visibleRef.current = false at lines 38-39, then schedules state updates in a setTimeout. If showToast is called again during the animation delay (before line 42's setTimeout executes), the visibleRef check at line 36 will be false (because it was set to false at line 39), causing the else branch to execute and potentially creating a race condition with the pending setTimeout. Consider using a more robust state machine or queueing mechanism to handle rapid consecutive calls.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/shared/hooks/useToast.ts (1)
20-24: 이중 상태 관리 패턴 개선 고려현재
visible상태를useState와useRef두 가지 방식으로 관리하고 있습니다.visibleRef는 동기적 확인을 위해 사용되는 것으로 보이나, 이러한 패턴은 상태 동기화 문제를 일으킬 수 있습니다.대부분의 경우
useState만으로도 충분하며, 타이머 내부에서 최신 상태를 참조해야 한다면 함수형 업데이트setVisible(prev => ...)패턴을 사용하는 것이 더 안전합니다. 현재 구현이 작동하긴 하지만, 향후 유지보수성을 위해 단일 상태 관리 방식으로 리팩토링을 고려해보세요.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (5)
CHANGELOG.mdpackage.jsonsrc/constants/version.tssrc/features/menu/components/MenuPageContainer.tsxsrc/shared/hooks/useToast.ts
🧰 Additional context used
🧬 Code graph analysis (1)
src/features/menu/components/MenuPageContainer.tsx (1)
src/shared/hooks/useToast.ts (1)
useToast(16-83)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Agent
🔇 Additional comments (7)
src/constants/version.ts (1)
1-1: 버전 업데이트 확인버전이 0.4.0에서 0.4.1로 정상적으로 업데이트되었습니다. package.json과 일관성이 유지되고 있습니다.
package.json (1)
3-3: 버전 업데이트 확인package.json의 버전이 0.4.1로 정상적으로 업데이트되었습니다.
src/shared/hooks/useToast.ts (2)
26-68: 토스트 교체 로직 검증 필요토스트 교체 로직이 복잡하게 구현되어 있습니다. 특히
showToast가 빠르게 연속으로 호출될 경우(예: 사용자가 버튼을 빠르게 여러 번 클릭) 다음과 같은 동작을 확인해주세요:
- 애니메이션 중간에 새로운 토스트가 요청되었을 때 예상대로 동작하는지
animationDuration대기 중에 또 다른showToast가 호출되면 타이머가 올바르게 정리되는지수동 테스트로 확인하시거나, 다음과 같이 연속 호출 시나리오를 테스트해보세요:
// 테스트 시나리오 showToast("첫 번째 메시지"); setTimeout(() => showToast("두 번째 메시지"), 100); setTimeout(() => showToast("세 번째 메시지"), 150);
71-80: 클린업 로직 확인컴포넌트 언마운트 시 타이머를 정리하는 로직이 올바르게 구현되었습니다.
src/features/menu/components/MenuPageContainer.tsx (3)
13-13: useToast 훅 임포트 확인새로운
useToast훅을 올바르게 임포트했습니다.
30-30: useToast 훅 사용 확인로컬 토스트 상태 관리에서 중앙화된
useToast훅으로 성공적으로 리팩토링되었습니다. 구조 분해 할당과 프로퍼티 이름 변경(visible: toastVisible,message: toastMessage)도 깔끔하게 처리되었습니다.
47-56: 장바구니 추가 핸들러 개선 확인
showToast를 사용하여 성공 및 에러 메시지를 표시하도록 깔끔하게 리팩토링되었습니다. 기존의 로컬 상태 관리와 수동 타이머 처리가 제거되어 코드가 더 간결해졌습니다.
✨ 변경 사항
✅ 테스트
Summary by CodeRabbit
신기능
리팩터링
버그 수정
Chores
✏️ Tip: You can customize this high-level summary in your review settings.