Conversation
| const [page, setPage] = useState(1); | ||
| const [pageSize, setPageSize] = useState(10); |
There was a problem hiding this comment.
state 초기값을 하드코딩하는것보다는 상수로 만들어 빼두는건 어떨까요?
const INITIAL_PAGE = 1;
const INITIAL_PAGE_SIZE = 10;| return items; | ||
| } | ||
| return items.filter((item) => | ||
| item.name.toLowerCase().includes(search.toLowerCase()) |
There was a problem hiding this comment.
검색 시 대소문자 구분을 없애주시는 처리 좋네요 👍
| useEffect(() => { | ||
| if (width >= 480 && width <= 767) setPageSize(4); // Mobile | ||
| else if (width >= 768 && width <= 1199) setPageSize(6); // Tablet | ||
| else setPageSize(10); // Desktop | ||
| }, [width]); |
There was a problem hiding this comment.
해당 값들도 하드코딩된 값 그대로 사용하는것보다는 이런식으로 조건을 이해하기쉽게 상수화해주시면 유지보수에 유리하겠죠?
const isMobile = width >= 480 && width <= 767;
const isTablet = width >= 768 && width <= 1199;There was a problem hiding this comment.
그리고 deps list에 있는 width가 바뀔때마다 effect가 실행되게하기보다는,
isMobile, isTablet등의 값이 true인지 아닌지(deviceType 계산) 먼저 계산한 후,
실제 이 값이 바뀔때에만 effect가 실행될수있도록 만들어주는편이 불필요한 작업을 줄일 수 있겠네요 :)
지금 구조대로라면 width가 1px이라도 바뀐다면 effect가 실행될테니까요!
// 디바이스 타입을 계산하는 로직
const deviceType = useMemo(() => {
if (width >= 480 && width <= 767) return "mobile";
if (width >= 768 && width <= 1199) return "tablet";
return "desktop";
}, [width]);
useEffect(() => {
if (deviceType === "mobile") setPageSize(4);
else if (deviceType === "tablet") setPageSize(6);
else setPageSize(10); // desktop
}, [deviceType]);| // 검색할 때 search 값이 변하면 page를 무조건 1로 변경(만약 page7에 있었어도 1로 변경됨) | ||
| useEffect(() => { | ||
| setPage(1); | ||
| }, [search]); |
There was a problem hiding this comment.
보통은 검색 결과가 하나라도 있을 경우 검색 결과에 해당하는 컴포넌트를 따로 렌더링하거나 페이지가 전환되는 패턴을 많이 쓰기때문에, 기존 페이지를 유지할 필요가 없어요. 따라서 이렇게 UX적으로 페이지 초기화를 실행하시는건 아주 잘하셨습니다!
작성하신 로직을 조금 더 보완해보자면,
이런식으로 검색 결과에 해당하는 케이스를 따로 처리하는 분기를 작성해보는건 어떨까요?
검색 로직이 더 명확해지고, 관리도 쉬워질거예요!
예시)
<div className="all_product_list">
{visibleItems.length > 0 ? (
visibleItems.map((item) => (
<ProductForm key={item.id} item={item} type="all" />
))
) : searchQuery ? (
<div className="no_search_results">
<p>"{searchQuery}"에 대한 검색 결과가 없습니다.</p>
<p>다른 검색어를 입력해보세요.</p>
</div>
) : (
<div className="no_items">
<p>등록된 상품이 없습니다.</p>
</div>
)}
</div>| if (width >= 768 && width <= 1199) pageSize = 2; // Tablet | ||
| if (width >= 480 && width <= 767) pageSize = 1; // Mobile |
There was a problem hiding this comment.
이 컴포넌트에서도 하드코딩되어있는 값들을 상수화해볼까요?
| const [groupStart, setGroupStart] = useState(1); | ||
| const groupSize = 5; | ||
|
|
||
| //현재 페이지가 6일 때 어떤 그룹인지 계산 하면... | ||
| //page = 6, groupSize = 5 | ||
| //(6 - 1) / 5 = 1, Math.floor(1) = 1 | ||
| //1 * 5 + 1 = 6 → 6번 페이지는 6~10 그룹의 첫 번째 | ||
| useEffect(() => { | ||
| const newGroupStart = Math.floor((page - 1) / groupSize) * groupSize + 1; | ||
| setGroupStart(newGroupStart); | ||
| }, [page]); |
There was a problem hiding this comment.
useEffect에서 setter를 사용하는 구조는 이중 렌더링을 유발할 수 있고,
상태가 꼭 필요하지 않다면 계산된 값을 바로 사용하는 것이 비용면에서 더 효율적일수있어요.
이 컴포넌트에서 groupStart는 page와 groupSize로부터 항상 계산될 수 있는 값이라서 굳이 상태로 관리할 필요가 없겠죠?
따라서, 아래 코드 예시와 같이
groupStart를 상태로 두지 않고, 렌더링 시점에 계산된 값으로 대체하면서 불필요한 useEffect 제거하는 리팩토링을 해보면 좋겠네요 :)
// groupStart를 상태로 두지 않고 계산
const groupStart = Math.floor((page - 1) / groupSize) * groupSize + 1;
if (totalPages <= 1) return null;
const pages = [];
for (let i = groupStart; i < groupStart + groupSize && i <= totalPages; i++) {
pages.push(i);
}There was a problem hiding this comment.
불필요한 effect 줄이기에 대한 자세한 예시는 아래 아티클 참고해보세요 :)
| const [likes, setLikes] = useState(item.favoriteCount); | ||
| const handleLikeClick = () => { |
There was a problem hiding this comment.
8,9번 사이에 공백 한칸 넣어줄까요? 코드가 뭉쳐있어서 연관된 코드 뭉치처럼 보이는 경향이 있네요 :)
| const useWindowDimensions = () => { | ||
| const [windowDimensions, setWindowDimensions] = useState( | ||
| getWindowDimensions() | ||
| ); | ||
| useEffect(() => { | ||
| const handleResize = () => { | ||
| setWindowDimensions(getWindowDimensions()); | ||
| }; | ||
| window.addEventListener("resize", handleResize); // 컴포넌트 마운트 시 resize 이벤트 등록 | ||
| return () => window.removeEventListener("resize", handleResize); // 컴포넌트 언마운트 시 이벤트 제거 | ||
| }, []); | ||
| return windowDimensions; | ||
| }; |
There was a problem hiding this comment.
resize 이벤트가 너무 자주 일어나는것을 최적화하고싶다면 debounce를 적용해보는것도 좋은 방법입니다 :)
src/pages/Items.jsx
Outdated
There was a problem hiding this comment.
전체보기 페이지가 스펙상 없는것으로 보이는데 필요하다고 생각한 이유가 있으셨을까요~?
질문에 대한 답변
관련해서 본문에 자세히 답변드렸습니다 :)
네, useEffect는 최대한 작게 목적별로 나누어 사용하는것이 유리합니다. 각 useEffect가 언제 실행되는지 명확하게 분리되어야있어야 디버깅과 유지보수에 유리하기도하고, deps list도 목적에 맞춰 쪼개져있어야 불필요한 실행을 최대한 방지할 수 있어 성능상 이점이 있기 때문입니다 :)
서버에서부터 페이징 API를 사용하시는게 좋습니다. 이유는 지금 필요한 (화면에 표시할) 데이터는 정해져있는데도 한꺼번에 많은 데이터를 불러온다면 그만큼 네트워크 대역폭을 낭비하게되어 네트워크 전송량이 커지고 비용이 증가하며, 클라이언트측 로드 시간과 비용 (메모리 저장 이슈) 또한 증가하게됩니다. 즉 네트워크 전송량에 따라 클라이언트측 로딩 퍼포먼스 또한 좌우되고, 데이터가 늘어나도 성능이 유지되는지가 관건이기때문에 확장성또한 중요해져서 프로젝트 규모를 떠나서 항상 네트워크 대역폭 낭비를 줄이고 확장성을 고려해 준수한 성능을 유지해줘야하는게 프론트엔드 개발자의 몫이라고 생각해보시면 될 것 같습니다.
백엔드측에서 검색 엔진을 사용하거나, 다른 성능을 고려해 부가적인 처리를 한 전체 조회용 API 엔드포인트를 따로 제공해주는 경우가 일반적이고, 만약 없다면 요청을 하시면 됩니다. 3번과 같은 기술적 이유로, 페이징 처리되어있는 API(페이징 설계 의도를 가진 API)를 사용하면서 임의로 리밋 설정을 바꾸어 전체 데이터를 불러오는건 리소스 측면에서 권장되는 방식은 아닙니다.
|
요구사항
기본
중고마켓
중고마켓 반응형
심화
주요 변경사항
스크린샷
멘토에게
-1. 검색 시 페이지 초기화 관련 질문
검색어를 입력했을 때, 현재 페이지가 예를 들어 7페이지라면 검색 결과가 없어서 아무것도 표시되지 않는 문제가 있었습니다. 그래서 search가 바뀔 때 page를 1로 초기화하는 로직을 useEffect(() => setPage(1), [search])로 처리했는데요, 이런 방식이 현업에서도 일반적으로 사용되는 패턴인지, 아니면 더 나은 방식이 있는지 궁금합니다.
-2. useEffect 여러 번 사용하는 것에 대한 질문
컴포넌트 내에서 useEffect를 목적별로 나누어 여러 번 사용하는 것이 괜찮은지 궁금합니다. 예를 들어 AllProductList 컴포넌트에서 하나는 검색어 변경 시 페이지 초기화, 또 하나는 화면 너비에 따라 페이지 사이즈를 조정하는 식으로요. 이런 식으로 분리해서 여러 개 작성하는 게 일반적인지, 아니면 하나로 묶는 게 더 나은지 궁금합니다.
-3. 전체 데이터를 불러오고 slice로 자르는 방식에 대한 질문
현재는 서버에서 전체 데이터를 한 번에 받아오고, 프론트에서 slice()를 사용해 보여줄 데이터만 잘라서 보여주고 있습니다. 이 방식이 소규모 프로젝트에선 괜찮은지, 아니면 초기부터 서버 페이징 API를 활용하는 게 더 좋은지 궁금합니다.
-4. 전체 데이터를 불러오기 위한 limit 설정 관련 질문
서버에서 따로 "전체 데이터"를 한 번에 가져오는 API가 없어서, 현재는 limit: 1000처럼 (Items.jsx)임의의 큰 숫자를 넣어서 전체 데이터를 불러오고 있습니다. 이렇게 처리하는 방식이 괜찮은 건지, 아니면 일반적으로 전체 데이터를 가져올 때 더 좋은 방법이 있는지도 여쭤보고 싶습니다.