Skip to content

[박연희] sprint5 #167

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 15 commits into
base: React-박연희
Choose a base branch
from

Conversation

pyeonh8
Copy link
Collaborator

@pyeonh8 pyeonh8 commented May 22, 2025

요구사항

기본

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

심화

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

주요 변경사항

멘토에게

  • 리액트 다루는 게 익숙하지 않은 탓에 페이지 하나 만드는 것도 정말 오래 걸리네요... 미션 4까지 완성했던 것도 리액트로 바꿔봐야 할텐데 혹시 팁이라도 있을까요?
  • 컴퍼넌트를 어느 단위?까지 쪼개야 좋을지 약간 감이 안 잡힙니다.

@pyeonh8 pyeonh8 added the 매운맛🔥 뒤는 없습니다. 그냥 필터 없이 말해주세요. 책임은 제가 집니다. label May 22, 2025
@addiescode-sj addiescode-sj self-requested a review May 23, 2025 05:01
Copy link
Collaborator

@addiescode-sj addiescode-sj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

수고하셨습니다~!
기본기가 탄탄하시네요 :)

주요 리뷰 포인트

  • 유지보수를 고려한 개발
  • 리턴문 가독성 높이기
  • 디렉토리 구조

return body;
}

export function useProductData({ page, pageSize, isPageinated = true }) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

isPageinated -> isPaginated 오타났네요!

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://marketplace.cursorapi.com/items?itemName=streetsidesoftware.code-spell-checker

code spell checker extension 설치하셔서 오타나 틀린 문법 교정 도움받아보세요! :)

Comment on lines +21 to +43
useEffect(() => {
if (!pageSize) return; // pageSize가 없으면 API를 아예 호출하지 않게 차단한다.
const fetchData = async () => {
try {
const query = isPageinated ? { page, pageSize } : { pageSize };
const data = await getProducts(query);
setProducts(data.list);
console.log("getProducts 결과:", data.list);

if (isPageinated) {
if (data.totalPages) {
setTotalPages(data.totalPages);
} else if (data.totalCount) {
setTotalPages(Math.ceil(data.totalCount / pageSize));
}
}
} catch (error) {
console.error("상품 불러오기 실패", error);
}
};

fetchData();
}, [page, pageSize, isPageinated]);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

페이지네이션 로직과 data fetching 로직을 분리해주면 코드 중복을 줄이고 재사용에 도움이 되겠죠?

pagination만 처리하는 커스텀 훅을 따로 만들고,
해당 훅에서 이 훅을 재사용해보는 구조로 바꿔봅시다 :)

예시를 보여드릴게요!

import { useState, useEffect } from 'react';

export function usePagination({ 
  fetchData, 
  pageSize, 
  initialPage = 1,
  isEnabled = true 
}) {
  const [currentPage, setCurrentPage] = useState(initialPage);
  const [totalPages, setTotalPages] = useState(1);
  const [data, setData] = useState([]);
  const [isLoading, setIsLoading] = useState(false);
  const [error, setError] = useState(null);

  useEffect(() => {
    if (!isEnabled || !pageSize) return;

    const loadData = async () => {
      setIsLoading(true);
      setError(null);
      
      try {
        const result = await fetchData({ page: currentPage, pageSize });
        setData(result.list);
        
        if (result.totalPages) {
          setTotalPages(result.totalPages);
        } else if (result.totalCount) {
          setTotalPages(Math.ceil(result.totalCount / pageSize));
        }
      } catch (err) {
        setError(err);
        console.error('데이터 로딩 실패:', err);
      } finally {
        setIsLoading(false);
      }
    };

    loadData();
  }, [currentPage, pageSize, isEnabled, fetchData]);

  const goToPage = (page) => {
    if (page >= 1 && page <= totalPages) {
      setCurrentPage(page);
    }
  };

  const nextPage = () => {
    if (currentPage < totalPages) {
      setCurrentPage(prev => prev + 1);
    }
  };

  const prevPage = () => {
    if (currentPage > 1) {
      setCurrentPage(prev => prev - 1);
    }
  };

  return {
    currentPage,
    totalPages,
    data,
    isLoading,
    error,
    goToPage,
    nextPage,
    prevPage,
    setCurrentPage
  };
}

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이제 기존의 useProductData 훅을 수정해서 새로 만든 usePagination 훅을 사용하도록 변경해볼게요.

export function useProductData({ pageSize, isPageinated = true }) {
  const {
    currentPage,
    totalPages,
    data: products,
    isLoading,
    error,
    goToPage,
    nextPage,
    prevPage,
  } = usePagination({
    fetchData: getProducts,
    pageSize,
    isEnabled: isPageinated,
  });

  return {
    products,
    currentPage,
    totalPages,
    isLoading,
    error,
    goToPage,
    nextPage,
    prevPage,
  };
}

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이렇게 구조를 바꿔주면, 이런 장점들이 생길 수 있습니다.

  • 재사용성: usePagination 훅은 어떤 종류의 데이터 페칭 함수와도 함께 사용될 수 있고, 이때 페이지네이션 로직을 재사용할 수 있습니다.
  • 일관성 향상 및 기능 확장: 일관적이고 명확한 방식으로 로딩 상태를 관리하고, 에러를 처리하고, 페이지 이동 메서드를 관리하고, 현재 페이지 상태를 관리할 수 있습니다. 또한 페이지네이션 관련 기능을 수정하고 확장하기도 용이해집니다.
  • 관심사 분리: 페이지네이션 로직이 데이터 페칭 로직과 분리되어 있어 각각의 책임이 명확해집니다.

// api 불러오기, 20개 불러온 뒤 반응형에 따라 자름
const { products } = useProductData({ pageSize: 20, isPageinated: false });

const sortedItems = [...products].sort((a, b) => b.favoriteCount - a.favoriteCount).slice(0, visibleCount);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

스프레드 연산자로 복사하신점 의도하신걸까요? 자바스크립트 공부 잘하셨네요 👍

NIT: 이미 알고계실수도 있지만 배경 설명을 좀 드려보자면,
sort() 메서드는 원본 배열을 직접 수정(mutate)하는 메서드라서 리액트의 상태 관리 원칙상 상태를 직접 수정하는 것은 피하는게 좋습니다. 따라서 원본 배열을 보존하기 위해 스프레드 연산자로 복사하는 것이 맞아요! 잘하셨어요 👍

왜 리액트를 사용할때 상태의 불변성을 유지하는게 좋은지에 대해서 더 찾아보시면 좋을것같네요 :)

참고

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이 컴포넌트에서도, 아까 페이지네이션 로직을 따로 분리한 커스텀훅을 재사용해주세요 :)

{sortedItems.map((product) => (
<li key={product.id}>
<div className="product__image">
{!product.images[0] || imgLoadStatus[product.id] === "error" ? (
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

리턴문은 최대한 깔끔히 유지할수있도록 습관을 들여보시는게 좋아요.
인라인으로 조건 평가식을 쓰고있어서 가독성이 떨어지네요!

상수로 만들어주면 어떨까요?

const isError = !product.images[0] || imgLoadStatus[product.id] === "error";

이렇게 상수를 만들어주고 리턴문에서는 해당 상수를 사용해주면, 리턴문에 작성한 주석도 없앨 수 있어요! :)

Comment on lines +11 to +16
const [sortKey, setSortKey] = useState("updatedAt");
const [page, setPage] = useState(1); //현재 페이지
const [pageSize, setPageSize] = useState(() => pageSizebyScreenWidth(window.innerWidth).all); //상품 개수

// api 불러오기
const { products, totalPages } = useProductData({ page, pageSize, isPageinated: true });
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

지금과 같이 프로덕트 리스트뿐만 아니라 페이지네이션 정렬이 필요한 모든 컴포넌트에서 해당 코드가 반복될테니, 위에서 코멘트드린것처럼 재사용을 위해 커스텀 훅으로 로직을 분리해주시면 좋겠죠? :)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이건 컴포넌트가 아닌 함수가 아닌가요? components 폴더에 들어가있으면 안되고, 확장자도 바꿔주는게 좋을것같아요.
아래 아티클 참고해보시고, 연희님이 편한 구조로 바꿔보세요! :)

참고

@addiescode-sj
Copy link
Collaborator

질문에 대한 답변

멘토에게

  • 리액트 다루는 게 익숙하지 않은 탓에 페이지 하나 만드는 것도 정말 오래 걸리네요... 미션 4까지 완성했던 것도 리액트로 바꿔봐야 할텐데 혹시 팁이라도 있을까요?
  • 컴퍼넌트를 어느 단위?까지 쪼개야 좋을지 약간 감이 안 잡힙니다.

뭐든 천천히 하나씩 시도해보시고 경험을 누적하시면 처음 시도할때보다 훨씬 수월해집니다!
원래 시간이 걸리는 일이니, 너무 조급해하지마시고 열심히 코드 작성해보고 리뷰 받아봅시다 ㅎㅎ

컴포넌트를 쪼개는 방법은 구현 의도에 따라 정말 여러가지 기준이 있을수있어서 딱 정해서 말씀드리기 어렵네요.

우선 리턴문 가독성을 생각했을때 3 depth 이상으로 들어가는것같으면 컴포넌트로 분리해주시는게 좋고
자바스크립트 리뷰해드릴때 단일 책임 원칙을 강조해드린것처럼, 컴포넌트도 너무 많은 역할을 담당하지않게끔 쪼개주시는게 좋고
컴포넌트 고유의 데이터가 필요할때 분리해주시는게 좋고
코드 스플리팅을 시도하기위해 분리해주셔도 좋고
나중에 CSR, SSR 모두를 시도할수있게끔 된다면 해당 컴포넌트를 렌더링 방식에 맞게끔 구동되도록 분리해주시는게 좋습니다.

@addiescode-sj addiescode-sj removed their assignment May 27, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
매운맛🔥 뒤는 없습니다. 그냥 필터 없이 말해주세요. 책임은 제가 집니다.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants