-
Notifications
You must be signed in to change notification settings - Fork 2
Refactor(client): 라우팅 구조 개선 및 접근 제어 로직 리팩토링 #211
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
base: develop
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| export { default as LoginCallbackPage } from './ui/login-callback-page'; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,29 +5,32 @@ import { createBrowserRouter } from 'react-router'; | |
|
|
||
| import { NotFoundPage } from '@pages/not-found'; | ||
|
|
||
| import AuthGuard from './auth-guard'; | ||
| import { privateRoutes } from './routes/private-route'; | ||
| import { publicRoutes } from './routes/public-route'; | ||
| import { PrivateRouteGuard, PublicRouteGuard } from './auth-guard'; | ||
| import { routes } from './routes'; | ||
|
|
||
| const PrivateLayoutWithGuard = () => { | ||
| return ( | ||
| <AuthGuard> | ||
| <PrivateLayout /> | ||
| </AuthGuard> | ||
| ); | ||
| }; | ||
| const GuardedPublicLayout = () => ( | ||
| <PublicRouteGuard> | ||
| <PublicLayout /> | ||
| </PublicRouteGuard> | ||
| ); | ||
|
|
||
| const GuardedPrivateLayout = () => ( | ||
| <PrivateRouteGuard> | ||
| <PrivateLayout /> | ||
| </PrivateRouteGuard> | ||
| ); | ||
|
|
||
| export const router = createBrowserRouter([ | ||
| { | ||
| Component: GlobalLayout, | ||
| children: [ | ||
| { | ||
| Component: PublicLayout, | ||
| children: publicRoutes, | ||
| Component: GuardedPublicLayout, | ||
| children: routes.filter((r) => !r.auth), | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 이런식으로 filter 함수를 사용하면 가독성이 떨어질 것 같아요 ! routes 파일을 다시 분리하게 된다면 해결될 문제 같아요 ! |
||
| }, | ||
| { | ||
| Component: PrivateLayoutWithGuard, | ||
| children: privateRoutes, | ||
| Component: GuardedPrivateLayout, | ||
| children: routes.filter((r) => r.auth), | ||
| }, | ||
| { | ||
| path: '*', | ||
|
|
||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 저는 publicRoutes, privateRoutes로 라우트를 분리하는 방식이 유지보수성과 가독성 측면에서 더 낫다고 생각해요 !! 인증 필요/불필요라는 기준이 명확한 도메인 규칙이기 때문에 오히려 지금처럼 라우트를 한 배열에 섞어두면 규모가 커질수록 분류 비용과 실수 가능성이 크다고 생각합니다 ! 특히 새로운 페이지 추가 시에 이 페이지가 인증이 필요한가?를 판단하고 그 결과에 따라 private/public 중 한 곳에만 넣으면 되기 때문에 변경 범위가 작고 리뷰 포인트도 명확해진다고 생각하는데 지연님의 생각은 어떨까요 ? 또한 SSoT 위배 라고 말씀해주셨는데 분리 자체는 중복 정의가 아니라 책임 분리에 가깝다고 생각해요 ! |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,54 @@ | ||
| import { RouteObject } from 'react-router'; | ||
|
|
||
| import { LandingPage } from '@pages/landing'; | ||
| import { LoginCallbackPage } from '@pages/login-callback'; | ||
|
|
||
| import { | ||
| AiResultsPage, | ||
| AllMemoPage, | ||
| LabelPage, | ||
| LoginPage, | ||
| NewMemoPage, | ||
| } from '../lazy'; | ||
| import { PATH } from '../path'; | ||
|
|
||
| type AuthRouteObject = RouteObject & { | ||
| auth?: boolean; | ||
| }; | ||
|
|
||
| export const routes: AuthRouteObject[] = [ | ||
| // Private Routes | ||
| { | ||
| path: PATH.NEW_MEMO, | ||
| Component: NewMemoPage, | ||
| auth: true, | ||
| }, | ||
| { | ||
| path: PATH.ALL_MEMO, | ||
| Component: AllMemoPage, | ||
| auth: true, | ||
| }, | ||
| { | ||
| path: PATH.AI_RESULTS, | ||
| Component: AiResultsPage, | ||
| auth: true, | ||
| }, | ||
| { | ||
| path: PATH.LABEL, | ||
| Component: LabelPage, | ||
| auth: true, | ||
| }, | ||
| // Public Routes | ||
| { | ||
| path: PATH.LOGIN, | ||
| Component: LoginPage, | ||
| }, | ||
| { | ||
| path: PATH.LOGIN_CALLBACK, | ||
| Component: LoginCallbackPage, | ||
| }, | ||
| { | ||
| path: PATH.LANDING, | ||
| Component: LandingPage, | ||
| }, | ||
| ]; |
This file was deleted.
This file was deleted.
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.
PublicRouteGuard를 PublicLayout 전체에 감싸면
PublicLayout 아래에 있는 모든 라우트가 public 영역이 되고 있기 때문에
현재 pathname이 publicOnlyPaths에 해당하나를 검사할 이유가 없을 것 같아요 !!