-
Notifications
You must be signed in to change notification settings - Fork 42
[임재은] Sprint6 #233
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
base: React-임재은
Are you sure you want to change the base?
The head ref may contain hidden characters: "React-\uC784\uC7AC\uC740-sprint6"
[임재은] Sprint6 #233
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
수고하셨습니다!
이전 미션 리팩토링을 통해 불필요한 state를 안쓰는 방법에 대해 이해를 확실히 하신것같아요.
코드 퀄리티도 너무 좋아졌고요 👍 굳굳!
조금만 더 다듬어보면 멋진 결과물 나올것 같네요!
주요 리뷰 포인트
- 포맷팅 개선 (import 문과 컴포넌트 시작 구문 사이에 공백 넣어주기)
- 조건식 상수화해서 사용하기 (리턴문 가독성 높이기)
- lodash 패키지 사용에 대한 피드백, debounce + transition 조합으로 UX 개선하기
- deps list가 많은 useEffect의 경우, 방어 코드 추가해주기
import SearchInput from './SearchInput'; | ||
function AllProductsSection({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
import문과 컴포넌트 시작 구문 사이에 공백 한칸 띄워주세요! :)
import ProductCard from './ProductCard'; | ||
function BestProductsSection({ bestItems }) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
여기서도 포맷팅 고쳐야겠네요 :)
// 정렬 옵션 정의 | ||
const [isOpen, setIsOpen] = useState(false); | ||
// 현재 선택된 옵션의 label을 찾기 | ||
const option = [ | ||
{ value: 'recent', label: '최신순' }, | ||
{ value: 'favorite', label: '좋아요순' }, | ||
]; | ||
|
||
const selectedOption = option.find(opt => opt.value === value)?.label; | ||
// 옵션 클릭 시 처리 | ||
const handleOptionClick = value => { | ||
setOrder(value); | ||
setIsOpen(false); | ||
}; | ||
|
||
// 드롭다운 열고 닫기 토글 | ||
const toggleDropdown = () => { | ||
setIsOpen(!isOpen); | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
굳굳! 딱 필요한 state만 쓰셨고, 주석도 꼼꼼히 달아주셨네요 👍
return ( | ||
<NavWrap> | ||
<Logo> | ||
<a href='/'> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
여기서는 Link 컴포넌트가 아닌 a 태그를 사용한 이유가 있을까요~?
<ItemImgWrapper> | ||
<ItemImage | ||
onError={imageErrLoadHandler} | ||
src={!imgSrc || errorState ? tempImage : imgSrc} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
const isImageError = !imgSrc || errorState;
조건식을 상수화해서 사용하시면 좋을것같아요!
리턴문은 항상 가독성을 중시하고, 깔끔하게 정리해 작성하는 습관을 들여봅시다! :)
const onSearchItems = debounce(e => { | ||
handlerSearchItems(e.target.value); | ||
}, 500); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
현재 throttle과 debounce 두가지 함수를 위해 lodash를 쓰고 계신데,
lodash 자체가 매우 무거운 패키지 사이즈를 가지고있기때문에 다방면에서 활용될게 아니라면 되도록 사용하지않는것이 좋고, 필요하더라도 최적화된 버전인 lodash-es를 채택해 쓰시는게 좋습니다.
그리고 디바운스 처리 시 useTransition hook을 같이 활용하면 UI가 더 매끄럽게 반응하 게끔 만들수있어요! :)
lodash import를 제거하고, 커스텀 디바운스 로직을 hook으로 만들어볼까요?
function useDebounce(fn, delay) {
const timeoutRef = useRef();
const callback = useCallback(
(...args) => {
if (timeoutRef.current) clearTimeout(timeoutRef.current);
timeoutRef.current = setTimeout(() => {
fn(...args);
}, delay);
},
[fn, delay],
);
return callback;
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
그 다음, useTransition hook을 사용해 handlerSearchItems가 트랜지션 내에서 실행되도록 개선해봅시다:
const [isPending, startTransition] = useTransition();
const debouncedSearch = useDebounce(value => {
startTransition(() => {
handlerSearchItems(value);
});
}, 500);
해당 로직에서 useTransition의 역할
일부 업데이트(비동기 작업)를 트랜지션으로 처리할 수 있게 해줍니다.
즉, 사용자의 입력에 즉각적으로 반응해야 하는 업데이트(입력창에 글씨가 바로 보이는 것)와, 조금 느려도 되는 업데이트(검색 결과 목록 갱신)의 우선순위를 재조정하고 작업을 분리하여, 시간이 오래 걸릴 수 있는 작업에 대한 우선순위가 사용자 입력 처리보다 낮아지게되니, 입력창이 버벅이지않고 즉각적으로 반응하게되어 좀더 매끄럽고 효율적으로 처리될 수 있게 만들어줍니다 :)
isPending
의 경우에는, 트랜지션이 진행 중일 때 true가 되는 값으로 이 값을 활용해 로딩 스피너를 쓰는 등 사용자에게 UI로 상태를 표시할 수 있습니다 :)
이 값을 활용해 이런식으로 처리해주면 좀더 사용자에게 각 처리 상태에 대한 피드백을 올바로 줄 수 있겠죠?
{isPending && <PendingMessage>검색 중...</PendingMessage>}
import { throttle } from "lodash"; | ||
import { useEffect, useState } from 'react'; | ||
|
||
import { throttle } from 'lodash'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
여기서도 lodash import를 없애고 커스텀 throttle 로직을 구현해봅시다!
// 검색어 / 정렬 기준 변경 시 전체 상품 다시 불러오기 | ||
useEffect(() => { | ||
if (!allPageSize) return; | ||
|
||
const loadAllProducts = async () => { | ||
const { list } = await fetchProducts({ | ||
page: currentPage, | ||
pageSize: allPageSize, | ||
orderBy: order, | ||
keyword: searchValue, | ||
}); | ||
setAllItems(list); | ||
}; | ||
|
||
loadAllProducts(); | ||
}, [order, searchValue, allPageSize, currentPage]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
해당 useEffect deps list가 조금 많아보이네요!
불필요한 fetch가 발생하지않도록 조건을 추가하는 식으로 방어 코드를 작성해주면 어떨까요?
useEffect(() => {
if (!allPageSize) return;
// 이전 파라미터 저장용 ref
const prevParams = useRef({});
const params = {
page: currentPage,
pageSize: allPageSize,
orderBy: order,
keyword: searchValue,
};
// 파라미터가 이전과 동일하면 fetch하지 않음
const isSameParams = JSON.stringify(prevParams.current) === JSON.stringify(params);
if (isSameParams) return;
prevParams.current = params;
const loadAllProducts = async () => {
const { list } = await fetchProducts(params);
setAllItems(list);
};
loadAllProducts();
}, [order, searchValue, allPageSize, currentPage]);
요구사항
기본
심화
스크린샷
멘토에게