[코드리뷰] 9주차 작업 #139
[코드리뷰] 9주차 작업 #139
Conversation
build 관련 ci 추가
…#113 가이드라인, 시니어 추가 페이지 UI 수정
배포된 주소로 로그인 기능 구현
로그인 관련 수정사항 롤백
시니또 메인 페이지 기능 수정
코드리뷰 반영 + 로그아웃 기능 추가
더미 로그인 리다이렉트 페이지
시니또용 콜백 조회 API 및 기능 수정
There was a problem hiding this comment.
현재 시니또 - 내 정보 조회 API 가 필요한곳이 시니또 메인페이지와 시니또 마이페이지 이렇게 2개의 페이지가 있는데, 메인페이지와 마이페이지 컴포넌트 사이의 거리가 멀기때문에 단순하게 contextAPI 써야겠다 ㅇ0ㅇ! 하고 후다닥 구현을 했습니다 ㅎㅎ.. 근데 생각해보면 내 정보 조회라는게 결국 이름, 이메일, 휴대폰 번호로 이렇게 3개만 가져오게 되는데, 그 중에서도 시니또 메인페이지에서는 '이름'만 필요로 합니다.
그럼에도 불구하고 contextAPi를 활용해서 provider를 만들고, 전역상태로 상태관리 하는게 옳을까? 라는 생각이 들었습니다. 결국은 전역으로 상태관리를 하게되면 그만큼 성능을 사용하는거라고 생각해서 최대한 안쓰는게 성능면에서는 좋겠다는 생각이 많이 드는데, 막상 또 localStorage에 저장하자니 개인정보라서 애매하기도 하네요...
이런 경우 현재 구현한 것 처럼 전역상태로 관리하는게 옳은걸까요?? 아니면 저번 멘토링때 멘토님께서 말씀하셨던것 같은데, 쿼리를 사용하면 어짜피 캐싱이 되니까 (캐싱이 되나? 라고 말씀하셨긴 한데..) 그냥 메인페이지, 마이페이지에서 두 페이지에서 요청을 보낼까? 라는 생각도 들었습니다..!
성능을 생각한다면 어떤 방법이 더 좋은 방법일까요??
일단 작성해주신 것 처럼 Context로 관리하는게 좋아보입니다 ㅎㅎ
혹은 별도의 상태관리 라이브러리를 설치해서 사용하는 방법도 있을 것 같아요.
아직도 AccessToken 만료 시 RefreshToken이 갈아끼워주는 기능을 완벽하게 구현하지 못하여 refreshToken이 존재하긴 하지만 제 역할을 하지 못하는 것 같습니다 ㅜㅜ
이건 지금 보니까 클라이언트의 문제라기보단... 서버에서 refresh token 에 대한 처리를 제대로 못해주는거 아닌가 싶어요.
클라이언트를 통해 테스트하기보단
API를 하나하나 주고받으면서 시도해보면 어떨까요?
그래도 잘 안 된다면 api 통신을 주고 받을 때 처리하는 로직의 문제일 수 있어요.
refresh token과 access token을 비교해보면 이렇게 되어있는데요
리프레시 토큰에 담겨있는 정보가 액세스 토큰에 담겨있는 정보와 동일하고 만료 시간만 다른 것 같아요.
꼭 리프레시 토큰을 jwt로 만들 필요는 없을 것 같고... 뭔가 토큰 자체에 대한 어떤 고유값을 가지고 있어야 하지 않을까 싶네요!
--
이 외에 react-query를 사용하곤 있지만 캐시가 하나도 안 되고 있어요 ㅎㅎ
모든 API 요청을 그대로 보내고 있네요.
이 부분도 같이 살펴보시면 좋겠어요!
2024-11-02.4.34.12.mov
여기서는 한 페이지에서 API 요청을 두 번 보내는데, 꼭 이럴 필요가 있을까요?
이런건 백엔드에서 합쳐달라고 요청해주세요.
2024-11-02.4.35.28.mov
데이터를 가져온 다음에 뒤로가기를 하면 다시 데이터를 또 불러옵니다.
뒤로가기를 했을 때 이전 데이터를 그대로 보존하는 방법은 없을까요~?
| @@ -18,6 +18,7 @@ | |||
| "@tanstack/react-query": "^5.51.11", | |||
| "axios": "^1.7.5", | |||
| "date-fns": "^4.1.0", | |||
| "dayjs": "^1.11.13", | |||
| "msw": "^2.3.5", | |||
There was a problem hiding this comment.
이제보니까 msw가 dependencies에 있네요..!
이건 devDependencies에 있어야할 것 같아요~
| - name: Install NodeJs | ||
| uses: actions/setup-node@v4 | ||
| with: | ||
| node-version: 22 | ||
|
|
||
| - name: Install Pnpm package manager | ||
| run: | | ||
| npm install -g pnpm |
There was a problem hiding this comment.
- uses: actions/setup-node@v4
with:
node-version: 22
cache: 'pnpm'이렇게 하면 pnpm 따로 설치하지 않아도 된답니다!
| <ChakraProvider> | ||
| <QueryClientProvider client={queryClient}> | ||
| <AllSeniorInfoProvider> | ||
| <AuthProvider> | ||
| <Global styles={globalStyle} /> | ||
| <Routes /> | ||
| </AuthProvider> | ||
| <SinittoInfoProvider> | ||
| <UserEmailProvider> | ||
| <Global styles={globalStyle} /> | ||
| <Routes /> | ||
| </UserEmailProvider> | ||
| </SinittoInfoProvider> | ||
| </AllSeniorInfoProvider> | ||
| </QueryClientProvider> | ||
| </ChakraProvider> |
There was a problem hiding this comment.
ApplicationProvider 라는걸 하나 만들어서 전체 Provider를 모아두는 방법도 있을 것 같네요..!
아니면 이런식으로 쓰일 수 있도록 한다거나!?
const Providers = createProviders([
ChakraProvider,
[QueryClientProvider, { client: queryClient }],
AllSeniorInfoProvider,
SinittoInfoProvider,
UserEmailProvider,
])
const App = () => {
<Providers>
<Global styles={globalStyle} />
<Routes />
</Providers>
}| // 로컬 스토리지에 토큰 저장 | ||
| localStorage.setItem('accessToken', accessToken); | ||
| localStorage.setItem('refreshToken', refreshToken); | ||
| localStorage.setItem('isSinitto', isSinitto); |
There was a problem hiding this comment.
로컬 스토리지를 다룬 로직은 추상화해주면 좋을 것 같아요!
tokenStorage.set({ accessToken, refreshToken })
이런식으로 쓰일 수 있도록..!
| @@ -1,13 +1,15 @@ | |||
| import { Link } from 'react-router-dom'; | |||
|
|
|||
| import Logo from '../../assets/kakao.svg'; | |||
| import { KAKAO_AUTH_URL } from '@/shared/utils/env/config'; | |||
| import Logo from '@/pages/assets/main/kakao.svg'; | |||
There was a problem hiding this comment.
pages에서 pages에 있는걸 가져오게 되면 순환참조가 될 수 있답니다..!
같은 계층에 있는 것들은 상대경로로 가져와야 안전해요.
|
|
||
| return ( | ||
| <Flex w='100%' alignItems='center' my={10}> | ||
| <Text color='var(--color-primary)' fontSize='24px' fontWeight='700'> |
There was a problem hiding this comment.
color="primary" 이렇게 표현 가능하지 않을까요..!?
아니면 Charka의 토큰을 통해서 선언해주면 좋을 것 같아요.
| @@ -0,0 +1,8 @@ | |||
| import { fetchInstance } from '@/shared/api/instance'; | |||
|
|
|||
| const getLogoutPath = '/api/members/logout'; | |||
| @@ -69,27 +65,23 @@ export const GuideLineButton = ({ | |||
| }; | |||
|
|
|||
| return ( | |||
| <Wrapper mt={marginTop} mb={marginBottom}> | |||
| {GUIDE_LINE_CATEGORIES.map((data) => ( | |||
| <Box w='full' mt={marginTop} mb={marginBottom}> | |||
| @@ -0,0 +1,26 @@ | |||
| import { createContext, useContext, ReactNode } from 'react'; | |||
|
|
|||
| import { SinittoInfoResponse, useGetSinittoInfo } from '@/pages'; | |||
| export const useSinittoInfo = (): SinittoInfoType => { | ||
| const context = useContext(SinittoInfo); |
There was a problem hiding this comment.
return type 명시해주지 않아도 자연스럽게 추론될 것 같아요~
📝 작업 내용
💬 리뷰 요구사항
안녕하세요 멘토님! 저번주차 코드 리뷰를 너무 꼼꼼히 잘 봐주셔서 감사하다는 말씀 먼저 전해드립니다 ㅠㅠ 저희 프로젝트의 경우 아마 다음주까지 대부분의 기능은 모두 구현을 완료하고, 이후에는 리팩토링과 오류 수정에 집중할 것 같습니다...! 멘토님께서 이것 저것 많이 알려주신 덕분에 빠르게 구현할 수 있었습니다!
더미데이터 로그인
이번주차에 백엔드에서 '더미데이터 로그인'기능을 구현하여 관련 코드를 추가해두었습니다.
http://sinitto.site:8080/dummy
이 페이지에서 로그인이 가능하며, 모든 이메일에 대한 비밀번호는
1234를 입력하면 백엔드쪽에서 구현해 둔 더미데이터에 대해 로그인이 가능합니다!로그인 기능 수정
드디어 카카오 로그인 기능을 수정했습니다! ㅜㅜ 이제 배포환경, local 환경 모두 로그인이 가능합니다! 백엔드쪽에 해당 부분 로직을 수정하여 이제 정상적으로 로그인이 가능합니다!
http://sinitto.s3-website.ap-northeast-2.amazonaws.com/
상태관리 관련 질문!
현재 시니또 - 내 정보 조회 API 가 필요한곳이 시니또 메인페이지와 시니또 마이페이지 이렇게 2개의 페이지가 있는데, 메인페이지와 마이페이지 컴포넌트 사이의 거리가 멀기때문에 단순하게 contextAPI 써야겠다 ㅇ0ㅇ! 하고 후다닥 구현을 했습니다 ㅎㅎ.. 근데 생각해보면 내 정보 조회라는게 결국 이름, 이메일, 휴대폰 번호로 이렇게 3개만 가져오게 되는데, 그 중에서도 시니또 메인페이지에서는 '이름'만 필요로 합니다.
그럼에도 불구하고 contextAPi를 활용해서 provider를 만들고, 전역상태로 상태관리 하는게 옳을까? 라는 생각이 들었습니다. 결국은 전역으로 상태관리를 하게되면 그만큼 성능을 사용하는거라고 생각해서 최대한 안쓰는게 성능면에서는 좋겠다는 생각이 많이 드는데, 막상 또 localStorage에 저장하자니 개인정보라서 애매하기도 하네요...
이런 경우 현재 구현한 것 처럼 전역상태로 관리하는게 옳은걸까요?? 아니면 저번 멘토링때 멘토님께서 말씀하셨던것 같은데, 쿼리를 사용하면 어짜피 캐싱이 되니까 (캐싱이 되나? 라고 말씀하셨긴 한데..) 그냥 메인페이지, 마이페이지에서 두 페이지에서 요청을 보낼까? 라는 생각도 들었습니다..!
성능을 생각한다면 어떤 방법이 더 좋은 방법일까요??
⏰ 현재 버그
refreshToken
아직도 AccessToken 만료 시 RefreshToken이 갈아끼워주는 기능을 완벽하게 구현하지 못하여 refreshToken이 존재하긴 하지만 제 역할을 하지 못하는 것 같습니다 ㅜㅜ
https://github.com/kakao-tech-campus-2nd-step3/Team8_FE/blob/Weekly/src/shared/api/instance/Instance.tsx
다음주차에 accessToken 만료시간을 임시로 1분정도의 짧은 시간으로 수정하고, 해당 기능이 잘 동작하는지 확인하면서 수정해야하지 않을까.. 라는 생각이 듭니다..
시니또 - 마이페이지 내정보 및 계좌정보
원래는 시니또의 내 정보 확인 API 호출 시 내 정보와 계좌정보 및 계좌 검증 로직이 한번에 수행되어 백엔드쪽에서 내 정보 api 따로, 계좌 관련 정보 api 따로 구현하는 방법으로 수정을 진행하였고, 아직 마이페이지 부분은 수정중이라서 관련 사항이 동작하지 않을 것 같습니다..!
이 부분은 아마 쉽게 수정될 것 같습니다!