Conversation
xcjnzvc
added
매운맛🔥
판다마켓/.DS_Store
Outdated
There was a problem hiding this comment.
해당 파일은 Mac 색인 파일로 과제나 프로젝트에 필요치 않은 파일입니다. gitignore를 통해 커밋에서 제외해 보시는건 어떨까요?
| "https://thumbnail7.coupangcdn.com/thumbnails/remote/492x492ex/image/retail/images/2016/03/31/18/6/c21ad6ab-fbb0-445e-b0e6-248a46d9d84a.jpg"; | ||
| }} |
There was a problem hiding this comment.
이미지가 없을 때 대체로 사용하는 파일은 통상적으로 동일한 이미지를 사용할 것이라 예상됩니다. 그렇다면 매 에러마다 이미지를 하드 코딩 하는 것 보다는 변수로 분리하여 재사용성을 높이는 방향으로 작업해 보시는 것도 도움이 될 것 같습니다!
| const settingProduct = async () => { | ||
| const data = await getProductsApi(); | ||
| // console.log(data); | ||
| setProducts(data.list); | ||
| setTotalLength(data.totalCount / sellCount); | ||
| // 160 /10 = 16 | ||
| }; | ||
| settingProduct(); |
There was a problem hiding this comment.
API 호출 로직이 컴포넌트 안에 있어 코드 가독성과 재사용성이 떨어집니다. 해당 호출을 별도의 함수로 분리하고, 컴포넌트 안에서는 함수를 호출하는 방식으로 작업해 보시는건 어떨까요?
| window.addEventListener("resize", () => { | ||
| setWinSize(window.innerWidth); |
There was a problem hiding this comment.
이처럼 window.addEventListener를 사용하면 컴포넌트가 리랜더링 될 때 마다 핸들러를 추가하여 성능 하락을 유발할 수 있습니다.
따라서 컴포넌트가 언마운트 될 때 클린업 함수를 추가해 주셔야 합니다.
참고
| const [startPage, setStartPage] = useState(1); | ||
| const [lastPage, setLastPage] = useState(5); | ||
| const [text, setText] = useState(""); |
There was a problem hiding this comment.
개발팀의 컨벤션 마다 차이는 있지만 통상적으로 useState는 상단에 모아서 작성해 주시는 것이 가독성을 높여줍니다. 혹시 이처럼 중간에 작성하신 이유가 있는걸까요?
| const [sellCount, setSellCount] = useState(10); | ||
| const [winSize, setWinSize] = useState(1920); | ||
|
|
||
| const arr = []; |
There was a problem hiding this comment.
arr은 이 배열이 어떤 역할을 하는지 전달하기에 적합한 변수명은 아닌 것 같습니다. 페이지네이션을 위한 배열로 추측되는데 pageNumbers와 같이 구체적인 변수명을 지어보는건 어떠실까요?
| for (let i = startPage; i <= lastPage; i++) { | ||
| arr.push(i); | ||
| } |
There was a problem hiding this comment.
페이지네이션을 UseEffect로 관리하면 페이지 변경 시 자동으로 번호를 갱신하고, 이로 인해 컴포넌트가 재랜더링 되므로 상태관리를 더욱 편하게 하실 수 있을 것 같습니다
| const settingItem = async () => { | ||
| const data = await getItemApi(page, sellCount, "recent"); | ||
| // console.log("aaa", data); | ||
| setItem(data.list); | ||
| }; | ||
| settingItem(); |
There was a problem hiding this comment.
중복되는 로직으로 보입니다. 별도의 함수로 분리하여 컴포넌트에서 선언하는 형식으로 하신다면 재사용성과 유지보수 측면에서 더욱 좋을 것 같습니다.
| style={{ | ||
| display: "flex", | ||
| flexDirection: "column", | ||
| ...props, |
There was a problem hiding this comment.
props를 스타일에 직접 주입하는것은 위험합니다. style에서 사용할 수 없는 props가 넘어올 경우 에러가 발생할수도 있습니다. 만일 syle을 확장할 의도라면 props.style과 같은 매개변수를 이용하고, rest arg는 div에 주입하는 것이 안전합니다.
또한 CSS 를 별도로 관리하고 있기에 해당 스타일도 CSS 모듈을 사용하시는 것이 일관성 있는 관리 방식이 될거라 생각합니다.
coldplay126
merged commit b10fbc3
into
codeit-sprint-fullstack:react-강수정
기본
중고마켓 페이지
심화
중고마켓 페이지
멘토님께