[김진선] refactor/sprint5

[jinsunkimdev]

주요 변경사항

• [김진선] sprint5 #164 (comment)

전체적인 레이아웃으로 사용하기 위해서 layouts폴더 안에 따로 만들어서 사용중인 컴포넌트라서 DefaultLayout이라고 네이밍을 했었는데 Root가 더 적당한 네이밍일까요?저 같은 경우는 역할을 명확하게 알 수 있어서 DefaultLayout이라고 네이밍을 했었거든요.
네이밍 관련해서는 항상 고민이 많이 되는 것 같습니다.🤔

• [김진선] sprint5 #164 (comment)

프로젝트 최상위 위치에 .env파일을 생성해서 환경변수를 만들어줬습니다.
이름도 좀 더 명확히 하기 위해서 BASE_URL→BASE_API_URL로 바꾸어 주었습니다.

• [김진선] sprint5 #164 (comment)

useProductsPagination이라는 커스텀 훅을 하나만 만들어서 업데이트 호출순서를 제어하도록 만들어봤습니다.

• [김진선] sprint5 #164 (comment)

말씀대로 여러곳에서 재사용되는 게 아니라 Items페이지에서만 재사용되는 컴포넌트들을 공용 컴포넌트에 넣는건 프로젝트가 복잡해지면 관리하기 힘들 것 같아서 pages/Items/폴더 안에 components폴더를 만들어서 폴더 구조를 정리해봤습니다.

• [김진선] sprint5 #164 (comment)

말씀대로 useBestProducts훅 같은 경우에는단일 컴포넌트 전용이며 단순하기 때문에 직관적으로 BestProducts 내부에 로직을 통합하였습니다.

• [김진선] sprint5 #164 (comment)

Pagination 컴포넌트 외부에서 재사용될 일이 거의 없을 거라고 생각되는 상수라서 constants.js로 빼지는 않고 가르쳐주신대로 컴포넌트 파일 안에 정의하였습니다.

• [김진선] sprint5 #164 (comment)

variables.css파일에 z-index 위계를 변수화해두고 드롭다운 쪽에 z-index설정을 해주었습니다. 추후에 Portal도 사용해 보도록 노력하겠습니다.

• [김진선] sprint5 #164 (comment)

알려주신 예시를 참고해서 초기 limit값을 정해주고, resize이벤트 발생 시에 getLimitFromWindowWidth함수가 실행되도록 수정을 해보았습니다.

현재 환경에서는 100% CSR 환경이라 필요없겠지만 SSR환경을 대응해 본다면 typeof window === "undefined"조건을 사용해볼 수 있지 않을까 판단되어서 코드를 작성해 보았습니다.

이렇게 수정하면 UX 관점에서 약간 깜빡임이 생길 수 있다고 하는데 미션7에서 스켈레톤을 만들기 때문에 스켈레톤을 적용하면 괜찮을 거라고 판단했습니다.

멘토에게

• 일단은 제 나름대로 코멘트를 적어봤는데요 어떤 식으로 적으면 리뷰어가 보기 편하게 적을 수 있을지 알려주시면 감사하겠습니다.🙇🏻
• 셀프 코드 리뷰를 통해 질문 이어가겠습니다.

[addiescode-sj]

굳굳! 코드가 훨씬 더 잘 읽히네요 👍 리팩토링 수고하셨습니다!

주요 리뷰 포인트

• 편의성을 고려해 파일 위치 바꾸기
• URLSearchParams 객체 사용하기
• 메모리 누수 방지를 위해 AbortController + useRef 사용하기

[addiescode-sj]

URLSearchParams 객체 사용해서 여러개의 파라미터를 관리해보는 방법으로 바꾸면, 쿼리 스트링의 파싱, 조작, 인코딩 등의 작업을 간편하게 처리할 수 있으면서도 실수를 줄일 수 있겠죠? :)

아래 아티클 참고해보세요!
참고

[addiescode-sj]

해당 파일이 공용으로 쓰이는 파일이 아니라면 사용하는 입장에서 가장 가깝게 (찾기 쉽게) 유지해주시는 것이 좋습니다.
pages > Items > constants > products.js 로 옮겨보시는게 어떨까요?

[addiescode-sj]

이 파일도 위 코멘트 참고해서 옮겨보세요!

[addiescode-sj]

5,6 번 라인 사이 공백 한칸 띄워줄까요?

[addiescode-sj]

해당 코드의 메모리 누수를 방지하려면 항상 이전 요청을 취소하고 새 요청을 시작하도록 만들어주는게 좋은데요!
아래 예시처럼 AbortController와 useRef를 사용해보면 어떨까요?

// 컴포넌트 내부
function BestProducts({ title, itemsPerDevice }) {
  const abortControllerRef = useRef(null);

  const updateProducts = useCallback(async () => {
    try {
      // 이전 요청이 있다면 취소
      if (abortControllerRef.current) {
        abortControllerRef.current.abort();
      }
      
      // 새로운 AbortController 생성
      abortControllerRef.current = new AbortController();
      
      const data = await fetchProducts({ signal: abortControllerRef.current.signal });
      const sorted = data.sort((a, b) => b.favoriteCount - a.favoriteCount);
      const limit = getLimitFromWindowWidth(
        itemsPerDevice.desktop,
        itemsPerDevice.tablet,
        itemsPerDevice.mobile
      );
      setBestProducts(sorted.slice(0, limit));
    } catch (error) {
      if (error.name === 'AbortError') {
        // 요청이 취소된 경우 무시
        return;
      }
      // 다른 에러 처리
      console.error('Error fetching products:', error);
    }
  }, [itemsPerDevice]);

 ...
}

이렇게 하면 두가지 동작을 보장할수있어요 :)

• 새로운 요청이 시작될 때 이전 요청을 취소합니다.
• 컴포넌트가 언마운트될 때 진행 중인 요청을 취소합니다.

[addiescode-sj]

질문에 대한 답변

멘토에게

• 일단은 제 나름대로 코멘트를 적어봤는데요 어떤 식으로 적으면 리뷰어가 보기 편하게 적을 수 있을지 알려주시면 감사하겠습니다.🙇🏻
• 셀프 코드 리뷰를 통해 질문 이어가겠습니다.

리뷰를 위해서라면 주석보다는 PR 올리신 후 코멘트로 더 체크 원하시는 부분 라인 선택해서 달아주시면 좋습니다! :)