feat: 로그아웃 시 query clear 추가 - #83
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAppProviders에서 QueryClient를 전역으로 등록합니다. 인증 상태를 해제하면 진행 중인 쿼리를 취소하고 전체 캐시를 비웁니다. ProfileMenuTrigger는 알림 전용 캐시 삭제 대신 인증 처리와 경로 이동을 수행합니다. Changes인증 및 쿼리 캐시 정리
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant ProfileMenuTrigger
participant useAuthStore
participant appQueryClient
participant QueryClient
ProfileMenuTrigger->>useAuthStore: clearSession()
useAuthStore->>appQueryClient: clearAppQueryCache()
appQueryClient->>QueryClient: cancelQueries()
appQueryClient->>QueryClient: clear()
ProfileMenuTrigger->>ProfileMenuTrigger: 인증 상태 처리 및 경로 이동
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/stores/useAuthStore.ts (1)
154-160: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win모든 비인증 전환에서 캐시를 정리해 주세요.
Line 159는
clearSession경로만 처리합니다. 그러나checkAuth는useAuthStore.tsLine 229-230과 Line 282에서markUnauthenticated를 호출합니다.markUnauthenticated는 Line 163-165에서 상태만 변경하므로 이전 사용자의 cache가 남을 수 있습니다.비인증 전환을 공통 helper로 통합하거나
markUnauthenticated에서도clearAppQueryCache()를 호출하세요. 토큰을 제거한 뒤markUnauthenticated를 호출하고, 다른 사용자로 로그인했을 때 이전 query data가 없는지 테스트하세요.제안
markUnauthenticated: () => { + clearAppQueryCache(); set({ ...UNAUTHENTICATED_STATE }); },🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/stores/useAuthStore.ts` around lines 154 - 160, Ensure every unauthenticated transition clears application query data, not only clearSession. Update markUnauthenticated and the checkAuth call paths to reuse clearAppQueryCache after removing tokens, while preserving the existing unauthenticated state update; verify a subsequent login cannot observe the previous user’s query data.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/lib/query/appQueryClient.ts`:
- Around line 3-12: Move the module-level appQueryClient state and the
setAppQueryClient and clearAppQueryCache side effects out of src/lib/query into
the application boundary under src/providers or src/stores. Keep src/lib/query
limited to pure query-related lookup and transformation functions, and update
callers to use the relocated registration and cache-clearing APIs.
In `@src/providers/AppProviders.tsx`:
- Around line 30-32: Update AppProviders so setAppQueryClient(queryClient) runs
synchronously during render rather than in useEffect, ensuring AuthProvider’s
checkAuth and clearSession always see the registered client. Preserve the
existing queryClient instance and remove the effect-based registration.
---
Outside diff comments:
In `@src/stores/useAuthStore.ts`:
- Around line 154-160: Ensure every unauthenticated transition clears
application query data, not only clearSession. Update markUnauthenticated and
the checkAuth call paths to reuse clearAppQueryCache after removing tokens,
while preserving the existing unauthenticated state update; verify a subsequent
login cannot observe the previous user’s query data.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: eb5e19fb-5739-456f-a353-d5ef24c2ab44
📒 Files selected for processing (4)
src/components/common/Header/ProfileMenuTrigger.tsxsrc/lib/query/appQueryClient.tssrc/providers/AppProviders.tsxsrc/stores/useAuthStore.ts
💤 Files with no reviewable changes (1)
- src/components/common/Header/ProfileMenuTrigger.tsx
juengseulki
left a comment
There was a problem hiding this comment.
📋 PR 리뷰
👍 좋았던 점
- 로그아웃 시 알림 Query 하나만 제거하던 구조에서 전체 Query Cache를 초기화하도록 확장했습니다.
- 다른 사용자로 다시 로그인했을 때 이전 사용자의 견적, 찜, 알림 등 사용자 종속 데이터가 남는 문제를 예방할 수 있습니다.
- React 컴포넌트 외부의 Auth Store에서도 동일한 QueryClient를 사용할 수 있도록 Provider에서 실제 인스턴스를 등록했습니다.
- 별도의 QueryClient를 새로 만들지 않고
QueryClientProvider의 인스턴스를 재사용한 점이 좋았습니다. - 캐시 삭제를 특정 로그아웃 버튼이 아니라
markUnauthenticated()에 집중시켜 인증 해제 경로 전체에 적용했습니다. clearSession()도markUnauthenticated()를 재사용하게 해 인증 상태 초기화와 Cache 삭제가 분리되지 않도록 했습니다.- ProfileMenu에 있던 Notification 전용
removeQueries()를 제거해 중복 책임을 정리했습니다. - 현재 PR은 열려 있고 mergeable 상태이며 변경 파일은 4개입니다.
🔍 확인 및 제안
전체 방향은 적절합니다.
다만 AppQueryClientRegistrar에서 setAppQueryClient(queryClient)를
렌더 함수 안에서 바로 실행하고 있는 부분은 한 번 정리하는 것을 권장드립니다.
전역 모듈 변수를 변경하는 작업은 side effect이므로
가능하면 useEffect에서 수행하는 편이 React 렌더 규칙에 더 잘 맞습니다.
현재도 동일한 QueryClient 인스턴스를 반복해서 대입하는 구조라
실제 기능 오류로 이어질 가능성은 낮지만,
React Strict Mode나 향후 렌더 흐름 변경까지 고려하면 Effect 기반 등록이 더 명확합니다.
또한 queryClient.clear()는 Query뿐 아니라 Mutation Cache까지 함께 비우기 때문에
로그아웃 시 사용자 범위 데이터를 완전히 초기화하려는 목적에는 잘 맞습니다.
현재 구현에서는 인증 해제 시 캐시가 전체 삭제되므로
기존 ProfileMenu의 Notification 전용 제거 로직을 없앤 것도 적절합니다.
렌더 중 전역 등록 부분만 정리한다면 더 깔끔한 구조가 될 것 같습니다.
To Reviewer 내용 기준으로 QueryClient 등록과 로그아웃 시 Cache clear 흐름을 중점적으로 확인했습니다!
현재 AppProviders에서 생성한 실제 QueryClient 인스턴스를
setAppQueryClient()로 등록하고,
Zustand Auth Store에서는 해당 인스턴스를 통해 clear()하는 구조라
서로 다른 QueryClient를 비우는 문제는 없어 보입니다.
또한 Cache 삭제를 ProfileMenuTrigger 같은 특정 UI에 두지 않고
markUnauthenticated()에 배치한 점이 좋습니다.
이 구조에서는
- 명시적 로그아웃
clearSession()- 인증 만료 등
markUnauthenticated()를 사용하는 경로
모두 동일하게 Query Cache 정리를 수행할 수 있습니다.
기존 ProfileMenu의 Notification 전용 removeQueries()를 제거한 것도
전체 Cache clear로 대체되므로 적절합니다.
다만 한 가지 확인하고 싶은 부분은
AppQueryClientRegistrar에서 setAppQueryClient(queryClient)를
렌더 중에 직접 호출하고 있다는 점입니다.
동일 인스턴스를 등록하는 것이기 때문에 현재 기능에는 큰 문제가 없어 보이지만,
전역 모듈 상태 변경은 렌더 side effect에 해당하므로
useEffect 안에서 QueryClient를 등록하는 편이 더 안전하고 의도도 명확해 보입니다.
이 부분 외에는 로그아웃 시 사용자 Query Cache를 초기화하는 흐름 자체는 적절하게 구성된 것으로 확인했습니다.
…into fix/auth-login-fix
…into fix/auth-login-fix
📋 작업 내용
🔥 변경 사항
appQueryClient.ts를 작성하였습니다.setAppQueryClient는AppProvider에서queryClient인스턴스를 set 합니다.clearAppQueryCache는 query cache 를 비웁니다.markUnauthenticated에clearAppQueryCache를 붙이고, 해당 부분을clearSession에서 가져와서 사용합니다.ProfileMenuTrigger에서 쓰던removeQueries를 삭제하였습니다. (logout 에서 해당 과정 이뤄짐)✅ 체크리스트
💬 To Reviewer
Summary by CodeRabbit