Feat: 카카오 로그인 추가, 게스트 로그인 컴포넌트화 #39
Conversation
jungjinbeom
commented
Oct 14, 2024
jungjinbeom
commented
- 게스트 로그인 컴포넌트로 분리
- 카카오 로그인 기능 구현
- 카카오 로그인 oauth 요청 컴포넌트 개발
- 카카오 로그인 컴포넌트 개발
jungjinbeom
requested review from
Seung-wan,
Kimyerim1935,
Shin-Yu-1 and
junhyeong00
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
| accessToken: string; | ||
| }; | ||
|
|
||
| type KakaoLoginParamsProps = { |
There was a problem hiding this comment.
Props는 �타입 이름에 붙을 필요가 없을 것 같은데 어떠신가요?
| }; | ||
|
|
||
| export const kakaoLogin = async ({ code }: KakaoLoginParamsProps): Promise<KakaoLoginResponse> => { | ||
| const { data } = await http.get(`${API_PATH.KAKAO_LOGIN}?code=${code}`); |
There was a problem hiding this comment.
query parameter를 axios의 params 속성을 이용해서 넣어주시면 더 좋을 것 같습니다.
There was a problem hiding this comment.
api endpoint를 별도 상수로 추상화하면 얻는 장점이 있을까요?
There was a problem hiding this comment.
medium이랑 narrow는 어떤 의미로 파일명에 붙여주신건가요?
There was a problem hiding this comment.
기존에 카카오 페이지 받은 파일명을 그대로 사용했습니다.
불필요한 네이밍은 빼겠습니다
|
|
||
| const GuestLogin = () => { | ||
| const navigate = useNavigate(); | ||
| const handlePage = () => navigate(ROUTE_PATH.STUDY_ROOMS); |
There was a problem hiding this comment.
요런 이름은 어떠신가요?
| const handlePage = () => navigate(ROUTE_PATH.STUDY_ROOMS); | |
| const �goToStudyRoom = () => navigate(ROUTE_PATH.STUDY_ROOMS); |
There was a problem hiding this comment.
의도가 명확하여 좋은거 같습니다
| import { tokenStorage } from '@/utils/storage'; | ||
| import { useMutation } from '@tanstack/react-query'; | ||
|
|
||
| interface UseKakaoLoginMutationProps { |
There was a problem hiding this comment.
컴포넌트에서만 일반적으로 Props라는 표현이 쓰이기 때문에 여기서는 제거해줘도 괜찮을 것 같습니다.
| import { useMutation } from '@tanstack/react-query'; | ||
|
|
||
| interface UseKakaoLoginMutationProps { | ||
| handlePage: () => void; |
There was a problem hiding this comment.
useKakaoLoginMutation 함수의 입장에서 매개변수로 handlePage를 받는 건 동작의 예상이 전혀 안되는 것 같아요.
There was a problem hiding this comment.
onSuccess 함수를 넘기는건 어떤가요?
| onSuccess: ({ accessToken }) => { | ||
| tokenStorage.setItem(accessToken); | ||
| handlePage(); | ||
| }, |
There was a problem hiding this comment.
handlePage는 훅 내부에서 실행되는 시점과 의도를 전혀 드러내지 못하는 것 같습니다.
There was a problem hiding this comment.
onSuccess 자체를 넘기는건 어떤가요??
| return useMutation({ | ||
| mutationFn: kakaoLogin, | ||
| onSuccess: ({ accessToken }) => { | ||
| tokenStorage.setItem(accessToken); |
There was a problem hiding this comment.
서버로부터 받아온 accessToken을 스토리지에 저장하는 로직이 훅 내부에 위치하는게 썩 나쁘지는 않다고 생각하는데요. 저는 그래도 이런 로직은 외부에서 다루는게 어떨까 싶습니다.
| @@ -2,11 +2,11 @@ import { login } from '@/apis/auth/login'; | |||
| import { tokenStorage } from '@/utils/storage'; | |||
| import { useMutation } from '@tanstack/react-query'; | |||
|
|
|||
| interface useLoginMutationProps { | |||
| interface UseLoginMutationProps { | |||
