feat: 접근 제어 구현#67
Merged
Merged
Conversation
📝 WalkthroughWalkthrough비로그인 사용자의 접근 제어를 위해 변경사항
Sequence Diagram(s)sequenceDiagram
actor User as 사용자
participant Browser as 브라우저
participant App as App (Router)
participant PrivateRoute as PrivateRoute
participant AuthStore as AuthStore
participant Header as Header
Browser->>App: 요청 /mypage (또는 보호된 경로)
App->>PrivateRoute: 라우트 검사 시작
PrivateRoute->>AuthStore: hasHydrated?, isAuthenticated? 조회
alt hasHydrated = false
AuthStore-->>PrivateRoute: 아직 초기화 중 → 렌더 지연 (null)
else hasHydrated = true and isAuthenticated = false
PrivateRoute->>Browser: navigate('/', { state: { openLogin: true } })
Browser->>Header: location.state.openLogin 감지
Header->>Header: setLoginOpen(true) 및 history.replace(path, {}) 실행
else isAuthenticated = true
PrivateRoute->>App: Outlet 렌더 (요청 컴포넌트 표시)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
No actionable comments were generated in the recent review. 🎉 🧹 Recent nitpick comments
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@src/App.tsx`:
- Around line 69-94: The StoreRegistrationPage route is outside the PublicLayout
so it renders without the common header; move or wrap the
"/mypage/store/register" route so it is a child of the same PublicLayout used by
the other protected routes (or wrap it in MyPageLayout inside the existing
PrivateRoute block) to ensure the header appears; update the route grouping that
contains PrivateRoute and PublicLayout (references: PrivateRoute, PublicLayout,
StoreRegistrationPage, MyPageLayout) so "/mypage/store/register" is nested with
the other /mypage routes.
In `@src/components/RouteGuards.tsx`:
- Around line 8-12: Remove the blocking browser alert in the useEffect that
watches isAuthenticated; instead rely on the existing Navigate(state={{
openLogin: true }}) flow so Header.tsx opens the login dialog, or replace the
alert with a non-blocking toast if you want an extra hint. Specifically, edit
the useEffect in RouteGuards (the block referencing isAuthenticated and alert)
to delete the alert(...) call and ensure the redirect/Navigate with state={{
openLogin: true }} is preserved so the login dialog in Header.tsx is the single
source of truth for prompting authentication.
- Around line 5-6: PrivateRoute reads isAuthenticated before zustand persist
hydration, causing false redirects; add a hasHydrated flag to the auth store and
wait for it in PrivateRoute. In useAuthStore (the persist config), add
hasHydrated to AuthState default (false), exclude it from partialize, and set
state.hasHydrated = true in onRehydrateStorage; in RouteGuards.tsx read
hasHydrated via useAuthStore((s) => s.hasHydrated) alongside isAuthenticated and
return null (or a loader) while hasHydrated is false, then run the existing
alert/Navigate logic once hydrated. Ensure references: PrivateRoute,
useAuthStore, hasHydrated, onRehydrateStorage, and persist.
🧹 Nitpick comments (3)
src/components/restaurant/RestaurantDetailModal.tsx (2)
71-79: 인증 체크 로직이PrivateRoute와 중복돼요
alert("로그인이 필요한 서비스입니다.")+nav("/", { state: { openLogin: true } })패턴이RouteGuards.tsx와 동일하게 반복되고 있어요. 여기서도 마찬가지로 alert와 로그인 다이얼로그가 이중으로 표시됩니다.공통 로직을 커스텀 훅으로 추출하면 일관성 유지가 쉬워지고, 나중에 alert를 토스트로 교체할 때도 한 곳만 수정하면 돼요.
♻️ 커스텀 훅 추출 예시
// src/hooks/useRequireAuth.ts import { useNavigate } from "react-router-dom"; import { useAuthStore } from "@/stores/useAuthStore"; export function useRequireAuth() { const nav = useNavigate(); const isAuthenticated = useAuthStore((state) => state.isAuthenticated); const requireAuth = (onAuthenticated: () => void) => { if (!isAuthenticated) { // toast 등으로 교체 가능 nav("/", { state: { openLogin: true } }); return; } onAuthenticated(); }; return { isAuthenticated, requireAuth }; }사용 예:
const { requireAuth } = useRequireAuth(); const handleReserveClick = () => { requireAuth(() => { onOpenChange(false); // 필요 시 modal close 등 전처리 onClickReserve(); }); };
68-69: 모달 컴포넌트에 네비게이션/스토어 의존성 추가에 대해
RestaurantDetailModal은 원래 순수하게 props 기반으로 동작하던 프레젠테이셔널 컴포넌트인데, 이제useNavigate와useAuthStore를 직접 사용하게 되었어요. 단일 책임 원칙 측면에서 보면, 인증 게이팅 로직은 이 모달을 호출하는 부모 컴포넌트나 위에서 제안한 훅에서 처리하는 게 더 깔끔합니다.당장은 동작에 문제 없으니 이번 PR에서 꼭 변경할 필요는 없지만, 향후 리팩터링 시 고려해 주세요.
As per coding guidelines,
src/components/**: "컴포넌트는 단일 책임, props 타입/네이밍 명확히, 접근성(aria) 체크."src/components/main/Header.tsx (1)
28-37:useEffect의존성 배열 최적화 가능현재
[location, nav]로 되어 있어서,location객체는 모든 라우트 변경마다 새 참조가 생기기 때문에 이 effect가 매번 재실행돼요. 기능적으로는location.state?.openLogin조건 분기 덕분에 문제없지만, 불필요한 실행을 줄이려면 의존성을 좁힐 수 있어요.💡 개선 예시
+ const openLogin = location.state?.openLogin; + useEffect(() => { - if (location.state?.openLogin) { + if (openLogin) { const timer = setTimeout(() => { setLoginOpen(true); nav(location.pathname, { replace: true, state: {} }); }, 0); return () => clearTimeout(timer); } - }, [location, nav]); + }, [openLogin, location.pathname, nav]);
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/stores/useAuthStore.ts`:
- Around line 55-57: The onRehydrateStorage handler currently mutates state
directly (state!.hasHydrated = true) which won't notify subscribers and can
crash if state is undefined; change it to call the store's setter instead (use
the provided set(...) or useAuthStore.setState(...)) and guard against undefined
rehydrate payload (only set hasHydrated when state param exists), e.g. use the
onRehydrateStorage callback to invoke set({ hasHydrated: true }) or
useAuthStore.setState({ hasHydrated: true }) so subscribers (e.g., PrivateRoute)
receive updates safely without non-null assertions.
🧹 Nitpick comments (1)
src/stores/useAuthStore.ts (1)
62-66:hasHydrated전용 selector hook이 없어요다른 상태값들(
useAuthToken,useIsAuthenticated,useUserId)은 편의 hook이 있는데hasHydrated만 빠져있네요. PrivateRoute 등에서 직접useAuthStore(s => s.hasHydrated)쓰면 동작은 하지만 일관성 차원에서 추가해두면 좋을 것 같아요.✨ 제안
export const useUserId = () => useAuthStore((s) => s.userId); +export const useHasHydrated = () => useAuthStore((s) => s.hasHydrated);
jjjsun
approved these changes
Feb 11, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
💡 개요
접근 제어 구현
🔢 관련 이슈 링크
💻 작업내용
📌 변경사항PR
🤔 추가 논의하고 싶은 내용
✅ 체크리스트
Summary by CodeRabbit