Skip to content
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

refactor: 로그인 상태 확인 전 페이지 이동 문제 수정 #286

Open
wants to merge 7 commits into
base: dev
Choose a base branch
from
9 changes: 6 additions & 3 deletions src/App.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import BalanceGamePage from './pages/BalanceGamePage/BalanceGamePage';
import BalanceGameMobilePage from './pages/mobile/BalanceGameMobilePage/BalanceGameMobilePage';
import BalanceGameCreationPage from './pages/BalanceGameCreationPage/BalanceGameCreationPage';
import { useNewSelector } from './store';
import { selectAccessToken } from './store/auth';
import { selectAccessToken, selectIsRefreshing } from './store/auth';
import useIsMobile from './hooks/common/useIsMobile';
// import NotAuthRoutes from './components/Routes/NotAuthRoutes';
// import { useMemberQuery } from './hooks/api/member/useMemberQuery';
Expand All @@ -49,7 +49,8 @@ const App: React.FC = () => {
const navigate = useNavigate();

const isMobile = useIsMobile();
const isLoggedIn = !!useNewSelector(selectAccessToken);
const accessToken = useNewSelector(selectAccessToken);
const isTokenRefreshing = useNewSelector(selectIsRefreshing);

useEffect(() => {
const searchParams = new URLSearchParams(location.search);
Expand All @@ -61,6 +62,8 @@ const App: React.FC = () => {
}, [location.search, navigate]);
useTokenRefresh();

if (isTokenRefreshing) return null;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

혹시 이 부분에서 null이 반환된 이후에는 수동으로 새로고침을 해줘야하나요? 아니면 자동으로 리렌더링이 발생하나요?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

로컬에서 테스트해본 결과 페이지가 정상적으로 잘 떠서 로그인 후에 새로고침해도 자동 리렌더링될 것 같아요!!


return (
<LocalizationProvider dateAdapter={AdapterDayjs} adapterLocale="ko">
<Routes>
Expand Down Expand Up @@ -93,7 +96,7 @@ const App: React.FC = () => {
<Route path={PATH.SEARCH.GAME} element={<SearchGamePage />} />
</Route>

<Route element={<ProtectedRoutes isLoggedIn={isLoggedIn} />}>
<Route element={<ProtectedRoutes token={accessToken} />}>
<Route path={PATH.MYPAGE} element={<LayoutNoFooter />}>
<Route index element={<MyPage />} />
</Route>
Expand Down
9 changes: 3 additions & 6 deletions src/components/Routes/ProtectedRoutes.tsx
Original file line number Diff line number Diff line change
@@ -1,16 +1,13 @@
import React from 'react';
import { PATH } from '@/constants/path';
import { useTokenRefresh } from '@/hooks/common/useTokenRefresh';
import { Navigate, Outlet } from 'react-router-dom';

type Props = {
isLoggedIn: boolean;
token: string | undefined;
};

const ProtectedRoutes = ({ isLoggedIn }: Props) => {
useTokenRefresh();

return isLoggedIn ? <Outlet /> : <Navigate to={PATH.LOGIN} />;
const ProtectedRoutes = ({ token }: Props) => {
return token ? <Outlet /> : <Navigate to={PATH.LOGIN} />;
Comment on lines +9 to +10
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

토큰 값을 가지고 분기를 나눈게 좋은 처리인거 같습니다!

};

export default ProtectedRoutes;
9 changes: 7 additions & 2 deletions src/hooks/common/useTokenRefresh.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
/* eslint-disable no-console */
import { useQueryClient } from '@tanstack/react-query';
import { getRefreshToken } from '@/api/interceptor';
import { useNewDispatch, useNewSelector } from '@/store';
import { selectAccessToken, tokenActions } from '@/store/auth';
import { useQueryClient } from '@tanstack/react-query';
import { useEffect } from 'react';
import { useNavigate } from 'react-router-dom';

Expand All @@ -15,7 +15,10 @@ export const useTokenRefresh = () => {

useEffect(() => {
const tokenRefresh = async () => {
if (accessToken) return;
if (accessToken) {
dispatch(tokenActions.setRefreshing(false));
return;
}

try {
const newAccessToken = await getRefreshToken();
Expand All @@ -26,6 +29,8 @@ export const useTokenRefresh = () => {
});
} catch (error) {
dispatch(tokenActions.deleteToken());
} finally {
dispatch(tokenActions.setRefreshing(false));
Comment on lines +32 to +33
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

토큰 갱신 상태 관리 개선 필요

finally 블록에서 refreshing 상태를 false로 설정하는 것은 좋지만, 에러 발생 시에도 동일하게 처리되어 사용자에게 잘못된 상태를 보여줄 수 있습니다.

다음과 같이 에러 처리를 개선하는 것을 제안합니다:

      } catch (error) {
        dispatch(tokenActions.deleteToken());
+       throw error; // 상위에서 에러 처리할 수 있도록 전파
      } finally {
        dispatch(tokenActions.setRefreshing(false));
      }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
} finally {
dispatch(tokenActions.setRefreshing(false));
} catch (error) {
dispatch(tokenActions.deleteToken());
throw error; // 상위에서 에러 처리할 수 있도록 전파
} finally {
dispatch(tokenActions.setRefreshing(false));

}
};
tokenRefresh().catch((error) => {
Expand Down
12 changes: 11 additions & 1 deletion src/store/auth.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,12 @@ import { PayloadAction, createSlice } from '@reduxjs/toolkit';

interface TokenState {
accessToken: string | undefined;
isRefreshing: boolean;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

전역 상태에 토큰 재발급 상태 관리를 통해 재발급 중임을 명확히 할 수 있어서 좋은 방식인거 같습니다!

}

const initialState: TokenState = {
accessToken: undefined,
isRefreshing: true,
};

export const tokenSlice = createSlice({
Expand All @@ -15,18 +17,26 @@ export const tokenSlice = createSlice({
reducers: {
setToken: (state, action: PayloadAction<string>) => {
state.accessToken = action.payload;
state.isRefreshing = false;
},
deleteToken: (state) => {
state.accessToken = undefined;
state.isRefreshing = false;
},
setRefreshing: (state, action: PayloadAction<boolean>) => {
state.isRefreshing = action.payload;
},
},
});

export const { setToken, deleteToken } = tokenSlice.actions;
export const { setToken, deleteToken, setRefreshing } = tokenSlice.actions;

export const tokenActions = tokenSlice.actions;

export const selectAccessToken = (state: { token: TokenState }) =>
state.token.accessToken;

export const selectIsRefreshing = (state: { token: TokenState }) =>
state.token.isRefreshing;

export default tokenSlice.reducer;
Loading