Conversation
| <Route path="/Items" element={<Items />} /> | ||
| <Route path="/Items/:productId" element={<ItemInfo />} /> | ||
| <Route path="/AddItem" element={<AddItem />} /> | ||
| <Route path="/Community" element={<Community />} /> |
There was a problem hiding this comment.
p4;
크리티컬한 부분은 아니지만, �경로 부분은 소문자로 사용하는게 일반적입니다!
대문자가 유효하지 않은 건 아니지만, 특히 협업하는 경우에 혼란이 발생할 수도 있고
사용자경험 측면에서도, 소문자로 일관성을 유지하는게 좋은 것 같습니다!
이미 프로젝트 전반에 적용하셨으니, 지금 당장 수정하실 필요는 없지만 참고하시라고 말씀드립니다!
| const file = eventObject.target.files[0]; | ||
| const handleImageChange = (e: ChangeEvent<HTMLInputElement>) => { | ||
| if (e && e.target && e.target.files) { | ||
| const file = e.target.files[0]; |
There was a problem hiding this comment.
p4;
이렇게 보수적으로 조건을 작성하셔도 좋습니다!
다만, ChangeEvent 객체에서 e.target은 항상 존재하므로 아래와 같이 간결하게 적어도 괜찮을 것 같아요!
const handleImageChange = (e: ChangeEvent<HTMLInputElement>) => {
const file = e.target.files ? e.target.files[0] : null;
onImageChange(file);
};| </li> | ||
| <li> | ||
| <Link | ||
| to="/Items" |
There was a problem hiding this comment.
p4;
경로를 상수화해서 프로젝트 전반에서 사용해도 좋을 것 같아요!
ex)
export const ROUTES = {
HOME: "/",
SIGN_IN: "/Signin",
COMMUNITY: "/Community",
ITEMS: "/iItems",
ITEM_INFO: (productId: string) => `/items/${productId}`,
};| { list: [], totalCount: 0 } | ||
| ); | ||
|
|
||
| const { list = [], totalCount = 0 } = data || {}; |
There was a problem hiding this comment.
p5;
보수적으로 data가 null이거나 undefined인 상황을 대비해 기본값을 설정하신 것 좋습니다!
다만, 이미 useFecth에 initialValue를 전달하고 있어서, 폴백 객체를 설정하지 않으셔도 무방할 것 같아요!
p4;
이왕이면 조금 더 직관적으로 어떤 list인지 이름을 변경해 사용하면 더 좋을 것 같아요!
const { list:productList = [], totalCount: productTotalCount = 0 } = productResponseData;| return () => { | ||
| window.removeEventListener("resize", handleResize); | ||
| }; | ||
| }, [pageSize]); |
There was a problem hiding this comment.
p4;
handleResize 함수를 effect 내부로 옮기거나, useCallback으로 감싸 useEffect 의존성에 hnadleResize를 지정하면 좋을 것 같아요!
|
|
||
| const navigate = useNavigate(); | ||
|
|
||
| const comments: CommentResponse[] = Array.isArray(commentsData) |
There was a problem hiding this comment.
p4;
이것도 useFetch 호출시에 initial_comments로 상태를 초기화 시켜주고 있으니, 생략해도 괜찮을 것 같아요!
�혹시나 comments가 null이나 undefined가 되는 상황이 생겨, 런타임 에러가 발생하는걸 방지하고 싶다면
아래 렌더링 하는 부분에 옵셔널 체이닝으로 map을 돌리면 좋을 것 같아요!
const comments = Array.isArray(commentsData)
? commentsData
: []; 제거
return (
...
{commentsData?.map(...)}
)| declare module "*.svg" { | ||
| const value: string; | ||
| export default value; | ||
| } |
There was a problem hiding this comment.
이미지 파일을 모듈로 인식하도록 잘 설정하셨습니다!
이렇게 하면 이미지를 모듈로 임포트하여 직관적인 사용이 가능해 좋은 방식이에요!
| const isValid = | ||
| Boolean(itemName) && | ||
| Boolean(itemDescription) && | ||
| Boolean(itemPrice) && |
There was a problem hiding this comment.
p5;
불리언임을 명확하게 보여주어 이런 방식도 좋지만, 간결성 측면에서는 두번의 NOT연산을 붙여 값을 불리언 타입으로 변환하는 것도 좋습니다!
const isValid =
!!itemName &&
!!itemDescription &&
!!itemPrice &&
itemTags.length > 0;| banner: "#cfe5ff", | ||
| }, | ||
| }, | ||
| }, |
There was a problem hiding this comment.
tailwind의 테마를 잘 확장하여 사용하셨습니다!
| autoprefixer: {}, | ||
| }, | ||
| ], | ||
| }; |
There was a problem hiding this comment.
기타)
나중에 팀 프로젝트에서 tailwind를 사용하게 된다면, 자동으로 class 순서를 포매팅해주는 플러그인 도입도 고민해보세요!
https://tailwindcss.com/blog/automatic-class-sorting-with-prettier
폴더 구조는 나쁘지 않은 것 같습니다. page 폴더에 각 페이지에 대응하는 컴포넌트를 잘 대응 시켜서 명확하게 페이지 컴포넌트를 표시하셨고, 조금 더 사족을 붙이자면, hooks나 utils도 사용되는 페이지에 따라 폴더로 구분하고 공통으로 사용되는 것들은 common에 구분하여도 좋을 것 같습니다! |
|
전반적으로 타입 정의도 잘하시고, tailwind 적용 및 기타 컴포넌트 작성도 잘 하신 것 같아요! 코드를 보면서 최대한 타입스크립트를 중심으로 개선되면 좋을 만한 것들에 대해 리뷰를 남겼어요! +) 리뷰를 남긴 코멘트 위의 p(1~5); 마크의 의미는 아래와 같습니다! 참고하면 좋을 것 같아요! |
요구사항
기본
주요 변경사항
스크린샷
멘토에게