-
Notifications
You must be signed in to change notification settings - Fork 26
[서지권] Sprint 5 #104
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
[서지권] Sprint 5 #104
Conversation
|
스프리트 미션 하시느라 수고 많으셨어요. |
XSS 공격 방지를 위해 프론트와 백엔드 양쪽 모두 sanitize 처리를 해야한다고 알고 있습니다. 저는 dompurify 라이브러리를 사용했는데, 이 방식이 괜찮을지 궁금합니다. (사용한 파일: sanitize.js, ItemCard.jsx)보안까지 고려하시다니 훌륭합니다 👍👍👍👍👍 즉. 의도적으로 |
|
반응형을 구현하기 위해 useDeviceType 훅을 만들어서 화면 크기에 따라 타입을 판별하고 있습니다. 그런데 console.log를 찍어보니 화면 크기가 변경될 때마다 계속 콘솔에 출력되는 문제가 있었고, 이를 해결하기 위해 실제 크기 변화가 감지될 때만 디바운싱과 클린업을 적용했습니다. 성능이냐 vs 편의성이냐지금 상황은 사실 저울과 같기에 또 다른 솔루션은 없어보입니다. 다만,
|
| const { | ||
| products: bestProducts, | ||
| isLoading: bestProductsLoading, | ||
| isError: bestProductsError, | ||
| } = useGetBestProductData({ pageSize: bestPageSize }); | ||
|
|
||
| // 전체 상품 데이터 | ||
| const { | ||
| products: allProducts, | ||
| isLoading: allProductsLoading, | ||
| isError: allProductsError, | ||
| } = useGetProductData({ | ||
| page, | ||
| pageSize: allPageSize, | ||
| orderBy, | ||
| keyword: "", | ||
| }); | ||
|
|
||
| return ( | ||
| <> | ||
| <main className="products"> | ||
| <BestProductSection | ||
| products={bestProducts} | ||
| isLoading={bestProductsLoading} | ||
| isError={bestProductsError} | ||
| setBestPageSize={setBestPageSize} | ||
| /> | ||
| <AllProductSection | ||
| products={allProducts} | ||
| onOrderByChange={handleOrderByChange} | ||
| isLoading={allProductsLoading} | ||
| isError={allProductsError} | ||
| setAllPageSize={setAllPageSize} | ||
| /> | ||
| </main> |
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.
API 요청 시 isLoading, isError 상태를 관리해서 컴포넌트를 조건부로 렌더링하고 있습니다. 여기에 Empty 상태까지 추가하게 되면 코드가 꽤 복잡해질 것 같아서, 이런 경우 어떻게 하면 가독성 있게 관리할 수 있을지 고민입니다
크으 역시 지권님 👍👍 정말 좋은 고민입니다 !
개발자 사이에서 인기있는 도서 "클린 코드"의 저자 "로버트 마틴"도 말합니다.
"함수의 매개변수는 적을수록 좋다."
현재 지권님께서도 고민중인 사항이신 것 같아요.
지권님은 현재 Container / Presentational 패턴을 사용하고 계십니다.
Container/Presentational ?
React에서 관심사의 분리(SoC) 를 강제하는 방법은 Container/Presentational Pattern을 이용하는 방법이 있다. 이 를 통해 비즈니스 로직에서 뷰를 분리해낼 수 있다.
즉. 현재 Container(Items.jsx)에서 모든 데이터 통신을 받아 Presenter(BestProductSection)로 내려주고 있어요.
사실, Container / Presentational 는 오래된 패턴 중 하나예요. 리액트 훅이 나오면서 이러한 패턴은 많이 사용하지 않습니다.
그럼 어떻게할까?
props로 내리지 않고 훅을 사용해볼 수 있어요.
지권님은 현재 API 함수를 hook으로 잘 감싸서 상태를 관리하고 있어요. 정석적이고 훌륭한 패턴입니다.
이미 훅을 사용하고 계시기에 props로 전달하지 않고 각 컴포넌트에서 hook으로 호출하면 됩니다 !
이어서 작성해볼게요 !
| const Items = () => { | ||
| const [page, setPage] = useState("1"); | ||
| const [allPageSize, setAllPageSize] = useState("10"); | ||
| const [bestPageSize, setBestPageSize] = useState("4"); | ||
| const [orderBy, setOrderBy] = useState("recent"); | ||
|
|
||
| const handleOrderByChange = (koreanValue) => { | ||
| const orderByMap = { | ||
| 최신순: "recent", | ||
| 좋아요순: "favorite", | ||
| }; | ||
| setOrderBy(orderByMap[koreanValue] || "recent"); | ||
| }; | ||
|
|
||
| // 베스트 상품 데이터 | ||
| const { | ||
| products: bestProducts, | ||
| isLoading: bestProductsLoading, | ||
| isError: bestProductsError, | ||
| } = useGetBestProductData({ pageSize: bestPageSize }); | ||
|
|
||
| // 전체 상품 데이터 | ||
| const { | ||
| products: allProducts, | ||
| isLoading: allProductsLoading, | ||
| isError: allProductsError, | ||
| } = useGetProductData({ | ||
| page, | ||
| pageSize: allPageSize, | ||
| orderBy, | ||
| keyword: "", | ||
| }); | ||
|
|
||
| return ( | ||
| <> | ||
| <main className="products"> | ||
| <BestProductSection | ||
| products={bestProducts} | ||
| isLoading={bestProductsLoading} | ||
| isError={bestProductsError} | ||
| setBestPageSize={setBestPageSize} | ||
| /> | ||
| <AllProductSection | ||
| products={allProducts} | ||
| onOrderByChange={handleOrderByChange} | ||
| isLoading={allProductsLoading} | ||
| isError={allProductsError} | ||
| setAllPageSize={setAllPageSize} | ||
| /> | ||
| </main> |
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 Items = () => { | |
| const [page, setPage] = useState("1"); | |
| const [allPageSize, setAllPageSize] = useState("10"); | |
| const [bestPageSize, setBestPageSize] = useState("4"); | |
| const [orderBy, setOrderBy] = useState("recent"); | |
| const handleOrderByChange = (koreanValue) => { | |
| const orderByMap = { | |
| 최신순: "recent", | |
| 좋아요순: "favorite", | |
| }; | |
| setOrderBy(orderByMap[koreanValue] || "recent"); | |
| }; | |
| // 베스트 상품 데이터 | |
| const { | |
| products: bestProducts, | |
| isLoading: bestProductsLoading, | |
| isError: bestProductsError, | |
| } = useGetBestProductData({ pageSize: bestPageSize }); | |
| // 전체 상품 데이터 | |
| const { | |
| products: allProducts, | |
| isLoading: allProductsLoading, | |
| isError: allProductsError, | |
| } = useGetProductData({ | |
| page, | |
| pageSize: allPageSize, | |
| orderBy, | |
| keyword: "", | |
| }); | |
| return ( | |
| <> | |
| <main className="products"> | |
| <BestProductSection | |
| products={bestProducts} | |
| isLoading={bestProductsLoading} | |
| isError={bestProductsError} | |
| setBestPageSize={setBestPageSize} | |
| /> | |
| <AllProductSection | |
| products={allProducts} | |
| onOrderByChange={handleOrderByChange} | |
| isLoading={allProductsLoading} | |
| isError={allProductsError} | |
| setAllPageSize={setAllPageSize} | |
| /> | |
| </main> | |
| const Items = () => { | |
| const [page, setPage] = useState("1"); | |
| // 해당 코드들은 컴포넌트 내부에 작성해도 될 것 같네요 ! | |
| return ( | |
| <> | |
| <main className="products"> | |
| <BestProductSection | |
| // Props가 아닌 컴포넌트 내부로 ! | |
| /> | |
| <AllProductSection | |
| // Props가 아닌 컴포넌트 내부로 ! | |
| /> | |
| </main> |
| const BestProductSection = ({ | ||
| products, | ||
| isLoading, | ||
| isError, | ||
| setBestPageSize, | ||
| }) => { | ||
| const { isMobile, isTablet, isDesktop } = useDeviceType(); | ||
|
|
||
| useEffect(() => { | ||
| if (isDesktop) setBestPageSize("4"); | ||
| else if (isTablet) setBestPageSize("2"); | ||
| else if (isMobile) setBestPageSize("1"); | ||
| }, [isMobile, isTablet, isDesktop, setBestPageSize]); | ||
|
|
||
| return ( | ||
| <section className="best-products"> | ||
| <h1 className="best-products-title">베스트 상품</h1> | ||
| <article className="best-products-wrapper"> | ||
| {/* isLoading, isError 상태 렌더링 */} | ||
| {isLoading && <IsLoading type="BEST" />} | ||
| {isError && ( | ||
| <IsError message="베스트 상품을 불러오는데 실패했습니다." /> | ||
| )} | ||
|
|
||
| {products?.list?.map((product) => ( | ||
| <ItemCard | ||
| key={product.id} | ||
| img={product.images[0]} | ||
| type="BEST" | ||
| title={product.name} | ||
| price={product.price} | ||
| heartCount={product.favoriteCount} | ||
| /> | ||
| ))} | ||
| </article> | ||
| </section> | ||
| ); | ||
| }; |
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 BestProductSection = ({ | |
| products, | |
| isLoading, | |
| isError, | |
| setBestPageSize, | |
| }) => { | |
| const { isMobile, isTablet, isDesktop } = useDeviceType(); | |
| useEffect(() => { | |
| if (isDesktop) setBestPageSize("4"); | |
| else if (isTablet) setBestPageSize("2"); | |
| else if (isMobile) setBestPageSize("1"); | |
| }, [isMobile, isTablet, isDesktop, setBestPageSize]); | |
| return ( | |
| <section className="best-products"> | |
| <h1 className="best-products-title">베스트 상품</h1> | |
| <article className="best-products-wrapper"> | |
| {/* isLoading, isError 상태 렌더링 */} | |
| {isLoading && <IsLoading type="BEST" />} | |
| {isError && ( | |
| <IsError message="베스트 상품을 불러오는데 실패했습니다." /> | |
| )} | |
| {products?.list?.map((product) => ( | |
| <ItemCard | |
| key={product.id} | |
| img={product.images[0]} | |
| type="BEST" | |
| title={product.name} | |
| price={product.price} | |
| heartCount={product.favoriteCount} | |
| /> | |
| ))} | |
| </article> | |
| </section> | |
| ); | |
| }; | |
| const BestProductSection = () => { | |
| const [pageSize, setPageSize] = useState("4"); | |
| const { isMobile, isTablet, isDesktop } = useDeviceType(); | |
| useEffect(() => { | |
| if (isDesktop) setPageSize("4"); | |
| else if (isTablet) setPageSize("2"); | |
| else if (isMobile) setPageSize("1"); | |
| }, [isMobile, isTablet, isDesktop]); | |
| // 베스트 상품 데이터 | |
| const { | |
| products, | |
| isLoading, | |
| isError, | |
| } = useGetBestProductData({ pageSize }); | |
| return ( | |
| <section className="best-products"> | |
| <h1 className="best-products-title">베스트 상품</h1> | |
| <article className="best-products-wrapper"> | |
| {/* isLoading, isError 상태 렌더링 */} | |
| {isLoading && <IsLoading type="BEST" />} | |
| {isError && ( | |
| <IsError message="베스트 상품을 불러오는데 실패했습니다." /> | |
| )} | |
| {products?.list?.map((product) => ( | |
| <ItemCard | |
| key={product.id} | |
| img={product.images[0]} | |
| type="BEST" | |
| title={product.name} | |
| price={product.price} | |
| heartCount={product.favoriteCount} | |
| /> | |
| ))} | |
| </article> | |
| </section> | |
| ); | |
| }; |
| const AllProductSection = ({ | ||
| products, | ||
| onOrderByChange, | ||
| isLoading, | ||
| isError, | ||
| setAllPageSize, | ||
| }) => { | ||
| const { isMobile, isTablet, isDesktop } = useDeviceType(); | ||
|
|
||
| useEffect(() => { | ||
| if (isDesktop) setAllPageSize("10"); | ||
| else if (isTablet) setAllPageSize("6"); | ||
| else if (isMobile) setAllPageSize("4"); | ||
| }, [isMobile, isTablet, isDesktop, setAllPageSize]); | ||
|
|
||
| return ( | ||
| <section className="all-products"> | ||
| {/* 제목, 옵션 */} | ||
| <OptionsBox onOrderByChange={onOrderByChange} isMobile={isMobile} /> | ||
|
|
||
| {/* isLoading, isError 상태 렌더링 */} | ||
| {isLoading && <IsLoading type="ALL" />} | ||
| {isError && <IsError message="전체 상품을 불러오는데 실패했습니다." />} | ||
|
|
||
| <article className="all-products-wrapper"> | ||
| {products?.list?.map((product) => ( | ||
| <ItemCard | ||
| key={product.id} | ||
| img={product.images[0]} | ||
| type="ALL" | ||
| title={product.name} | ||
| price={product.price} | ||
| heartCount={product.favoriteCount} | ||
| /> | ||
| ))} | ||
| </article> | ||
| </section> | ||
| ); | ||
| }; |
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.
(이어서) 해당 컴포넌트 또한 props 대신 다음과 같이 작성해볼 수 있습니다 😉
| const AllProductSection = ({ | |
| products, | |
| onOrderByChange, | |
| isLoading, | |
| isError, | |
| setAllPageSize, | |
| }) => { | |
| const { isMobile, isTablet, isDesktop } = useDeviceType(); | |
| useEffect(() => { | |
| if (isDesktop) setAllPageSize("10"); | |
| else if (isTablet) setAllPageSize("6"); | |
| else if (isMobile) setAllPageSize("4"); | |
| }, [isMobile, isTablet, isDesktop, setAllPageSize]); | |
| return ( | |
| <section className="all-products"> | |
| {/* 제목, 옵션 */} | |
| <OptionsBox onOrderByChange={onOrderByChange} isMobile={isMobile} /> | |
| {/* isLoading, isError 상태 렌더링 */} | |
| {isLoading && <IsLoading type="ALL" />} | |
| {isError && <IsError message="전체 상품을 불러오는데 실패했습니다." />} | |
| <article className="all-products-wrapper"> | |
| {products?.list?.map((product) => ( | |
| <ItemCard | |
| key={product.id} | |
| img={product.images[0]} | |
| type="ALL" | |
| title={product.name} | |
| price={product.price} | |
| heartCount={product.favoriteCount} | |
| /> | |
| ))} | |
| </article> | |
| </section> | |
| ); | |
| }; | |
| const AllProductSection = () => { | |
| const [pageSize, setPageSize] = useState("10"); | |
| const [orderBy, setOrderBy] = useState("recent"); | |
| const { isMobile, isTablet, isDesktop } = useDeviceType(); | |
| useEffect(() => { | |
| if (isDesktop) setPageSize("10"); | |
| else if (isTablet) setPageSize("6"); | |
| else if (isMobile) setPageSize("4"); | |
| }, [isMobile, isTablet, isDesktop]); | |
| // 전체 상품 데이터 | |
| const { | |
| products, | |
| isLoading, | |
| isError, | |
| } = useGetProductData({ | |
| page, | |
| pageSize, | |
| orderBy, | |
| keyword: "", | |
| }); | |
| const handleOrderByChange = (koreanValue) => { | |
| const orderByMap = { | |
| 최신순: "recent", | |
| 좋아요순: "favorite", | |
| }; | |
| setOrderBy(orderByMap[koreanValue] || "recent"); | |
| }; | |
| return ( | |
| <section className="all-products"> | |
| {/* 제목, 옵션 */} | |
| <OptionsBox onOrderByChange={onOrderByChange} isMobile={isMobile} /> | |
| {/* isLoading, isError 상태 렌더링 */} | |
| {isLoading && <IsLoading type="ALL" />} | |
| {isError && <IsError message="전체 상품을 불러오는데 실패했습니다." />} | |
| <article className="all-products-wrapper"> | |
| {products?.list?.map((product) => ( | |
| <ItemCard | |
| key={product.id} | |
| img={product.images[0]} | |
| type="ALL" | |
| title={product.name} | |
| price={product.price} | |
| heartCount={product.favoriteCount} | |
| /> | |
| ))} | |
| </article> | |
| </section> | |
| ); | |
| }; |
| <img | ||
| src={clean(img) || "/ProductNullImage.png"} | ||
| alt={clean(title) || "상품 이미지"} | ||
| className={ | ||
| type === "BEST" ? "item-card-best-img" : "item-card-all-img" | ||
| } | ||
| /> |
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.
fallback에 대한 질문
아이템 리스트에서 이미지 경로가 아예 없는 경우에는 기본 이미지(ProductNullImage)로 대체하고 있는데, 이미지 경로는 있지만 실제 이미지가 깨지는 경우엔 현재 alt 텍스트만 보여지고 있습니다. 이런 경우 fallback 이미지를 어떻게 처리하는 게 좋을까요? 지금은 src={clean(img) || "/ProductNullImage.png"} 형태로 사용하고 있습니다.
<img> 태그에는 onError 속성을 사용할 수 있어요. 😉
이미지를 불러오지 못했을 때 대체 이미지를 지정하는 가장 흔한 방식입니다.
| <img | |
| src={clean(img) || "/ProductNullImage.png"} | |
| alt={clean(title) || "상품 이미지"} | |
| className={ | |
| type === "BEST" ? "item-card-best-img" : "item-card-all-img" | |
| } | |
| /> | |
| <img | |
| src={clean(img)} | |
| alt={clean(title) || "상품 이미지"} | |
| className={type === "BEST" ? "item-card-best-img" : "item-card-all-img"} | |
| onError={(e) => { | |
| e.currentTarget.src = "/ProductNullImage.png"; | |
| }} | |
| /> |
| const useDeviceType = () => { | ||
| const [deviceType, setDeviceType] = useState(getDeviceType()); | ||
| const deviceTypeRef = useRef(deviceType); // 이전 값을 저장해서 비교 | ||
|
|
||
| useEffect(() => { | ||
| let resizeTimeout; | ||
|
|
||
| const handleResize = () => { | ||
| clearTimeout(resizeTimeout); | ||
| resizeTimeout = setTimeout(() => { | ||
| const newType = getDeviceType(); | ||
| if (newType !== deviceTypeRef.current) { | ||
| deviceTypeRef.current = newType; | ||
| setDeviceType(newType); | ||
| } | ||
| }, 150); // 디바운싱 150ms | ||
| }; | ||
|
|
||
| window.addEventListener("resize", handleResize); | ||
| // 컴포넌트 언마운트 시 이벤트 리스너 제거 | ||
| return () => { | ||
| clearTimeout(resizeTimeout); | ||
| window.removeEventListener("resize", handleResize); | ||
| }; | ||
| }, []); | ||
|
|
||
| return { | ||
| isMobile: deviceType === "mobile", | ||
| isTablet: deviceType === "tablet", | ||
| isDesktop: deviceType === "desktop", | ||
| }; | ||
| }; |
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 fetchProducts = async () => { | ||
| try { | ||
| const response = await fetch(`${import.meta.env.VITE_API_URL}/products?${params}`, { | ||
| method: "GET", | ||
| headers: { | ||
| "Content-Type": "application/json", | ||
| }, | ||
| }); | ||
| const data = await response.json(); | ||
| setProducts(data); | ||
| } catch (err) { | ||
| setIsError(err); | ||
| console.error("전체 상품 데이터 가져오기 실패:", err); | ||
| } finally { | ||
| setIsLoading(false); | ||
| } | ||
| }; |
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.
(제안/심화) 해당 fetch 함수를 따로 분리하시는 것도 좋은 방법이 될 수 있을 것 같아요.
레이어를 한 층 더 두는 것도 좋은 설계 방법입니다 !
이는 관심사 분리에도 용이할 수 있어요.
관심사 분리(separation of concerns, SoC)는 컴퓨터 프로그램을 구별된 부분으로 분리시키는 디자인 원칙으로, 각 부문은 개개의 관심사를 해결
관심사를 분리하게되면 다음과 같은 목표를 이룰 수 있어요:
- 훅은 통신 요청에 대한 리액트의 '상태'를 다루는 데에 책임을 가지고,
fetch함수는 "데이터를 가져오는 행위(API 통신)"에 대한 책임을 가지게 됩니다. 😉
즉. 다음과 같이 함수를 분리해볼 수 있을거예요:
// apis/product.js
export async function fetchProducts({ page = "1", pageSize = "10", orderBy = "latest", keyword = "" }) {
const params = new URLSearchParams();
params.append("page", page);
params.append("pageSize", pageSize);
params.append("orderBy", orderBy);
if (keyword) params.append("keyword", keyword);
const response = await fetch(`${import.meta.env.VITE_API_URL}/products?${params}`, {
method: "GET",
headers: {
"Content-Type": "application/json",
},
});
if (!response.ok) throw new Error("상품 데이터를 불러오지 못했습니다.");
return await response.json();
}해당 함수는 순수 함수이며 리액트와 관련이 없으니
.js파일로 작성해도 되겠네요 😉
|
수고하셨습니다 지권님 ! 이번 미션도 정말 수고 많으셨어요 지권님 🎉🎉 |
요구사항
기본
베스트 상품
전체 상품
심화
스크린샷
멘토에게
XSS 공격 방지를 위해 프론트와 백엔드 양쪽 모두 sanitize 처리를 해야한다고 알고 있습니다. 저는 dompurify 라이브러리를 사용했는데, 이 방식이 괜찮을지 궁금합니다. (사용한 파일: sanitize.js, ItemCard.jsx)
API 요청 시 isLoading, isError 상태를 관리해서 컴포넌트를 조건부로 렌더링하고 있습니다. 여기에 Empty 상태까지 추가하게 되면 코드가 꽤 복잡해질 것 같아서, 이런 경우 어떻게 하면 가독성 있게 관리할 수 있을지 고민입니다. (파일: BestProductSection.jsx)
아이템 리스트에서 이미지 경로가 아예 없는 경우에는 기본 이미지(ProductNullImage)로 대체하고 있는데, 이미지 경로는 있지만 실제 이미지가 깨지는 경우엔 현재 alt 텍스트만 보여지고 있습니다. 이런 경우 fallback 이미지를 어떻게 처리하는 게 좋을까요? 지금은
src={clean(img) || "/ProductNullImage.png"}형태로 사용하고 있습니다.반응형을 구현하기 위해 useDeviceType 훅을 만들어서 화면 크기에 따라 타입을 판별하고 있습니다. 그런데 console.log를 찍어보니 화면 크기가 변경될 때마다 계속 콘솔에 출력되는 문제가 있었고, 이를 해결하기 위해 실제 크기 변화가 감지될 때만 디바운싱과 클린업을 적용했습니다.
다만 디바운싱을 적용하면서 반응 속도가 느려져 사용자 입장에서 즉각적인 피드백이 줄어드는 문제가 생겨, 오히려 UX가 나빠졌다는 생각이 들었습니다. 이런 경우엔 어떤 방식으로 개선하는 게 좋을지 궁금합니다.
셀프 코드 리뷰를 통해 질문 이어가겠습니다.