Conversation
React 김주동 sprint6
* First commit Sprint Mission5 on React * Sprint mission 6 commit. * Sprint Mission 6 Update 1 * update sprint mission 6 * Sprint mission 6 update commit. * delete .bak files * Update Comments * modify ItemComment
| type IconProps = { | ||
| iconComponent: React.FunctionComponent<React.SVGProps<SVGSVGElement>>; | ||
| size?: number; | ||
| fillColor?: string; | ||
| outlineColor?: string; | ||
| } |
There was a problem hiding this comment.
P2:
어떤 경우에는 type을 사용하시고, 어떤 경우에는 interface를 사용하셨는데
타입스크립트에서는 우선적으로 interface 사용을 권장하니 interface를 쓰시고 type의 특징이 필요한 경우 type을 사용하시는 것을 추천드립니다~
| const [keyword, setKeyword] = useState(""); | ||
|
|
||
| useEffect(() => { | ||
| const currentKeyword = (router.query.q as string) || ""; |
There was a problem hiding this comment.
P2:
가능하면 as 보다 다른 방식으로 사용해주세요~
q를 string으로 변환하거나 typeof를 통해 타입가드문을 추가해주시면 더 좋을 것 같습니다.
|
|
||
| const BestItemsSection: React.FC = () => { | ||
| const [itemList, setItemList] = useState<Product[]>([]); | ||
| const [pageSize, setPageSize] = useState<number | null>(null); |
There was a problem hiding this comment.
P2:
pageSize는 number보다는 더 좁은 타입인 1 | 2 | 3이라고 말할 수 있습니다.
이 경우에는 타입을 좁힐 필요가 크게 없지만 일반적으로는 타입은 좁힐 수 있다면 좁히는 것이 좋습니다.
enum PAGE_SIZE_BY_SCREEN {
PC = 3,
TABLET = 2,
MOBILE = 1,
}
const getPageSize = (width: number): PAGE_SIZE_BY_SCREEN => {
if (width < 768) return PAGE_SIZE_BY_SCREEN.MOBILE;
if (width < 1200) return PAGE_SIZE_BY_SCREEN.TABLET;
return PAGE_SIZE_BY_SCREEN.PC;
};
const BestItemsSection = () => {
const [pageSize, setPageSize] = useState<PAGE_SIZE_BY_SCREEN>();
...
}위의 예시처럼 enum으로 어떤 경우 1,2,3이 되는지를 명시해주셔도 좋습니다.
이렇게 하시면 코드의 의도가 더 명확해집니다.
| item: Product; | ||
| } | ||
|
|
||
| const ItemCard = ({ item }: ItemCardProps) => { |
There was a problem hiding this comment.
P3:
const ItemCard = ({ name, image, id }: ItemCardProps) => { ... }가 아니라 이렇게 하신 이유가 있을까요?
다른 props를 받는것도 아니라서 item이라는 객체를 받는것보다 name, image, id 이렇게 각각의 값을 전달받는게 사용하기에 더 편할 것 같아서요~
| function getLinkStyle(isActive: boolean) { | ||
| return { color: isActive ? "var(--blue)" : undefined }; | ||
| } |
There was a problem hiding this comment.
P3:
해당 함수가 실행되면 CSSProperties를 반환하니 아래처럼 반환값을 명시해주시는 것이 더 좋을 것 같습니다.
| function getLinkStyle(isActive: boolean) { | |
| return { color: isActive ? "var(--blue)" : undefined }; | |
| } | |
| function getLinkStyle(isActive: boolean): CSSProperties['color'] { | |
| return { color: isActive ? "var(--blue)" : undefined }; | |
| } |
| if (!router.isReady) return; | ||
|
|
||
| async function fetchProduct() { | ||
| if (!productId) { | ||
| setError("상품 아이디가 제공되지 않았어요."); | ||
| setIsLoading(false); | ||
| return; | ||
| } | ||
|
|
||
| setIsLoading(true); | ||
| try { | ||
| const data: Product = await getProductDetail(productId); | ||
| if (!data) { | ||
| throw new Error("해당 상품의 데이터를 찾을 수 없습니다."); | ||
| } | ||
| setProduct(data); | ||
| } catch (error) { | ||
| if (error instanceof Error) { | ||
| setError(error.message); | ||
| } else { | ||
| setError("알 수 없는 오류가 발생했습니다."); | ||
| } | ||
| } finally { | ||
| setIsLoading(false); | ||
| } | ||
| } |
There was a problem hiding this comment.
P3:
useEffect내부에 로직이 너무 길어서 가능하면 custom hook으로 분리하시면 더 가독성에 좋을 것 같아요
| const [orderBy, setOrderBy] = useState<ProductSortOption>("recent"); | ||
| const [page, setPage] = useState(1); | ||
| const [pageSize, setPageSize] = useState<number | null>(null); | ||
| const [itemList, setItemList] = useState<Product[]>([]); |
| favoriteCount: number; | ||
| } | ||
|
|
||
| const LikeButton = ({ productId, isFavorite, favoriteCount }: LikeButtonProps) => { |
There was a problem hiding this comment.
P2:
productId는 사용하지 않는 값인데 왜 받으신걸까요? 안 쓰는 값은 없는것이 더 좋을 것 같습니다.
| const handleInputChange = (e: ChangeEvent<HTMLTextAreaElement>) => { | ||
| setComment(e.target.value); | ||
| }; |
There was a problem hiding this comment.
P3:
제가 기억하기로는 debounce 함수를 만들어두셨던걸로 기억하는데
이런 경우에도 적용시켜주시면 더 좋을 것 같아요.
그렇지 않으면 사용자가 값을 입력하면 계속 리렌더링이 되니까요~
요구사항
기본
심화
주요 변경사항
스크린샷
멘토에게