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
5 changes: 4 additions & 1 deletion 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 @@ -50,6 +50,7 @@ const App: React.FC = () => {

const isMobile = useIsMobile();
const isLoggedIn = !!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
3 changes: 0 additions & 3 deletions src/components/Routes/ProtectedRoutes.tsx
Original file line number Diff line number Diff line change
@@ -1,15 +1,12 @@
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;
};

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

return isLoggedIn ? <Outlet /> : <Navigate to={PATH.LOGIN} />;
};

Expand Down
15 changes: 7 additions & 8 deletions src/hooks/common/useTokenRefresh.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
/* 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';
Expand All @@ -9,27 +8,27 @@ import { useNavigate } from 'react-router-dom';
export const useTokenRefresh = () => {
const dispatch = useNewDispatch();
const navigate = useNavigate();
const queryClient = useQueryClient();

const accessToken = useNewSelector(selectAccessToken);

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

try {
const newAccessToken = await getRefreshToken();
dispatch(tokenActions.setToken(newAccessToken));

await queryClient.invalidateQueries({
queryKey: ['members'],
});
Copy link
Collaborator

Choose a reason for hiding this comment

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

이 부분은 재발급 받은 useMemberQuery를 무효화하는 작업이라 흐름 상 새로운 발급 받은 token에 대해서 새로 useMemberQuery가 실행되는게 적절해보이는데 삭제하신 이유가 있을까요?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

전에 원준님께서 헤더의 회원 정보 api 관련으로 말씀하셨던 부분을 이 부분으로 착각했네요,,,,😲
062f176 에서 다시 되돌려놓았습니다!!!!!!!!

} 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) => {
console.error('토큰 에러: ', error);
});
}, [accessToken, dispatch, navigate, queryClient]);
}, [accessToken, dispatch, navigate]);
};
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