Conversation
There was a problem hiding this comment.
@jangmoonwon
문원님! 고생 많으셨어요
ㅎㅎ 다양한 패턴들이 섞여지고 있네요
그래도 이전보다 확실히 관심사의 차원에서 구분이 되어가고 있는게 보입니다.
디테일 한 리뷰는 코드단에 남겨두었으니 참고 부탁드려요!
There was a problem hiding this comment.
@jangmoonwon
요 형태를 한번 참고해보면 좋겠어요!
https://github.com/karlhadwen/netflix/blob/master/src/containers/header.js
|
|
||
| const cx = classNames.bind(styles); | ||
|
|
||
| export default function AddFolderButton() { |
There was a problem hiding this comment.
onClick 이벤트 핸들러는 아직 전달이 안되는거죠?
| onClick: () => void; | ||
| }; | ||
|
|
||
| export default function FolderButtons({ |
There was a problem hiding this comment.
복수형인 이유가 있나요?
| onClick: () => void; | ||
| }; | ||
|
|
||
| export default function ToolBarBtnList({ |
There was a problem hiding this comment.
@jangmoonwon
리스트 요소를 렌더링 하는걸로 보이지 않는데 이름이 리스트인 이유가 있나요?
There was a problem hiding this comment.
getElapsedTime에서만 사용되는 상수라면, 따로 상수 페이지로 빼기보다
그 함수에서만 구현해서 활용해도 충분할 것 같아보여요!
| @@ -0,0 +1,14 @@ | |||
| import axiosInstance from "./axiosInstance"; | |||
|
|
|||
| type DataType = { | |||
There was a problem hiding this comment.
data type이라는 이름의 타입보다는 어떤 함수의 인자를 정의하는 요소임을 뜻하는 타입 이름이 설정되는게 더 좋지 않을까 싶어요
| const newRes = res.data; | ||
| const accessToken = newRes.data.accessToken; |
There was a problem hiding this comment.
이런식으로 axios라이브러리를 통해 지속적으로 반복되는 데이터 파싱을 구현체 단에서 처리하기보단
axios에서 제공해주는 interceptor를 활용해 데이터 파싱을 axios instance선언되는곳에서 처리하도록 하는게 조금 더 깔끔할 것 같아요
| type Data = { | ||
| id: number; | ||
| created_at: string; | ||
| name: string; | ||
| user_id: number; | ||
| favorite: boolean; | ||
| link: Link; | ||
| }; |
There was a problem hiding this comment.
서버에서 전달받은 folder entity인데 folder가 아닌 그냥 data라는 타입으로 선언하면 네이밍이 파악하기 어려울 것 같아요
또한 로컬 스코프에서 타입 설정을 해두어 외부에서 이 값을 사용하기 껄끄러워질 것 같은데 이부분도 함께 고민이 필요해보입니다
| try { | ||
| const response = await axiosInstance.get<DataType>("/users/1/folders"); | ||
| const data = response.data.data; | ||
| setData(data); | ||
| } catch (error) { | ||
| setIsError(error); | ||
| } finally { | ||
| setIsLoading(false); | ||
| } |
There was a problem hiding this comment.
axios에서 에러 제공해주는 형태가 있을텐데 그냥 무턱대고 any를 에러객체로 꽂아버리는건 위험해보입니다.
공식문서 한번 참고해보시구 어떻게 처리해야 할 지 고민이 필요해 보여요
요구사항
기본
심화
주요 변경사항
스크린샷
멘토에게