[최창환] sprint5

[3rdflr]

요구사항

기본

중고마켓

• 중고마켓 페이지 주소는 “/items” 입니다.
• 페이지 주소가 “/items” 일때 상단네비게이션바의 '중고마켓' 버튼의 색상은 “3692FF”입니다.
• 상단 네비게이션 바는 이전 미션에서 구현한 랜딩 페이지와 동일한 스타일로 만들어 주세요.
• 상품 데이터 정보는 https://panda-market-api.vercel.app/docs/#/ 에 명세된 GET 메소드 “/products” 를 사용해주세요.
• '상품 등록하기' 버튼을 누르면 “/additem” 로 이동합니다. ( 빈 페이지 )
• 전체 상품에서 드롭 다운으로 “최신 순” 또는 “좋아요 순”을 선택해서 정렬을 할 수 있습니다.

중고마켓 반응형

• 베스트 상품
• Desktop : 4개 보이기
• Tablet : 2개 보이기
• Mobile : 1개 보이기
• 전체 상품
• Desktop : 12개 보이기
• Tablet : 6개 보이기
• Mobile : 4개 보이기

심화

• 페이지 네이션 기능을 구현합니다.

스크린샷

멘토에게

• Api를 페이지네이션 때문에 App.jsx랑 ProductList 이렇게 두번 불러오는데 Context를 사용하는게 유리할까요 아니면 ProductList 안에 페이지네이션을 넣어서 Api를 한번에 불러오는게 유리할까요..?
• 셀프 코드 리뷰를 통해 질문 이어가겠습니다.

[addiescode-sj]

첫 리액트 미션 수고하셨습니다!
어렵지않게 코드 깔끔히 잘 작성하신것같아요.
몇가지 개선점들과 알아두면 좋을만한 피드백 위주로 코멘트 넣어드렸으니 참고해보세요 :)

주요 리뷰 포인트

• useEffect 훅 사용시 주의점 (불필요한 리렌더링 방지하기)
• 재사용성 높이기
• 포맷팅, 네이밍

[addiescode-sj]

import 구문들은 보통 따로 띄어쓰기를 사용해 구분해주지않습니다.
import 순서를 일괄적으로 사용하는건 협업을 가정할때 일관성 측면에서 도움이 될수도있는데 prettier 플러그인 사용하시면 이런 글 참고해서 정리해보새요 :)

참고

[addiescode-sj]

네이밍을 봤을때 handle~ 으로 시작하는건 보통 이벤트 핸들러의 함수 이름으로 많이 사용해주기때문에, 안된다는건 없지만 이 경우 좀 더 직접적이고 명확한 네이밍은 없을지 고민해보시면 좋을듯합니다. 예를 들어, searchProducts 와 같이 어떤것을 조회하는지 지어주면 함수의 이름만 봐도 어떤 일을 처리하는 함수인지 더 잘 알 수 있겠죠?

[addiescode-sj]

allProduct는 네이밍만 봐서는 data response의 일부인것처럼 느껴지네요. 전체 프로덕트의 갯수라면 allProductCount 정도로 수정해보시는건 어떨까요? :)

[addiescode-sj]

페이지네이션 관련 로직이 App.js에 있어서 재사용성도 떨어지고, 한번에 읽어야하는 코드양이 많아서 유지보수하기 쉽지 않아요. 스프린트 미션4에서 리팩토링했던것처럼 리액트에서 재사용 가능한 로직은 커스텀 훅으로, UI는 컴포넌트로 분리해볼까요? :)

[addiescode-sj]

72와 73사이에 공백 한칸 띄워주세요 :)

[addiescode-sj]

NIT: 이벤트 핸들러 네이밍의 경우 onChangeValue, 혹은 handleValueChange와 같이 이밴트 타입+타겟이 되는 매체, handle+이벤트 타겟이 되는 매체+이벤트 타입 조합 두가지중 하나로 명명해주는게 가장 일반적이예요. handle, on 두가지 종류의 접두사가 같이 들어갈 필요가 없습니다 :)

[addiescode-sj]

3,4번 사이에 공백 하나, 6,7번 사이에 공백 하나 넣어줄까요?
좋은 포맷팅 습관을 들여보면 좋을것같아요.
prettier format rule 참고해보시면서 교정해보세요! :)

참고

[addiescode-sj]

해당 useEffect는 마운트 직후 최초 한번 + currPage가 바뀔때마다 실행될거예요.
마운트 직후 최초 한번 실행될때, 다른 사이드이펙트는 없을지 + currPage가 유효한 값이 아닐때(e.g undefined, null, 0, 음수 등)는 어떻게 실행되어야할지 더 고민해보고 이런식으로 안정적이고 예측 가능한 흐름을 만들어주면 어떨까요?

예시)

useEffect(() => {
  // currPage가 유효하지 않은 경우 기본값 1 사용
  const validCurrPage = currPage && currPage > 0 ? currPage : 1;
  
  // visiblePageCount가 유효하지 않은 경우 기본값 5 사용
  const validVisiblePageCount = visiblePageCount && visiblePageCount > 0 ? visiblePageCount : 5;
  
  // 페이지 그룹 계산
  const pageGroup = Math.floor((validCurrPage - 1) / validVisiblePageCount);
  const newStartPage = pageGroup * validVisiblePageCount + 1;
  
  setStartPage(newStartPage);
}, [currPage, visiblePageCount]);

[addiescode-sj]

추가적으로, useEffect는 state setter 용도로 사용하지않으시는게 좋습니다.

useEffect는 주로 사이드 이펙트(side effects)를 다루기 위해 설계된 훅으로, 여기서의 사이드이펙트란 API 호출, DOM 직접 조작, 이벤트 리스너 등 컴포넌트의 렌더링과 직접적인 관련이 없는 작업들을 의미합니다.

즉, 단순한 계산이나 state 업데이트는 사이드 이펙트가 아니므로 useEffect로 관리하게될 시 아래와 같은 불필요한 렌더링이 발생됩니다.

1. 컴포넌트 렌더링
2. useEffect 실행
3. setState 호출
4. 다시 렌더링

(이렇게 최소 두 번의 렌더링이 발생합니다)

또한, useEffect는 비동기적으로 실행되므로, state setter 용도로 사용하게되면 state 업데이트의 타이밍을 예측하기 어렵게 만들기도합니다.

반면 useMemo나 useCallback은 렌더링 중에 동기적으로 실행되어 더 예측 가능합니다.

이러한 이유들로 인해, 단순한 계산이나 파생된 결과로 만들어지는 state를 다룰 때는 useEffect + setState 대신 useMemo나 useCallback을 사용하는 것이 더 바람직한 방법입니다.

[addiescode-sj]

저라면, 우선 이 로직을 재사용성을 위해 커스텀 훅으로 분리하는것부터 시작할거예요.
그리고 useMemo와 useCallback을 사용해 값 계산을 하고 직접적으로 계산한 값을 컴포넌트에서 쓰게 만들어볼까요?

예시)

• 페이지네이션 로직 분리

const usePagination = (currPage: number, visiblePageCount: number) => {
  const calculateStartPage = useCallback((page: number, pageCount: number) => {
    const validCurrPage = page && page > 0 ? page : 1;
    const validVisiblePageCount = pageCount && pageCount > 0 ? pageCount : 5;
    
    const pageGroup = Math.floor((validCurrPage - 1) / validVisiblePageCount);
    return pageGroup * validVisiblePageCount + 1;
  }, []);

  const startPage = useMemo(() => 
    calculateStartPage(currPage, visiblePageCount),
    [calculateStartPage, currPage, visiblePageCount]
  );

  return startPage;
};

• Pagination 컴포넌트에서 usePagination 훅 사용

const Pagination = ({ currPage, visiblePageCount }) => {
  const startPage = usePagination(currPage, visiblePageCount);
  
  return (
    <div>
      {/* startPage를 사용하는 로직 */}
    </div>
  );
};

이렇게 구조를 바꿔주면,
새로운 startPage가 의존해야하고, 올바르게 계산되게 만들어주는 currPage나 visiblePageCount가 변경되면 컴포넌트가 리렌더링됩니다.

그리고 리렌더링 시 useMemo와 useCallback이 의존성 배열의 값들을 비교하고,
의존성이 변경되었다면 새로운 값을 계산하고 UI를 갱신합니다.

즉 useEffect+ useState를 사용하는 방식과 비교하자면 두 방식 모두 원래 의도였던 UI 갱신은 동일하게 이루어지지만, 제가 제안 드린 방식이 아래와 같은 이유로 효율적입니다.

• 불필요한 state 관리를 제거하여 불필요한 렌더링 방지
• 더 예측 가능한 동작 (단방향 데이터 흐름에 어울리는 방식)
• 더 나은 성능 (계산 로직이 메모이제이션되어 성능 최적화에 도움)

[addiescode-sj]

쿼리 파라미터를 사용해 URL 쿼리 파라미터 목록이 바뀔때마다 새롭게 api를 요청하고 새로운 items를 가져올수있게끔 만들어질수있으면, UX적으로도 좋고(상품 조회에 따른 결과 페이지를 따로 공유하기 수월함) props에도 포함하지않아도 되니까 ProductList 사용 편의성도 더 괜찮아질것같긴해요.

나중에 props말고 페이지 URL의 쿼리 파라미터를 사용하는 방식으로 개선 시도 해보시죠! :)

[addiescode-sj]

재사용 가능한 로직을 커스텀훅으로 잘 분리해주셨군요! 굳굳 👍

[addiescode-sj]

질문에 대한 답변

멘토에게

• Api를 페이지네이션 때문에 App.jsx랑 ProductList 이렇게 두번 불러오는데 Context를 사용하는게 유리할까요 아니면 ProductList 안에 페이지네이션을 넣어서 Api를 한번에 불러오는게 유리할까요..?
• 셀프 코드 리뷰를 통해 질문 이어가겠습니다.

dedupeRequest를 시도하고싶으신거죠? 구조적으로 불필요하게 여러번의 요청을 하지 않아도 되는 방향으로 선택하시는게 좋습니다.
컨테이너에서 두단계 이상으로 props를 내려줘여한다면 Context를 사용해 props drilling을 방지해주시고,
props를 내려주지않아도 무방한 더 좋은 구조가 있다면 api를 필요할때 딱 한번 적절히 요청할수있도록 만들어주세요 :)