[1단계 - 장바구니] 클레어(안은소) 미션 제출합니다. #364
Conversation
Co-authored-by: 우혁 <[email protected]>
Co-authored-by: 우혁 <[email protected]>
Co-authored-by: 우혁 <[email protected]>
Co-authored-by: 우혁 <[email protected]>
Co-authored-by: 우혁 <[email protected]>
Co-authored-by: 우혁 <[email protected]>
Co-authored-by: 우혁 <[email protected]>
Co-authored-by: 우혁 <[email protected]>
Co-authored-by: 우혁 <[email protected]>
Co-authored-by: 우혁 <[email protected]>
Co-authored-by: 우혁 <[email protected]>
Co-authored-by: 우혁 <[email protected]>
Co-authored-by: 우혁 <[email protected]>
Co-authored-by: 우혁 <[email protected]>
Co-authored-by: 우혁 <[email protected]>
Co-authored-by: 우혁 <[email protected]>
Co-authored-by: 우혁 <[email protected]>
Co-authored-by: 우혁 <[email protected]>
Co-authored-by: 우혁 <[email protected]>
Co-authored-by: 우혁 <[email protected]>
Co-authored-by: 우혁 <[email protected]>
Co-authored-by: 우혁 <[email protected]>
Co-authored-by: 우혁 <[email protected]>
Co-authored-by: 우혁 <[email protected]>
Co-authored-by: 우혁 <[email protected]>
Co-authored-by: 우혁 <[email protected]>
Co-authored-by: 우혁 <[email protected]>
Co-authored-by: 우혁 <[email protected]>
Co-authored-by: 우혁 <[email protected]>
Co-authored-by: 우혁 <[email protected]>
Co-authored-by: 우혁 <[email protected]>
Co-authored-by: 우혁 <[email protected]>
Co-authored-by: 우혁 <[email protected]>
Co-authored-by: 우혁 <[email protected]>
Co-authored-by: 우혁 <[email protected]>
Co-authored-by: 우혁 <[email protected]>
Co-authored-by: 우혁 <[email protected]>
Co-authored-by: 우혁 <[email protected]>
Co-authored-by: 우혁 <[email protected]>
Co-authored-by: 우혁 <[email protected]>
|
|
||
| return ( | ||
| <S.ButtonWrapper> | ||
| {quantity === 1 ? ( |
src/pages/cart/hooks/useCartItem.ts
Outdated
| errorMessage, | ||
| refetch: refetchCartItems, | ||
| } = useFetchData({ fetchFn: getCartItems }); | ||
| const [orderList, setOrderList] = useState<CartItemType[]>([]); |
There was a problem hiding this comment.
주문 목록의 전체 선택을 관리하는 상태인 isAllChecked를 orderList라는 주문 목록 리스트가 cartItems와 일치하는지로 판단하는 파생 상태로 구현했어요. 그러나 orderList는 cartItems와 중복되는 정보를 들고 있기 때문에 SSOT를 위배된다고도 보여져요.
상품 중에서, 주문 하고 싶은 일부만 선택하는 것이기 때문에, 새로운 상태나 필드가 필요한 것은 어쩔 수 없는 것 같아요!
다만 CartItemType을 전부 관리하진 않아도 될 것 같은데요, 선택한 id만 상태로 관리해보면 어떨까요?
api 요청으로 받은 cartItems에 isChecked라는 필드를 추가하고,
이 방법도 좋을 것 같아요! 조금 조심해야 할 부분이 있는데요.
만약 또 다른 곳에서 서버 상태를 사용할 때 isChecked가 서버 타입인지 아닌지 헷갈릴 수도 있습니다.
서버 타입과 클라이언트 타입을 엄밀하게 구분해서 관리하는 것이 중요할 것 같아요!
또는 여러 곳에서 모두 isChecked가 필요하긴 한데, 서로 다른 지역 상태를 유지해야 한다면 isChecked만 SSOT에 어울리지 않을 수도 있습니다!
There was a problem hiding this comment.
말씀해주신 내용을 정리해보면, 두가지 방식이 있을것 같아용.
orderIdList로 주문할 목록의 id만 상태로 관리clientCartItems에isChecked필드 포함
두가지 방식 중에 저는 전자를 택해서 수정했어요. ( commit : 88dfaee )
🌀 여기서 한가지 궁금한 점이 있는데요!
이렇게 되면 현재 useCartItem.ts에서 서버 데이터인 cartItems와 클라이언트 데이터인 orderIdList가 함께 쓰이고 있어요. 제 생각에 서버 데이터와 클라이언트 데이터는 분리되어 있어야 할 것 같은데요.
같은 파일 내에서 쓰인다면 네이밍으로 둘을 분리하는 것도 방법일 것 같아요.
이에 대해 쵸파만의 주관이나 의견이 있는지 궁금해요! 🙋♂️🙋♂️🙋♂️
There was a problem hiding this comment.
넵! 1번 방법에서 상태 관리 관심사가 크지 않다면,
저도 그냥 같은 파일 안에서 네이밍으로 분리하는 것이 좋아보입니다~!
src/pages/cart/hooks/useCartItem.ts
Outdated
| const isFirstLoad = useRef(true); | ||
|
|
||
| useEffect(() => { | ||
| if (cartItems && isFirstLoad.current) { | ||
| setOrderList(cartItems); | ||
| isFirstLoad.current = false; | ||
| } | ||
| }, [cartItems]); |
There was a problem hiding this comment.
지금처럼 ref를 사용하거나, 다른 방법으로는 useFetchData 에게 직접 onFetch 같은 콜백을 넘겨줄 수도 있겠네요~!
There was a problem hiding this comment.
음 이 피드백 질문주신 내용이 잘 이해가 안돼요.
기존에 있던 useCartItem이 사라지면서 현재 useOrderSelection에서 useRef(true)로 첫 로드를 감지하는 방식을 사용하고 있는데요! isFirstLoad을 useRef로 관리하는 건 똑같아요.
export const useOrderSelection = (cartItems: CartItemType[] | undefined) => {
const [orderIdList, setOrderIdList] = useState<OrderItemType>([]);
const isFirstLoad = useRef(true);
useEffect(() => {
if (cartItems && cartItems.length > 0 && isFirstLoad.current) {
setOrderIdList(cartItems.map((item) => item.id));
isFirstLoad.current = false;
}
}, [cartItems]);지금처럼 ref를 쓰지않고 useFetchData에 직접 onFetch 콜백을 넘겨주는 방법은 뭔지 궁금해욥
어떻게 개선하면 좋을지 조금만 힌트를 주실 수 있을까요 ? 🥺
There was a problem hiding this comment.
이런식으로 onFetch가 있다면, 추가적인 useEffect 없이 상태를 초기화 할 수 있겠다고 생각했어요~!
const fetchCartItems = async () => {
try {
const data = await getCartItems();
setCartItems(data);
onFetch?.(data);
// onFetch: (cartItems) => { setOrderIdList(cartItems.map((item) => item.id)); }
} catch (error) {
}
};
src/pages/order/OrderSuccessPage.tsx
Outdated
| useEffect(() => { | ||
| if (!location.state) { | ||
| navigate(ROUTES.CART); | ||
| } | ||
| }, []); |
There was a problem hiding this comment.
이런 디테일도 좋네요! 조금 더 해보자면,
특정 페이지의 state를 관리하고, 필요한 state만 줄 수 있도록 유도하는 장치가 있으면 좋을 것 같아요!
There was a problem hiding this comment.
페이지의 state 관리를 위해 아래와 같이 수정해봤어요! (commit : b273ff0)
// 수정 전
const navigateToOrderSuccess = (state: OrderSuccessState) => {
navigate(ROUTES.ORDER_SUCCESS, { state });
};
// 수정 후
const navigateTo = <T extends RouteState>(route: string, state?: T) => {
navigate(route, state ? { state } : undefined);
};- usePageNavigation 커스텀 훅 추가
- 페이지 이동 시 state 타입 검증 로직 통합
- CartContent, OrderSuccessPage 네비게이션 처리 방식 통일
이에 대한 쵸파의 의견이 궁금해요..! 너무 과도한 공통화일까요?
There was a problem hiding this comment.
앗 원래 수정 전 처럼 추상화 되어있었나요? 수정 전이 안전해보입니다!
저는 특정 페이지에서 쓰이는 state 필드가 존제한다면, navigate를 할 때 그 페이지에만 필요한 state만 줄 수 있으면 좋겠다는 생각이 들었어요.
나중에 페이지가 많아지면 헷갈리기도 하고, state를 줄 때 오타가 날 수도 있어서요~!
그래서 수정 전의 모습처럼, 각각의 페이지마다 navigate 훅이 있으면 어떨지? 제안 드려봤습니다!
src/pages/cart/hooks/useCartItem.ts
Outdated
| return { | ||
| cartItems, | ||
| isLoading, | ||
| errorMessage, | ||
| refetchCartItems, | ||
| orderList, | ||
| isAllChecked, | ||
| orderTotalPrice, | ||
| toggleAllCheckBox, | ||
| addOrderItem, | ||
| removeOrderItem, | ||
| updateOrderItem, | ||
| }; |
There was a problem hiding this comment.
또한 현재 이런 상태 관리에서 단점이나 문제 상황이 존재할지도 좀 더 알고 싶어요. 우선 제 눈에는 props drilling이 제일 큰 문제인 것 같아보여요.
좋은 고민이네요! 서버 데이터 관련해서 적용 해보면 어떨까요?
어디서든 api 호출이 가능하지만, 그 호출이 특정 상태에 전반적으로 영향을 주니까요~!
이전 미션에서도 context를 넣을지 고민해보셨을 것 같은데요, 이전 미션에서는 어떻게 구분했었는지도 궁금합니다!
There was a problem hiding this comment.
우선 문제상황을 분석하고 아래와 같이 해결해봤어요.
🌀 문제 상황
-
Props Drilling 문제
// 현재: 너무 많은 props 전달 <CartList cartItems={cartItems} orderIdList={orderIdList} refetchCartItems={refetchCartItems} addOrderItemId={addOrderItemId} removeOrderItemId={removeOrderItemId} />
-
서버 데이터가 전반적으로 영향을 미침
: 개별 아이템 변경 시마다 전체 목록을 다시 가져오고, 이것이 전체 상태를 초기화시킴// 🔄 CartItem.tsx - 아이템 삭제할 때 const removeCartItem = async () => { await removeCartItemMutate(undefined); // 서버에서 1개 아이템 삭제 refetchCartItems(); // 👈 전체 목록을 다시 가져옴! }; // 🔄 QuantityButton.tsx - 수량 변경할 때 const updateQuantity = async () => { await updateCartItemMutate({id, quantity}); // 서버에서 1개 아이템 수량 변경 refetchCartItems(); // 👈 전체 목록을 다시 가져옴! };
- 불필요한 네트워크 요청: 1개만 바뀌어도 전체를 다시 가져옴
- 상태 초기화: 선택된 아이템들(orderIdList)이 리셋될 수 있음
-
useCartItem이 4가지 책임을 갖음. 책임과 역할을 적절히 분리하는것이 필요하겠다고 생각됐어요.- 서버 데이터 관리 (cartItems, isLoading, errorMessage, refetchCartItems)
- 선택 상태 관리 (orderIdList, addOrderItemId, removeOrderItemId)
- 계산 로직 (orderTotalPrice, isAllChecked)
- UI 로직 (toggleAllCheckBox)
🌀 해결 방안
폴더구조는 아래와 같이 구성하고
// 폴더 구조
src/pages/cart/
├── CartPage.tsx
├── contexts/
│ └── CartServerContext.tsx 🆕 서버 상태 관리
├── hooks/
│ ├── useOrderSelection.ts 🆕 주문 선택 상태
│ └── useOrderCalculation.ts 🆕 가격 계산 로직
└── CartContent/데이터 흐름을 아래와 같이 수정했어요.
// 변경 전
아이템 삭제 → 서버 삭제 → 전체 목록 다시 가져오기 → 모든 컴포넌트 리렌더링
// 변경 후
아이템 삭제 → 서버 삭제 → 로컬 상태만 업데이트 → 필요한 컴포넌트만 리렌더링서버 상태와 클라이언트 상태를 분리하고, 각자의 책임을 명확히 하고자 아래와 같이 상태를 분리했어요.
📦 장바구니 상태 관리
├── 🌐 CartContext (cartItems, isLoading, errorMessage) → 서버 데이터 (전역 상태로 어디서든 접근 가능, 실시간 동기화 필요)
├── ✅ useOrderSelection (orderIdList, isAllChecked)→ 클라이언트상태 (로컬)
└── 🧮 useOrderCalculation (orderTotalPrice, paymentAmount) → 계산 로직 (순수함수)결과적으로 서버 상태는 Context로, 클라이언트 상태는 명시적 props로 관리하여서 위에 언급한 문제를 해결하고자 했어요!
There was a problem hiding this comment.
여기서 드는 궁금점은 위에 orderIdList 관련하여 질문했던 것처럼 서버 상태와 클라이언트 상태를 이렇게 분리하는 것이 괜찮은지, 쵸파의 의견이 궁금해요.
두번째로 useOrderCalculation로 계산로직을 분리했는데 shared/utils로 계산로직을 빼고, 이걸 사용하고 있어요. 이 로직에 대해서도 쵸파의 의견이 궁금해요.
// 현재 : useOrderCalculation로 계산 로직 분리
const { orderTotalPrice, deliveryFee, paymentAmount } = useOrderCalculation(
cartItems,
orderIdList,
);
// shared/utils/orderPricing의 계산 로직 함수를 사용
const { deliveryFee, paymentAmount } = calculatePaymentInfo(orderTotalPrice);There was a problem hiding this comment.
- 적절한 이유로 context 사용해주신 것 같습니다! 👍 👍
- 이전에 SSOT 관련해서 말씀 드린 것처럼, 지금은 사용처가 1곳밖에 없지만, 로컬 상태를 따로 정의해주는 방법도 좋다고 생각합니다!
- 계산 부분을 utils로 뺀 것 자체는 좋아보입니다! (domain으로 봐도 괜찮겠네요)
갠적으론 함수 크기는 덜 쪼개봐도 좋아보이네요~!
| if (isLoading) { | ||
| return <div>로딩중</div>; | ||
| } | ||
|
|
||
| if (errorMessage) { | ||
| return <div>에러남</div>; | ||
| } |
There was a problem hiding this comment.
각 상황별로 잘 분리해주셨네요!
에러시에 사용자가 조치를 취할 수 있도록 조금만 더 친절하면 좋겠습니다~!
| } = useCartItem(); | ||
| const navigate = useNavigate(); | ||
|
|
||
| if (isLoading) { |
There was a problem hiding this comment.
로딩 때문에 개수를 바꿀 때마다 어쩔 수 없이 깜빡이는 것 같은데요!
첫 로딩 시에만 로딩을 보여주고, 이후에는 깜빡이지 않도록 해볼 수 있을까요? 🤔
There was a problem hiding this comment.
네! 반영해서 수정했어요! (commit : c814464)
if (isLoading && !cartItems?.length) {
return <div>로딩중</div>;
}그런데 피드백 요청한 목적에 맞는 수정방안인지 모르겠어요.
쵸파는 어떻게 생각하시나요?
There was a problem hiding this comment.
충분히 좋습니다! 👍 👍
다른 방법으로는, 첫 로딩시에만 변하는 isFirstLoading상태를 만들고, 항상 변하는 isLoading 상태도 만들 수 있겠네요!
아니면 아예 요청을 보내지 않도록 캐싱하는 방법도 생각납니다!
| } finally { | ||
| setIsLoading(false); | ||
| } | ||
| }, []); |
There was a problem hiding this comment.
어떤 상태에 의해 fetchFn의 동작이 달라진다고 해도 괜찮을까요?
There was a problem hiding this comment.
말씀해주신 피드백 관련해서 현재 상황이 변경되었는데요!
프로젝트 구조가 변경되어 useFetchData 훅 대신 CartContext를 사용하는 방식으로 리팩토링했어요. 현재는 CartContext에서 데이터 페칭과 상태 관리를 모두 담당하고 있어서, useFetchData 훅은 더 이상 사용되지 않고 있어요.
혹시 useFetchData 훅을 다시 활용하는 방향으로 검토해보면 좋을지, 아니면 현재처럼 Context 기반으로 유지하는 게 좋을지 쵸파의 의견이 궁금합니다! 😊
There was a problem hiding this comment.
이전엔 deps에 fetchFn을 넣어주지 않아서 걱정되는 상황이었는데, 지금은 괜찮겠네요!
| refetchCartItems, | ||
| removeOrderItem, | ||
| }: RemoveCartItemButtonProps) { | ||
| const { mutate: removeCartItemMutate } = useMutation(() => deleteCartItem(cartItemId)); |
There was a problem hiding this comment.
훅으로 정의 해주신 만큼, isLoading 상태도 활용해보면 좋을 것 같아요~!
There was a problem hiding this comment.
아 윗 코멘트를 반영하면서 현재 RemoveCartItemButton 컴포넌트는 useMutation 훅을 사용하는 방식에서 CartContext를 사용하는 방식으로 변경됐어요.
// 수정 전
const { mutate: removeCartItemMutate } = useMutation(() => deleteCartItem(cartItemId));
// 수전 후
const { removeItem } = useCartContext();useMutation 훅도 useFetchData와 마찬가지로 프로젝트 구조가 Context 기반으로 변경되면서 더 이상 사용되지 않고 있어요. 따라서 isLoading 상태를 활용하는 것에 대한 피드백을 반영하기 위해서는 현재 Context 구조에 맞는 다른 접근이 필요할 것 같습니다.
혹시 Context 기반의 현재 구조에서 로딩 상태를 어떻게 관리하면 좋을지 힌트를 주실 수 있을까요? 👀👀
There was a problem hiding this comment.
메서드 + url을 키로 활용해서 로딩 상태 Map을 관리하는 방법이 생각나네요~!
아니면 context안에서 useMutation같은 범용적인 훅을 활용하는 방법도 생각납니다!
const { mutate: update, isLoading: isLoadingUpdate } = useMutation(updateCartItem)
const { mutate: remove, isLoading: isLoadingRemove } = useMutation(deleteCartItem)
return <Provider value={{ update, isLoadingUpdate, remove, isLoadingRemove }}>{children}</Provider>
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
안녕하세요 쵸파! 이것저것 생각해볼거리가 많은 피드백을 주셔서 생각해보고 .. 반영이 조금 늦어졌습니닷 그리고 사실 저는 코멘트를 다 써놓고 한번에 우르르 코멘트 작성 버튼을 클릭하는데요 ... 코멘트를 다 써놓은 상태에서 새로고침했더니 코멘트가 전부 날아가버려서 .. 다시 쓰느라 시간이 더 걸렸네요 .. 🥲🥲🥲 (앞으로는 코멘트는 작성하면 바로바로 보내야겠다는 걸 .. 배웠습니닷 ... 🥺) 🌀 type vs interface
🌀
|
There was a problem hiding this comment.
안녕하세요 클레어~!
고민 많이 해주시고, 리팩터링 잘 해주셨네요! 👍 👍
적절하게 수정한 것이 맞는지 이에 대한 쵸파의 의견이 궁금합니다!
취향에 따라 다를 것 같아요~! 클레어가 정리해주신 것처럼 휴리스틱하게 interface를 사용할 수도 있죠!
저는 갠적으로, API 타입 정의는 확장성 보다는 엄격한 것이 안전하다고 봐서, type을 좋아합니다!
낙관적 업데이트
에러까지 고려해서 적절하게 구현 해주셨네요! 👍
최근 개발한 기억으로는, 즉각 반영이 중요한 케이스가 아니라면
기본적으론 백엔드와 sync가 일치하는 것에 집중하고 있습니다!
서비스와 그 기능에 따라서 많이 다를 것 같아요~!
다른 질문들도 코멘트로 답변 남겼습니다~!
남긴 코멘트는 2단계에서 이어서 고민해주세요!
1단계는 이만 머지하겠습니다!
2단계도 파이팅입니다~! 💪 💪
src/pages/cart/hooks/useCartItem.ts
Outdated
| errorMessage, | ||
| refetch: refetchCartItems, | ||
| } = useFetchData({ fetchFn: getCartItems }); | ||
| const [orderList, setOrderList] = useState<CartItemType[]>([]); |
There was a problem hiding this comment.
넵! 1번 방법에서 상태 관리 관심사가 크지 않다면,
저도 그냥 같은 파일 안에서 네이밍으로 분리하는 것이 좋아보입니다~!
src/pages/order/OrderSuccessPage.tsx
Outdated
| useEffect(() => { | ||
| if (!location.state) { | ||
| navigate(ROUTES.CART); | ||
| } | ||
| }, []); |
There was a problem hiding this comment.
앗 원래 수정 전 처럼 추상화 되어있었나요? 수정 전이 안전해보입니다!
저는 특정 페이지에서 쓰이는 state 필드가 존제한다면, navigate를 할 때 그 페이지에만 필요한 state만 줄 수 있으면 좋겠다는 생각이 들었어요.
나중에 페이지가 많아지면 헷갈리기도 하고, state를 줄 때 오타가 날 수도 있어서요~!
그래서 수정 전의 모습처럼, 각각의 페이지마다 navigate 훅이 있으면 어떨지? 제안 드려봤습니다!
| } finally { | ||
| setIsLoading(false); | ||
| } | ||
| }, []); |
There was a problem hiding this comment.
이전엔 deps에 fetchFn을 넣어주지 않아서 걱정되는 상황이었는데, 지금은 괜찮겠네요!
| refetchCartItems, | ||
| removeOrderItem, | ||
| }: RemoveCartItemButtonProps) { | ||
| const { mutate: removeCartItemMutate } = useMutation(() => deleteCartItem(cartItemId)); |
There was a problem hiding this comment.
메서드 + url을 키로 활용해서 로딩 상태 Map을 관리하는 방법이 생각나네요~!
아니면 context안에서 useMutation같은 범용적인 훅을 활용하는 방법도 생각납니다!
const { mutate: update, isLoading: isLoadingUpdate } = useMutation(updateCartItem)
const { mutate: remove, isLoading: isLoadingRemove } = useMutation(deleteCartItem)
return <Provider value={{ update, isLoadingUpdate, remove, isLoadingRemove }}>{children}</Provider>| } = useCartItem(); | ||
| const navigate = useNavigate(); | ||
|
|
||
| if (isLoading) { |
There was a problem hiding this comment.
충분히 좋습니다! 👍 👍
다른 방법으로는, 첫 로딩시에만 변하는 isFirstLoading상태를 만들고, 항상 변하는 isLoading 상태도 만들 수 있겠네요!
아니면 아예 요청을 보내지 않도록 캐싱하는 방법도 생각납니다!
src/pages/cart/hooks/useCartItem.ts
Outdated
| const isFirstLoad = useRef(true); | ||
|
|
||
| useEffect(() => { | ||
| if (cartItems && isFirstLoad.current) { | ||
| setOrderList(cartItems); | ||
| isFirstLoad.current = false; | ||
| } | ||
| }, [cartItems]); |
There was a problem hiding this comment.
이런식으로 onFetch가 있다면, 추가적인 useEffect 없이 상태를 초기화 할 수 있겠다고 생각했어요~!
const fetchCartItems = async () => {
try {
const data = await getCartItems();
setCartItems(data);
onFetch?.(data);
// onFetch: (cartItems) => { setOrderIdList(cartItems.map((item) => item.id)); }
} catch (error) {
}
};
src/pages/cart/hooks/useCartItem.ts
Outdated
| return { | ||
| cartItems, | ||
| isLoading, | ||
| errorMessage, | ||
| refetchCartItems, | ||
| orderList, | ||
| isAllChecked, | ||
| orderTotalPrice, | ||
| toggleAllCheckBox, | ||
| addOrderItem, | ||
| removeOrderItem, | ||
| updateOrderItem, | ||
| }; |
There was a problem hiding this comment.
- 적절한 이유로 context 사용해주신 것 같습니다! 👍 👍
- 이전에 SSOT 관련해서 말씀 드린 것처럼, 지금은 사용처가 1곳밖에 없지만, 로컬 상태를 따로 정의해주는 방법도 좋다고 생각합니다!
- 계산 부분을 utils로 뺀 것 자체는 좋아보입니다! (domain으로 봐도 괜찮겠네요)
갠적으론 함수 크기는 덜 쪼개봐도 좋아보이네요~!
안녕하세요 쵸파! 🥸
이번 상품 목록 미션의 리뷰이, 클레어입니다! 🍀
이번 미션은 써밋과 함께 페어로 진행했어요 👻
마지막 미션인 만큼 잘 하고 싶었는데 몇가지 아쉬움도 남는 것 같네요 ㅎㅎ
그럼 이번 미션 잘 부탁드립니다! 🙇♂️
📦 장바구니 미션
이번 미션을 통해 다음과 같은 학습 경험들을 쌓는 것을 목표로 합니다.
1단계
🕵️ 셀프 리뷰(Self-Review)
제출 전 체크 리스트
기능 요구 사항을 모두 구현했고, 정상적으로 동작하는지 확인했나요?
RTL 테스트 케이스를 모두 작성했나요?
배포한 데모 페이지에 정상적으로 접근할 수 있나요?
리뷰 요청 & 논의하고 싶은 내용
1) 상태 설계 의도
페프를 시작하기 전에 우선 요구사항에서 필요한 상태를 아래와 같이 설계했습니다!
Custom Hook 기반 상태 관리 계층화
상태 흐름 설계
위 상태 설계에 맞춰서 아래와 같이 컴포넌트를 분리해서 설계했습니다!
2) 이번 단계에서 가장 많이 고민했던 문제와 해결 과정에서 배운 점
🌀 컴포넌트 설계
bottom에 있는OrderConfirmButton를CartContent내부로 넣은 것이 가장 큰 변동이예요.3) 이번 리뷰를 통해 논의하고 싶은 부분
🌀 상태 관리
CartContent에서useCartItem라는 훅으로CartPage의 모든 상태와 이벤트 핸들러가 관리되고 있는데요, 너무 비대하다는 생각도 들어요. 이를 분리하는 것에 대해. 어떻게 생각하시는지, 쵸파의 의견이 궁금해요!🌀
isAllChecked의 상태 관리isAllChecked를orderList라는 주문 목록 리스트가cartItems와 일치하는지로 판단하는 파생 상태로 구현했어요. 그러나orderList는cartItems와 중복되는 정보를 들고 있기 때문에 SSOT를 위배된다고도 보여져요.isAllChecked는orderList와cartItems로 비교하는게 아니라api 요청으로 받은
cartItems에isChecked라는 필드를 추가하고,이
cartItems내부 item들을 돌면서 모두isChecked가 true일때isAllChecked가 true가 될 수 있도록 상태 관리를 했을 것 같아요 .. ! (현재는 첫번째 방식으로 구현했어요.)그럼 이번 미션 잘 부탁드립니다!
좋은 하루 보내세요 : >
✅ 리뷰어 체크 포인트
1. 클라이언트 상태관리
2. MSW/Test