부산대 FE 12조 9주차 PR #49
Conversation
- nickname: 3-10자 한글, 영문, 숫자만 - 이메일, 연락처는 형식에 맞게 - 자기소개 최대 255자
어차피 컨텍스트 내부의 값이 바뀌면 다시 렌더링해야하기 때문에 불필요하다고 생각. 추후에 논의해볼 필요가 있음
| }); | ||
|
|
||
| axiosInstance.interceptors.response.use((config) => config, async (error) => { | ||
| if (!isAxiosError(error)) return error; |
There was a problem hiding this comment.
| if (!isAxiosError(error)) return error; | |
| if (!isAxiosError(error)) return Promise.reject(error); |
에러 상황일 때는 rejected promise를 반환해야 에러로 올바르게 인식될 거에요.
src/utils/network/index.ts
Outdated
| const { config, response } = error; | ||
| if (!response || !config || response.status !== HttpStatusCode.Unauthorized | ||
| ) return error; | ||
| const accessToken = await reIssueAccessToken(); |
There was a problem hiding this comment.
토큰 재발급이 실패됐다면 기존에 있던 토큰을 삭제해주는 코드도 필요해보여요
try {
const accessToken = await reIssueAccessToken();
// ...
} catch (error) {
// 로컬스토리지에서 각종 토큰 제거
return Promise.reject(error);
}
src/utils/network/index.ts
Outdated
| const { config, response } = error; | ||
| if (!response || !config || response.status !== HttpStatusCode.Unauthorized | ||
| ) return error; | ||
| const accessToken = await reIssueAccessToken(); |
There was a problem hiding this comment.
여러 http 요청이 401 응답으로 인해 동시에 실패하게 된다면 토큰 재발행은 여러 번 실행될 거에요. 이미 재발급 하고 있다면 같은 promise를 await 할 수 있도록 개선할 수 있긴 합니다. 지금은 이런 문제가 있다는 점만 인식하고 계시고 프로젝트 완료하다가 일정이 있을 때 개선해보면 좋을 거 같습니다.
추가로 access token 말고도 refresh token도 유효기간이 있을거라 재발급 하기 전에 유효기간이 남아있다면 요청하도록 개선하는 것도 가능해보여요.
There was a problem hiding this comment.
아하, 정말 생각도 못했던 문제점이었는데 지적해주셔서 정말 감사합니다.
이미 재발급 하고 있다면 같은 promise를 await 할 수 있도록 개선할 수 있긴 합니다. 지금은 이런 문제가 있다는 점만 인식하고 계시고 프로젝트 완료하다가 일정이 있을 때 개선해보면 좋을 거 같습니다.
만약에 멘토님이 말씀해주신 내용을 그대로 구현한다면 클로저를 이용할 듯 한데, 적절한 방법인지 궁금합니다!
There was a problem hiding this comment.
클로저보다는 토큰을 관리하는 객체를 하나 만드셔서 이미 재발급 중이라면 같은 promise를 반환해서 여러 요청이 중복 발생하지 않도록 관리해주는게 간단할 거 같아요.
|
|
||
| const endpoints = { | ||
| myInfo: `${prefix}/users`, | ||
| reIssue: `${prefix}/reissue`, |
There was a problem hiding this comment.
인증이 필요한 path가 늘어날 때 마다 isAuthRequired 관련해서 해당 파일을 수정해주셔야 할 거라 번거롭지 않을까요? request interceptor에서 토큰이 있다면 그냥 전달하는게 간단할 거 같아요.
| } | ||
| const stringifiedValue = JSON.stringify(value); | ||
| storage.setItem(key, stringifiedValue); | ||
| }; |
There was a problem hiding this comment.
보통 set에 기대하는 역할은 값을 설정하는 역할이고 제거는 remove 처럼 분리하는게 자연스럽지 않을까요? 지금은 값 설정과 삭제가 set에 여러 책임이 혼재되어 있어 보여요.
// before
tokenStorage.set();
// after
tokenStorage.remove();| } catch (e) { | ||
| // TODO: 에러 코드를 통해 token이 만료되어서 발생한 에러인지 확인 후 토큰 초기화 | ||
| tokenStorage.set(); | ||
| } |
There was a problem hiding this comment.
토큰 만료되어서 제거하는 코드는 인터셉터에서 처리하지 않고 api 함수 호출하는 코드에서 따로 처리하고 있네요. 이렇게 되면 요청마다 중복적으로 처리해줘야할 거라 interceptor 활용해서 일괄 처리하는게 좋겠습니다.
| export default function SubmitAssignmentModal({ | ||
| open, onClose, assignName = '', assignDesc = '', | ||
| }: SubmitAssignmentProps) { | ||
| const theme = useTheme(); |
There was a problem hiding this comment.
useTheme가 여러 컴포넌트에서 자주 사용되고 있네요. 지금처럼 구현해볼 수 있지만 반대로 useTheme에 대한 의존성이 코드 전반에 늘어나는 거긴 합니다. useTheme 사용이 필요하다면 UI 컴포넌트를 더 작은 요소로 분리해서 해당 요소에서만 useTheme을 사용하도록 하고 필요시 props에 따라 알맞은 theme 값을 소비하도록 구현해주면 실제 소비는 분리한 UI 컴포넌트로 이뤄지기에 theme의 내부 구현이 변경되더라도 다른 코드로 변경이 전파되지 않는다는 장점이 있을 수 있습니다.
프로젝트 마무리 시기에 꼭 수정해야하는건 아닌거 같고, 개발하시면서 한번쯤 생각해보시면 좋을 거 같습니다.
| await userEvent.type(screen.getByLabelText('이메일'), '[email protected]'); | ||
| await userEvent.type(screen.getByLabelText('연락처'), '010-1234-5678'); | ||
| await userEvent.type(screen.getByLabelText('자기소개'), '안녕하세요!'); | ||
| const agreeCheckbox = screen.getByTestId('agree-checkbox'); |
There was a problem hiding this comment.
testing-library의 query priority 설명을 보면 Query API의 우선순위가 있다는걸 알 수 있습니다. getByRole가 1순위이고 getByTestId가 가장 후순위인데요, 이는 접근성을 통해 원하는 요소를 조회하도록 유도하는걸 알 수 있습니다. 반대로 표현해보면 접근성이 부족하다면 getByRole을 사용하기 힘든 환경이라는거죠.
그래서 testing-library를 사용할 때는 가능하다면 우선순위 높은 Query API를 사용하는게 좋을 거 같고, 우선순위 높은 API를 사용하기 어렵다면, 어려운 원인을 분석해서 코드를 개선해줄 수 있습니다. 지금은 checkbox의 접근성을 개선시킬 수 있겠죠. 테스트 코드 작성시 참고 부탁드립니다.
There was a problem hiding this comment.
조금 더 알아보니 사용자 관점에서 테스트를 작성하자는 관점에서 priority를 지켜야 한다는 것을 알게 되었습니다. 지적해주셔서 감사합니다!
안녕하세요 멘토님,
어느덧 Step 3의 막바지에 접어들었네요. 아직 수료할 마음의 준비가 안됐는데 이제 다다음주면 끝난다고 생각하니 조금 착잡합니다.. 마지막까지 잘 부탁드리겠습니다!!
구현한 기능
리뷰받고 싶은 내용
시험기간에 공부하다가 이번 주차에 갑자기 몰아서 작업한 탓에, 커밋이 너무 많이 쌓여서 보시기에 불편할 수도 있다는 생각이 듭니다. 번거로우시겠지만 양해 부탁드립니다..!
감사합니다.