[장바구니 미션 Step 2] 윤생(이윤성) 미션 제출합니다. #220

2yunseong · 2023-05-22T07:17:41Z

안녕하세요 서니!
주말은 잘 보내셨나요??
step2 미션 제출합니다!

이번 미션 소감

recoil을 통해 어떤 데이터를 recoil로 다루어야 하는지, 또 어떤 상태가 파생된 상태인지 고민하는시간을 가졌습니다.
그리고 장바구니가 등록되는 로직은 이 부분을 참고해보았습니다!
(https://recoiljs.org/docs/guides/asynchronous-data-queries/#use-a-request-id)
또한 msw를 적용해보았는데요, 잘 적용했는지 모르겠군요..!! 특히 예외처리부분을 많이 신경쓰지 못한것 같아 아쉽습니다. (많은 피드백 부탁드립니다.)

궁금한 점

아래 코멘트로 몇가지 남겨보았어요!! 감사합니다!

2yunseong

궁금한 부분 질문 남겼습니다..!

2yunseong · 2023-05-22T07:22:10Z

src/atoms/cartSelects.ts

+export const cartSelects = atom<Set<number>>({
+  key: 'cartSelects',
+  default: new Set<number>(),
+});


cartSelects에는 사용자에 의해 선택된 cartId가 담깁니다.

아래 (총 금액, 배송비, 총 주문금액) 상태는 모두 이 cartSelects에 파생된 상태라고 보았습니다.

'선택' 여부를 상태로 관리한 이유가 궁금합니다.

장바구니 선택 여부는 클라이언트만의 상태라고 생각해, 서버 상태만 받아오는 cart와는 분리하였습니다!

클라이언트만의 상태인데, 전역으로 관리해주신 이유가 있을까요? 장바구니 페이지 내에서만 지역적으로 관리가 되어도 괜찮을 정보인 것 같아서, 전역으로 관리해주신 이유도 궁금해요~

2yunseong · 2023-05-22T07:22:40Z

src/atoms/cartState.ts

+interface RequestAction {
+  action: string;
+  payload?: any;
+}


recoil 에대한 typescript를 적용하다.. 시간이 부족해 any를 적어두었습니다.. 따끔한 피드백 부탁드립니다

자수하셨군요.. 검거..ㅋㅋ

request 요청에 대해서 payload가 어떤 것이 들어올 지 모르는 경우가 있는데요, 이 경우에는 any 보다는 unknown 타입을 활용하시거나, 제네릭을 활용해주면 더 좋을 것 같아요~

get post patch delete 별로 payload를 구체화 해보았습니다!
(제네릭을 썼는데 실패해서 .. 나름 전부 명세해주는 걸로 구현해보았습니다!)

2yunseong · 2023-05-22T07:23:49Z

src/components/CartPage/CartListItem.tsx

+  const { count, setCount } = useCount(quantity);
+  const { name, imageUrl, price } = product;
+  const [check, setCheck] = useState(false);
+  const [cartSelectsState, setCartSelectsState] = useRecoilState(cartSelects);


cartSelects에 대한 리코일 상태에 대한 이름을 어떻게 지어야할지 고민이에요. State라는 말이 무언가 불용어처럼 보여서요..!

Suggested change

const [cartSelectsState, setCartSelectsState] = useRecoilState(cartSelects);

const [cartSelects, setCartSelects] = useRecoilState(cartSelectsState);

와 같은 방식은 어떨까요? 사실 사용하는 측에서는 cartSelects가 props로 받아왔는지, 리코일에서 관리하고 있는 상태인지를 꼭 알 필요는 없을 것 같아요. 윤생이 이를 별도로 네이밍하고자 하는 이유가 있을까요?

코드를 읽는 제3자로 하여금 혼란을 줄수도 있다고 판단해 질문드렸습니다!

제 문제인데, 쓰는 곳곳마다 네이밍이 겹치지 않게 즉흥적으로 사용하는 것 같아서 서니는 네이밍을 어떻게 하는지 궁금했었던 의도였습니다!

제안주신 방법 너무 좋은 것 같아요!!

2yunseong · 2023-05-22T07:25:13Z

src/components/CartPage/DeleteButton.tsx

+        const newCartSelects = new Set(Array.from(cartSelectSet));
+        newCartSelects.delete(cartId);
+        setCartSelects(newCartSelects);


Set 자료형의 불변성을 지키기 위해 항상 새로운 Set을 만들거나 배열을 만들어 새로운 Set을 할당해주는데요, 더 좋은 방법이 없을까요?

불변성을 지키기 위함이라면, 지금 윤생이 사용해주신 방법말고는 저도 떠오르는 것이 없네요 😭

sunhpark42

안녕하세요 윤생, 리뷰가 늦었습니다. 리뷰 했다고 생각했는데, 제가 놓치고 있었더라구요 ㅠㅠ 죄송합니다.
남겨주신 코멘트에 답변 및 추가 코멘트 남겼습니다. 확인 부탁드려요.
이번 미션도 고생많으셨습니다.

sunhpark42 · 2023-05-24T21:24:44Z

src/api/api.ts

+export const productsQuery = async () => {
+  const response = await fetch('/products');
+  if (!response.ok) throw new Error();
+  const data = await response.json();
+  return data;
+};


전체적으로 url을 제외하고는, 반복되는 api 요청이 있는데, 이 부분을 따로 추출해서 별도의 함수로 만들어봐도 좋을 것 같아요.

한번 해보았는데, 제가 잘 이해했을까요?
8ac5fe5

의도한 바로는 fetch 자체를 한 번 래핑해서 사용해줘도 좋을 것 같다였는데, 지금과 같이 response 를 다루어주는 방식도 괜찮은 것 같아요~ 👍

sunhpark42 · 2023-05-24T21:25:01Z

src/atoms/cartSelects.ts

+export const cartSelects = atom<Set<number>>({
+  key: 'cartSelects',
+  default: new Set<number>(),
+});


'선택' 여부를 상태로 관리한 이유가 궁금합니다.

sunhpark42 · 2023-05-24T21:25:17Z

src/atoms/cartSelects.ts

+      let price = 0;
+      if (cartItem) {
+        price = cartItem.quantity * cartItem.product.price;
+      }


let 을 이용해서 price 를 설정하고, 하단에서 변경해주시고 계시는데요, let 을 사용해주신 이유가 특별히 있을까요?

cartItem이 없으므로 방어적 코드를 작성해야 된다 생각해 if문을 통한 Type Guard로 구현하였습니다! 삼항연산자를 써도 되었을 것 같네요! 서니의 생각은 어떠신가요!

let 으로 선언한 변수는 언제 변경될지 알수 없기때문에, 조금 더 단순하게 얼리리턴을 사용해줬어도 좋을 것 같아요~

sunhpark42 · 2023-05-24T21:27:50Z

src/atoms/cartState.ts

+interface RequestAction {
+  action: string;
+  payload?: any;
+}


request 요청에 대해서 payload가 어떤 것이 들어올 지 모르는 경우가 있는데요, 이 경우에는 any 보다는 unknown 타입을 활용하시거나, 제네릭을 활용해주면 더 좋을 것 같아요~

sunhpark42 · 2023-05-24T21:28:31Z

src/atoms/cartState.ts

+});
+
+export const cartState = selectorFamily<CartType[], any>({
+  key: 'exampleCartQuery',


key가 exampleCartQuery 인 이유가 있나용?

고치지않은.. 실수입니다 ㅠ 주의하겠습니다 😭

sunhpark42 · 2023-05-24T21:45:30Z

src/components/CartPage/SelectCartItem.tsx

+  useEffect(() => {
+    if (checkAll) {
+      const newCartSelects = cart.map((cartItem: CartType) => cartItem.id);
+      setCartSelectsState(new Set(newCartSelects));
+    }
+    if (!checkAll && cartSelectsState.size === cartTotal) {
+      setCartSelectsState(new Set());
+    }
+  }, [checkAll]);


오히려 이 부분이 onClick 로직으로 빠져도 괜찮을 것 같아요.

sunhpark42 · 2023-05-24T21:49:18Z

src/mocks/data.ts

products에 대한 mockup 데이터라면, 파일 명을 보다 products 배열이라는 것을 알 수 있게 지으면 더 좋을 것 같아요.

sunhpark42 · 2023-05-24T21:50:39Z

src/mocks/rest/product.ts

+  // TODO: 에러처리 스펙에 맞게 변경
+  const responseData = product ?? {
+    errorMsg: 'no exist data',
+  };


이 부분은 정해졌을까요?

에러 발생시 body에 아무것도 안 보내는 걸로 결정났습니다!

따로 명세는 없었는데, 백엔드와 통신하면서도 400 Bad Request만 보낸다고 하더라구요!

그런데 이걸 구현할 당시에는 백엔드가 없어서 에러 메세지를 전달해주어야 되지않을까 해서 이렇게 구현하였습니다 !!

sunhpark42 · 2023-05-24T21:51:45Z

src/public/mockServiceWorker.js

src 안에 public 폴더가 또 있는 이유가 있을까요?

msw 적용하다 잘못들어갔습니다!!

앞으로 주의깊게 보는 습관을 더 자주 들어야겠습니다 😰

sunhpark42 · 2023-05-24T21:53:58Z

src/mocks/rest/product.ts

+    return res(
+      ctx.status(400),
+      ctx.json({
+        errorMsg: 'no exist data',
+      })
+    );


요런 반복되는 부분은 하나로 추출해줘도 좋을 것 같아요~

2yunseong

안녕하세요 서니!! 리뷰가 늦으셨다고 말씀하셨는데, 제가 늦게 보낸 부분도 있고, 오히려 페어기간 동안 다음 미션 집중할 수 있어서 좋았습니다!! 오늘에서야 딱 끝나서 보니까 좋네요!! 열심히 리뷰 반영 해보았습니다!

실수한 부분이나 놓친부분이 많아서 부끄럽네요 ..!!ㅎㅎ

2yunseong · 2023-05-25T03:07:52Z

src/atoms/cartSelects.ts

+export const cartSelects = atom<Set<number>>({
+  key: 'cartSelects',
+  default: new Set<number>(),
+});


장바구니 선택 여부는 클라이언트만의 상태라고 생각해, 서버 상태만 받아오는 cart와는 분리하였습니다!

2yunseong · 2023-05-25T04:21:24Z

src/atoms/cartSelects.ts

+      let price = 0;
+      if (cartItem) {
+        price = cartItem.quantity * cartItem.product.price;
+      }


cartItem이 없으므로 방어적 코드를 작성해야 된다 생각해 if문을 통한 Type Guard로 구현하였습니다! 삼항연산자를 써도 되었을 것 같네요! 서니의 생각은 어떠신가요!

2yunseong · 2023-05-25T04:22:01Z

src/atoms/cartState.ts

+});
+
+export const cartState = selectorFamily<CartType[], any>({
+  key: 'exampleCartQuery',


고치지않은.. 실수입니다 ㅠ 주의하겠습니다 😭

2yunseong · 2023-05-25T08:29:55Z

src/atoms/cartState.ts

+      switch (action) {
+        case 'GET':
+          return await cartQuery();
+        case 'POST':
+          const cartId = await postCartItemQuery(payload.productId);
+          await patchCartItemQuantityQuery(cartId, payload.quantity);
+          return await cartQuery();
+        case 'PATCH':
+          await patchCartItemQuantityQuery(payload.cartId, payload.quantity);
+          return await cartQuery();
+        case 'DELETE':
+          await deleteItemQuery(payload.cartId);
+          return await cartQuery();
+        default:
+          return await cartQuery();
+      }


GET자체가 기본동작인데, GET이 아무동작없이 마지막에 한번만 하는 것이 어색하다는 생각이 들어 다음과같이 구현하였습니다!

각 동작마다 어떤 요청이 진행되는지 명확하게 표현해주고 싶어서 다음과 같이 표현했습니다.

다시보니 DRY 원칙을 어긴것 같아요! 수정해보도록 하겠습니다.

2yunseong · 2023-05-25T09:19:15Z

src/components/CartPage/CartList.tsx

+      <Suspense fallback={<Header>든든 배송 상품 (0개)</Header>}>
+        <CartListHeader />
+      </Suspense>


약간의 편법을 사용하였는데.. pending(대기 상태) => fulfill시 깜빡이는 현상이 있어 숫자만 변경하려고 다음과같이 했습니다..
하지만 좀 더 찾아보니 useTransition 이 있어 적용했습니다!
해당 데이터가 오기 전까지 이전 데이터를 사용하는 거라, 현재 컴포넌트에 적합하다고 생각했습니다.

2yunseong · 2023-05-25T12:05:46Z

src/components/CartPage/SelectCartItem.tsx

+  const [checkAll, setCheckAll] = useState(false);
+
+  useEffect(() => {
+    setCheckAll(cartSelectsState.size === cartTotal && cartTotal !== 0);
+  }, [cartTotal, cartSelectsState]);


이것도 위의 check 상태와 같이, 불필요한 상태를 생성했다고 생각합니다!
수정하도록 하겠습니다 !!👍

2yunseong · 2023-05-25T12:08:01Z

src/mocks/rest/product.ts

+  // TODO: 에러처리 스펙에 맞게 변경
+  const responseData = product ?? {
+    errorMsg: 'no exist data',
+  };


에러 발생시 body에 아무것도 안 보내는 걸로 결정났습니다!

따로 명세는 없었는데, 백엔드와 통신하면서도 400 Bad Request만 보낸다고 하더라구요!

그런데 이걸 구현할 당시에는 백엔드가 없어서 에러 메세지를 전달해주어야 되지않을까 해서 이렇게 구현하였습니다 !!

2yunseong · 2023-05-25T12:13:17Z

src/public/mockServiceWorker.js

msw 적용하다 잘못들어갔습니다!!

앞으로 주의깊게 보는 습관을 더 자주 들어야겠습니다 😰

2yunseong · 2023-05-25T12:25:41Z

src/components/CartPage/CartListItem.tsx

+    cartRequestAction({ action: 'GET' })
+  );
+
+  let countDebounceId = useRef<NodeJS.Timeout>();


countDebounceId 를 변경하는 부분이 없는데 let 으로 지정해주신 이유가 있을까요?

그러네요..!! ref 자체가 바뀌는게 아닌 current가 변경되는건데, 이전에 그냥 변수를 사용해 id를 관리하려다 ref로 바꾸면서 const로 변경하지 못해 놓쳤습니다.

2yunseong · 2023-05-25T13:18:32Z

src/atoms/cartState.ts

+interface RequestAction {
+  action: string;
+  payload?: any;
+}


get post patch delete 별로 payload를 구체화 해보았습니다!
(제네릭을 썼는데 실패해서 .. 나름 전부 명세해주는 걸로 구현해보았습니다!)

2yunseong

안녕하세요 서니!! 리뷰가 늦으셨다고 말씀하셨는데, 제가 늦게 보낸 부분도 있고, 오히려 페어기간 동안 다음 미션 집중할 수 있어서 좋았습니다!! 오늘에서야 딱 끝나서 보니까 좋네요!! 열심히 리뷰 반영 해보았습니다!

실수한 부분이나 놓친부분이 많아서 부끄럽네요 ..!!ㅎㅎ

sunhpark42

안녕하세요 윤생 남겨주신 코멘트와, 반영된 코드 확인했습니다~ 답글 남겨둔 건 확인만 해주세요. 이번미션도 고생많으셨습니다~

sunhpark42 · 2023-05-28T05:21:49Z

src/components/CartPage/CartListItem.tsx

+  const countDebounce = useDebounce(() => {
+    setRequestAction({
+      action: 'PATCH',
+      payload: { cartId: id, quantity: count.value },
+    });
+  });

  useEffect(() => {
-    if (countDebounceId.current) {
-      clearTimeout(countDebounceId.current);
-    }
-    countDebounceId.current = setTimeout(() => {
-      setRequestAction({
-        action: 'PATCH',
-        payload: { cartId: id, quantity: count.value },
-      });
-    }, 400);
-  }, [count, setRequestAction]);
+    countDebounce();
+  }, [count]);


useEffect 부분까지 useDebounce 훅에서 처리해주고, 아이템 컴포넌트에서는 이렇게만 사용해줄 수 있어도 좋을 것 같아요~

useDebounce(() => { setRequestAction({ action: 'PATCH', payload: { cartId: id, quantity: count.value }, }); });

2yunseong added 30 commits May 18, 2023 10:43

refactor: html 템플릿 변경

d3d1dcd

refactor: test에 사용하는 local url 상수화

aa317fb

chore: cypress 버전 변경

e6c6582

refactor: 콜백을 무기명함수로 전달

2f5aecd

refactor: action 상수화

7a2c53e

refactor: ref를 hook에 포함

e599430

refactor: 불필요한 fragment tag 제거

6623dab

refactor: key name 일관성 지키기

aaffb51

refactor: 조건부 렌더링을 텍스트만 조건부로 보여지게 변경

277e196

test: custom command로 코드 축소

f08f34c

feat: 장바구니 에러 툴팁 박스 구현

5d77819

feat: 유효한 숫자가 아니면 장바구니 추가가 아니게 구현

c8310f0

chore: msw 설치

05edb5c

chore: msw 세팅

7a67983

test: 상품 조회 mock API 구현

6a9c27f

refactor: 상품 목록 api 로직 계층 분리

8a01a3d

refactor: deprecated 된 코드 제거

66a8145

test: 장바구니 mock API 구현

2993d11

feat: Error Boundary Fallback 컴포넌트 개선 중

e5f71b7

feat: 장바구니 페이지 기본 html 작성

3d7f487

fix: key 값을 전달하지 않는 버그 수정

10bf58e

style: 코드 스타일 수정

541719b

refactor: ref 대신 state로 수량 상태 관리

377c2ce

test: useCount 테스트 작성

433d43a

test: 기능 변경에 따른 e2e 테스트 수정

93693fe

feat: 마크업 추가 및 css 스타일링

ac7a505

feat: 장바구니 페이지 스타일링

8dc3109

feat: 리다이렉트 버튼 추가

9b71995

feat: msw와 연결

0be6090

feat: patch 로직 개선

dedc647

2yunseong added 3 commits May 21, 2023 20:52

refactor: 인라인 함수 분리

3d03a46

style: formatter

9edb576

refactor: 전체 선택 로직 개선

343b5bc

2yunseong commented May 22, 2023

View reviewed changes

This comment was marked as duplicate.

Sign in to view

woowahan-cron requested a review from sunhpark42 May 24, 2023 02:17

woowahan-cron assigned sunhpark42 May 24, 2023

sunhpark42 requested changes May 24, 2023

View reviewed changes

2yunseong commented May 25, 2023

View reviewed changes

2yunseong added 18 commits May 25, 2023 22:24

feat: cartlist에 transition 적용

808132e

refactor: 중복된 코드 삭제

7b82f40

refactor: recoil key 네이밍 수정

a3f8740

refactor: 불필요한 useEffect 제거

6a074fb

refactor: 파일명 의미있게 수정

976131b

chore: delete EOL

b669a03

fix: 잘못된 import 경로 변경

8f21e7b

refactor: 의미없는 코드 제거 후 인라인

f693f46

refactor: cart 네이밍 일관성 부여

fecb7c6

refactor: 불필요한 상태 제거

7c38fd7

chore: 불필요한 파일 삭제

9989497

refactor: 불필요한 컴포넌트 상태 제거

c0fc2cf

refactor: 로직 재사용화

0a51d44

refactor: debounce 로직 훅으로 분리

732c918

fix: any 타입 구체화

efe9759

refactor: api 메서드 재사용 코드 분리

8ac5fe5

refactor: 인라인 이벤트 핸들러 분리

6d3f379

refactor: useTransition 사용

ce609c9

sunhpark42 approved these changes May 28, 2023

View reviewed changes

sunhpark42 merged commit a142665 into woowacourse:2yunseong May 28, 2023

	const [cartSelectsState, setCartSelectsState] = useRecoilState(cartSelects);
	const [cartSelects, setCartSelects] = useRecoilState(cartSelectsState);

[장바구니 미션 Step 2] 윤생(이윤성) 미션 제출합니다. #220

[장바구니 미션 Step 2] 윤생(이윤성) 미션 제출합니다. #220

Conversation

2yunseong commented May 22, 2023 • edited Loading

이번 미션 소감

궁금한 점

2yunseong left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

This comment was marked as duplicate.

sunhpark42 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

2yunseong left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

2yunseong left a comment

Choose a reason for hiding this comment

sunhpark42 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

2yunseong commented May 22, 2023 •

edited

Loading