Conversation
| }; | ||
|
|
||
| const loadedItem = async () => { | ||
| const loadedItem = useCallback(async () => { |
There was a problem hiding this comment.
loadedItem 처럼 계산된 결과값 안에서 값을 불러오고 싶으면 useCallback 보다 useMemo 를 쓰는게 더 좋아보입니다.
이 부분이 함수라면 동사로 시작하는것이 좋아보여요 :) loadItems 처럼요
There was a problem hiding this comment.
loadedItem 처럼 계산된 결과값 안에서 값을 불러오고 싶으면 useCallback 보다 useMemo 를 쓰는게 더 좋아보입니다.
이 부분이 함수라면 동사로 시작하는것이 좋아보여요 :) loadItems 처럼요
loadedItem가 함수라서 useCallback을 사용했는데 useMemo 사용이 되나용??
| useEffect(() => { | ||
| const { title, description, price, tag } = formValues; | ||
| setIsActive(() => { | ||
| const active = |
There was a problem hiding this comment.
이 active 계산은 setIsActive 함수 밖에서 사용되어서 넘겨주었어도 좋았을 것 같아요. 들여쓰기가 추가되어서 가독성이 조금 떨어져 보입니다
|
|
||
| const handleAddTags = e => { | ||
| let { value } = e.target; | ||
| if (e.key === 'Enter' && tagValue !== '') { |
There was a problem hiding this comment.
이렇게 if 문 들여쓰기가 많을때에는 guard clause 패턴을 써보면 좋아요.
쉽게 말해서 if(true) return; early return 을 통해서 불필요한 조건에서 먼저 탈출하는거죠!
| let { value } = e.target; | ||
| if (e.key === 'Enter' && tagValue !== '') { | ||
| const existingTag = | ||
| tags && tags.some(tag => tag.name === tagValue.trim()); |
|
|
||
| const getLocationActive = ({ isActive, to }) => { | ||
| if ( | ||
| (location.pathname === "/items" || location.pathname === "/additem") && |
There was a problem hiding this comment.
if 문 안에 조건이 복잡하게 들어가있어요 이럴때는 위에서 isActive 로 표현해줬던 것 처럼 명확하게 변수로 이름지어 표현해주면 좋을 것 같아요!
| }; | ||
|
|
||
| useEffect(() => { | ||
| if (location.pathname === "/additem") { |
There was a problem hiding this comment.
이 경우에는 setTempLogin(location.pathname === '/additem') 과 같은 코드입니다 :)
| import Section from '../../ui/Section/Section.jsx'; | ||
| import Loading from '../../ui/Loading/Loading.jsx'; | ||
| import styles from './AllProduct.module.css'; | ||
|
|
||
| const deviceSize = { |
There was a problem hiding this comment.
이렇게 상수로 사용하는 습관 매우 좋습니다!
상수표기는 DEVICE_SIZE 의 형태로 많이 쓰이니 참고해보셔도 좋을 것 같아요 !
| @@ -29,10 +30,10 @@ export default function AllProduct() { | |||
| const [loading, setLoading] = useState(false); | |||
There was a problem hiding this comment.
페이지네이션과 한번에 컴포넌트를 사용하게 되면서 state 를 9개나 사용하게 되었어요. itemList 불러오는 로직, pagination 을 구성하는 로직을 분리해보면 좋을것 같습니다
There was a problem hiding this comment.
- 불필요한 state 를 없애고, 만약 AllProduct 에서 계산할 필요가 없는 state ex) maxPage 같은 것들은 하단 Pagination 컴포넌트 안으로 밀어넣어서 state 를 줄이면 좋을 것 같음
- keyword, order option 을 하나의 객체로 만들어서 변경하도록 하면 state 를 줄일 수 있게 되고
- custom hook 같은것들을 사용하면. (hook 함수 분리) is Loading error 데이터를 보여주는 layer의 state 들을 하나로 합쳐지도록 보이게 할 수 있어요.
요구사항
기본
배포링크 입니당
심화
주요 변경사항
스크린샷
멘토에게