-
Notifications
You must be signed in to change notification settings - Fork 1
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
feat: axios 세팅 #13
feat: axios 세팅 #13
Conversation
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.
axios 설정과 로그인/회원가입 기능 구현해주신 내용 확인했습니다.
axios 설정 같은 부분들은 한번 정리 잘 해두시면 다른 프로젝트에서도 계속 활용해보실수 있으니, 블로깅하거나 계속해서 디벨롭해보시면 좋을것 같아요.
기능적인 부분에서는 문제가 될만한 부분은 보이지 않았고,
부분적으로 개선했으면 하는 내용이 있어 리뷰 드렸습니다.
src/api/client.ts
Outdated
export const handleAxiosError = (error: any) => { | ||
if (axios.isAxiosError(error)) { | ||
const axiosError = error as AxiosError; | ||
console.error('Axios Error:', axiosError); | ||
return axiosError.response?.data as APIResponse; | ||
} | ||
console.log('Unknown Error: ', error); | ||
return { | ||
success: false, | ||
msg: '알 수 없는 오류가 발생했습니다.', | ||
}; | ||
}; |
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.
API 요청에 대한 오류 처리 잘해주신 것 같아요 👍
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.
axios error와 일반 error를 분기해 처리해주었습니다!
src/api/client.ts
Outdated
}); | ||
|
||
const handleHeadersWithAccessToken = (config: AxiosRequestConfig): InternalAxiosRequestConfig => { | ||
const accessToken = localStorage.getItem('ACCESS_TOKEN') || ''; |
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.
P3:
ACCESS_TOKEN이라는 값도 constant로 관리 되면 좋겠습니다.
다른곳에도 활용이되면서 정적인 값이라면 constant로 분리하는것을 고려해주시면 더욱 프로젝트에 활용하기가 유용할 것 같아요.
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.
const 폴더에서 관리할 수 있도록 분리하겠습니다. 👀
src/api/client.ts
Outdated
client.interceptors.request.use(handleHeadersWithAccessToken); | ||
client.interceptors.response.use( | ||
(response: AxiosResponse) => response, | ||
(error: AxiosError) => Promise.reject(error), | ||
); |
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.
interceptor 활용해서 반복되는 request, response에 대한 요청을 처리해주신 부분이 보기 좋네요!
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.
해당 부분은 axios 공식문서를 보고 학습하고 TIL로 남겨두었습니다. 이번 기회에 인터셉터를 직접 사용해 봤네요😌
src/pages/SigninPage.tsx
Outdated
const response = await http.post('/user/signin', { | ||
email: user.email, | ||
password: user.password, | ||
}); |
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.
P3:
axios를 모듈로 재정의해주시면서 제네릭으로 response, request 타입을 지정되도록 설정을 해주셨는데요.
이부분 활용해서 로그인 함수에도 적용해보시면 좋을것 같습니다.
그러면 각 API요청에 대한 타입을 작성하는것이 필요할 것이고, 이런것들을 하나씩 반영해보시면서 프로젝트를 개선 할 수 있을것 같네요.
그리고 사소하지만
변수명 response
보다는 좀 더 명확히 responseSignIn
정도로 변수명을 작성해보시는건 어떨까요?
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.
post의 응답 타입을 지정했습니다. 이와 관련한 테스트 오류가 있어서 블로그에 [트러블 슈팅] 모듈 시스템과 Jest 테스트
글을 남겨 두었습니다!
src/pages/SigninPage.tsx
Outdated
const accessToken = response.data.user.user_token; | ||
localStorage.setItem('ACCESS_TOKEN', JSON.stringify(accessToken)); |
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.
로그인했을때 accessToken만 저장되고 있는것으로 보여지네요.
유저에 대한 정보는 없었을까요? 🤔
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.
로그인에 성공할 경우 유저 토큰과 name이 함께 넘어옵니다. name도 로컬 스토리지에 저장해 주는 것이 좋을까요? 회원가입 때 입력한 닉네임이 마이 페이지에서 보여지도록 구현하려고 했습니다. 생각해보니 닉네임의 수정 가능 여부도 고려를 해봐야 할 것 같네요.
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.
로그인 시 다음과 같이 유저 정보가 넘어옵니다. 우선은 로컬 스토리지에 USER 키값에 오브젝트로 name을 저장하도록 구현했습니다.
const accessToken = responseSignIn.data.user.user_token;
const userProfile = {
name: responseSignIn.data.user.name,
};
localStorage.setItem(ACCESS_TOKEN, JSON.stringify(accessToken));
localStorage.setItem(USER, JSON.stringify(userProfile));
} catch (e) { | ||
const error = handleAxiosError(e); | ||
alert(error.msg); | ||
} | ||
}; | ||
|
||
const isError = { |
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.
P3:
isError에서 input 항목들의 validation 관리를 해주는것은 좋아보입니다.
조금 더 확장해서 message도 여기서 관리해주면 어떨까요?
const isError = {
email: {
error: user.email.length > 0 && !userValidation.email,
message: '이메일 형식이 올바르지 않습니다.'
},
...
}
<ErrorMessage>{isError.email.error && isError.email.message}</ErrorMessage>
위와 같이 개선해볼수 있겠습니다.
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.
해당 부분 개선했습니다!
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.
카카오 로그인 구현해주신다고 고생하셨어요!
몇가지 궁금한 점이 있어 리뷰 드렸습니다.
src/pages/AuthKakaoPage.tsx
Outdated
const responseSignIn = await http.post<{ token: string; user: { name: string } }>('/user/oauth/kakao', { code }); | ||
const isSuccess = responseSignIn.success; | ||
|
||
if (isSuccess && responseSignIn.data) { | ||
const accessToken = responseSignIn.data.token; | ||
const userProfile = { | ||
name: responseSignIn.data.user.name, | ||
}; | ||
localStorage.setItem(ACCESS_TOKEN, JSON.stringify(accessToken)); | ||
localStorage.setItem(USER, JSON.stringify(userProfile)); | ||
navigate(PATH.CALENDAR); | ||
} |
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.
P2:
카카오 로그인 요청을 성공했을때 처리는 잘해주신 것 같아요.
혹시 요청에 대한 실패나 오류가 있을때는 어떻게 처리가 될까요?
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.
try catch로 실패했을 경우의 처리를 해주어야겠네요! 이 부분 반영하겠습니다.
const KAKAO_CLIENT_ID = 'a3d08c94732b0c85334d04b474f49873'; | ||
const KAKAO_REDIRECT_URL = 'http://localhost:5173/oauth/kakao'; | ||
export const KAKAO_AUTH_URL = `https://kauth.kakao.com/oauth/authorize?client_id=${KAKAO_CLIENT_ID}&redirect_uri=${KAKAO_REDIRECT_URL}&response_type=code`; |
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.
constant를 변수로 잘 분리해주셨네요. 👍
src/pages/SigninPage.tsx
Outdated
@@ -41,19 +43,40 @@ export default function SigninPage() { | |||
[userValidation.email, userValidation.password], | |||
); | |||
|
|||
const handleClickSignIn = async () => { | |||
if (!isDisabledSubmit) { |
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.
P3:
disabled 일 경우에만 실행되도록 잘 작성해주신 것 같아요.
하지만 코드를 봤을때 한번더 생각되게 만드는것 같습니다.
if (isDisabledSubmit) {
return;
}
이런식으로 작성해주시면 직관적으로 disabled일 경우, 함수의 동작이 되지 않는다는걸 바로 알 수 있을것 같습니다.
작업 내용
/calendar
페이지로 리다이렉션 됩니다.REST API
를 이용한 카카오 로그인을 구현했습니다./home
페이지로 리다이렉션 됩니다.ACCESS_TOKEN
값이 삭제됩니다./home
페이지로 리다이렉션 됩니다.화면 스크린샷 (optional)
카카오 로그인
사용자가 처음 카카오 로그인을 시도할 경우 뜨는 동의 화면 창입니다.
로그아웃 버튼
마이 페이지에서 로그아웃을 할 수 있습니다.
스타일링은 추후 마이 페이지를 구현할 때 보완할 예정입니다.
참고 문서 (optional)
리뷰 규칙
P1: 꼭/적극적 반영해 주세요 (Request changes)
P2: 웬만하면 반영해 주세요 (Comment)
P3: 반영해도 좋고 넘어가도 좋습니다 (Approve)