-
Notifications
You must be signed in to change notification settings - Fork 0
[Fix] 로그인/로그아웃 즉시 AuthProvider의 accessToken 상태가 업데이트 되도록 수정 #253
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.
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
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Walkthrough세 개의 인증 관련 훅에서 useAuth 프로바이더를 도입하여 토큰 관리 기능을 추가했습니다. 로그인 성공 시 accessToken을 설정하고, 로그아웃 시 제거하며, 알림 조회 쿼리를 토큰 기반으로 제어합니다. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
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 |
📊 Coverage Report
📉 #253을 main에 병합하면 coverage가 Coverage 요약@@ Coverage Diff @@
## main #253 +/- ##
===========================================
- Coverage 37.86% 37.78% -0.08%
===========================================
Files 186 186 0
Lines 7966 7983 +17
Branches 361 361 0
===========================================
Hits 3016 3016 0
+ Misses 4950 4967 +17 영향받은 파일✅ 이 PR로 영향받은 파일이 없습니다 수정된 모든 파일이 현재 coverage를 유지했습니다. |
🎭 Playwright Report✨ E2E Test가 성공적으로 완료되었습니다. Test 요약 내용을 확인해주세요.
📊 Test Summary
📜 Test Details✅ Passed Tests (3)
|
🎨 Storybook Report✅ 변경 사항이 없습니다 모든 Story가 이전 빌드와 동일합니다.
|
🚀 PR Preview Report✨ Build가 성공적으로 완료되었습니다. Preview에서 변경사항을 확인하세요.
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/hooks/use-auth/use-auth-logout/index.ts (1)
17-29: 토큰 제거 시점을 finally 블록으로 이동 권장현재
accessToken.remove()가 try 블록에 위치하여, 로그아웃 API 호출이 실패하면 토큰이 제거되지 않습니다. 하지만 finally 블록에서는 API 성공 여부와 관계없이 캐시를 정리하고 홈으로 이동합니다.이로 인해 다음과 같은 불일치 상태가 발생할 수 있습니다:
- 로그아웃 API 실패 → 토큰은 유지 → 캐시는 정리됨 → 사용자는 홈으로 이동
- 결과: 토큰은 있지만 캐시는 비어있고 홈 페이지에 있는 상태
토큰 제거를 finally 블록으로 이동하여 캐시 정리 및 네비게이션과 동일한 시점에 실행되도록 하는 것이 일관성 측면에서 더 안전합니다.
🔎 제안하는 수정
const handleLogout = async () => { try { await API.authService.logout(); - accessToken.remove(); } catch (error) { console.error('[LOGOUT ERROR]', error); } finally { + accessToken.remove(); // 로그인 유저 관련 캐시 정리 queryClient.removeQueries({ queryKey: userKeys.all }); router.push('/'); } };src/hooks/use-auth/use-auth-login/index.ts (1)
59-77: 토큰 설정 시점을 네비게이션 이전으로 이동 고려현재
accessToken.set()이router.replace()이후에 호출되어, 페이지 이동 중 또는 이동 직후에 토큰이 설정됩니다.PR 목표에서 "로그인 직후 즉시 미확인 알림 수가 표시되고 SSE가 연결된다"고 명시되어 있으나, 토큰이 네비게이션 이후에 설정되면 다음과 같은 타이밍 이슈가 발생할 수 있습니다:
- 이동된 페이지의 컴포넌트들이 마운트되는 시점에 토큰이 아직 설정되지 않음
- 알림 카운트 조회나 SSE 연결이 지연되거나 초기 렌더링에서 실행되지 않을 수 있음
토큰 설정을 네비게이션 이전으로 이동하면 이동된 페이지에서 즉시 인증 상태를 사용할 수 있습니다.
🔎 제안하는 수정
Cookies.set('userId', String(result.user.userId), { path: '/', sameSite: 'lax', secure: process.env.NODE_ENV === 'production', }); + accessToken.set(result.accessToken); formApi.reset(); const nextPath = normalizePath(searchParams.get('path')); router.replace(nextPath); - accessToken.set(result.accessToken); } catch (error) {
🧹 Nitpick comments (1)
src/hooks/use-notification/use-notification-get-unread-count/index.ts (1)
16-21: 타입 안정성 개선 제안
finalData의 타입이0 | number | undefined로 혼합되어 있습니다.queryResult.data는 로딩 중일 때undefined일 수 있으므로, 더 명확한 폴백 처리를 권장합니다.🔎 제안하는 수정
- const finalData = accessToken.value ? queryResult.data : 0; + const finalData = accessToken.value ? (queryResult.data ?? 0) : 0; return { ...queryResult, data: finalData, };이렇게 하면 토큰이 있지만 데이터가 아직 로드되지 않은 경우에도
0을 반환하여 일관된 타입을 유지할 수 있습니다.
📜 Review details
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/hooks/use-auth/use-auth-login/index.tssrc/hooks/use-auth/use-auth-logout/index.tssrc/hooks/use-notification/use-notification-get-unread-count/index.ts
🧰 Additional context used
🧬 Code graph analysis (2)
src/hooks/use-notification/use-notification-get-unread-count/index.ts (3)
src/hooks/use-notification/index.ts (1)
useGetNotificationUnreadCount(2-2)src/lib/query-key/query-key-notification/index.ts (1)
notificationKeys(3-11)src/api/index.ts (1)
API(25-25)
src/hooks/use-auth/use-auth-logout/index.ts (2)
src/hooks/use-auth/index.ts (1)
useLogout(2-2)src/api/index.ts (1)
API(25-25)
🔇 Additional comments (1)
src/hooks/use-notification/use-notification-get-unread-count/index.ts (1)
12-12: retry: false 추가 의도 확인 필요
retry: false옵션이 추가되었으나 AI 요약이나 PR 설명에 언급되지 않았습니다. 이 변경이 의도적인지 확인이 필요합니다.인증되지 않은 상태에서 알림 카운트 조회 실패 시 재시도를 방지하려는 의도로 보이지만, 네트워크 일시적 오류 시에도 재시도가 되지 않아 사용자 경험이 저하될 수 있습니다.
만약 의도하지 않은 변경이라면 제거를 권장합니다:
🔎 제안하는 수정
const queryResult = useQuery({ queryKey: notificationKeys.unReadCount(), queryFn: () => API.notificationService.getUnreadCount(), - retry: false, enabled: !!accessToken.value, });
…into chiyoung-fix/notification-provider
📝 변경 사항
💡 로그인 직후
💡 로그아웃 직후
❌ 현 문제점
LoginForm에서 모든 로직을 useLogin 훅에 이관했기 때문에
useLogin 훅은 비즈니스 로직, 도메인로직을 모두 포함하고 있습니다.
AuthProvider의 accessToken 상태도 useLogin 훅에서 업데이트 해야하기 때문에 로직 복잡도가 상승합니다.
useLogin 훅에서는 도메인 로직(API 호출, 쿠키설정) 만 담당해야 합니다.
🔗 관련 이슈
Closes #
🧪 테스트 방법
📸 스크린샷 (선택)
📋 체크리스트
💬 추가 코멘트
CodeRabbit Review는 자동으로 실행되지 않습니다.
Review를 실행하려면 comment에 아래와 같이 작성해주세요
Summary by CodeRabbit
릴리즈 노트
✏️ Tip: You can customize this high-level summary in your review settings.