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

2yunseong · 2023-05-11T08:19:00Z

안녕하세요 서니! 반갑습니다 😁
Step1 간 잘 부탁 드립니다!!

필수 요구사항

상품목록 페이지

상품 목록 페이지에 필요한 UI 마크업
header의 숫자 표시를 통해 장바구니에 담긴 품목의 갯수 표시

mock 데이터 활용

Mock 데이터를 활용하여 상품 데이터를 처리한다. 협업 미션을 고려하여 장바구니 API 예상 명세 참고
상품 목록 데이터를 현재는 서버가 없어 가짜 데이터(public/products.json)를 받아 처리하였습니다. 아래는 명세입니다.

상품 목록 조회 - 예상 응답 값

HTTP/1.1 200 OK
Content-Type: application/json

[
    {
        "id": 1,
        "name": "치킨",
        "price": 10000,
        "imageUrl": "http://example.com/chicken.jpg"
    },
    {
        "id": 2,
        "name": "피자",
        "price": 20000,
        "imageUrl": "http://example.com/pizza.jpg"
    }
]

전역 상태 관리

recoil을 사용하여 전역 상태 관리
위 필수 요구사항에서 상품 목록데이터만 가짜 데이터로 처리하라는 의미라고 들어, 장바구니는 recoil을 통해 상태관리를 진행하였습니다.

테스트 도구 선정

적합한 테스트 도구를 선택하여 사용하고, 중요한 테스트 케이스를 정의하여 테스트 진행
현재 step1 에서 가장 중요한 기능은 장바구니에 목록을 추가해 항목이 추가되고 반영되는지 확인하는 것이라 판단해 해당 기능을 e2e테스트 하였습니다.

프로그래밍 요구사항

Readability

API 요청을 처리하는 공통 함수나 커스텀 훅을 작성하여 재사용 가능하게 만든다.
API 요청을 처리하는 함수를 selector에서 사용하긴 하는데, 아직 재사용까지는 고민을 못해보았습니다.
~~- [ ] 페이지간 공통 스타일이 있는 경우 재사용한다.~~
아직 단일 페이지라서 고민하지 않은 사항입니다. 추후 고민하도록 하겠습니다!

Reusability

서버와의 통신을 담당하는 코드와 UI를 렌더링 하는 코드를 분리하여 관심사를 분리한다.
recoil의 selector를 통해 데이터를 받아오고, 로딩 중과 에러 상태를 관리하기 위해 Suspense 와 ErrorBoundary 를 사용하였습니다.
에러 처리 로직을 명확하게 작성하여 코드의 가독성을 높인다.

Performance

불필요한 상태 관리를 최소화하고, 상태 업데이트를 최적화한다.(세모)
recoil을 처음써보았는데 철학에 맞게 사용했는지 잘 모르겠네요! 이 부분 피드백 드립니다!
~~- [ ] 컴포넌트의 리렌더링을 최소화하기 위해 memoization을 적용한다.~~

궁금한 점

나중에 장바구니를 서버와 통신을 한다면, 장바구니의 상태를 관리할 때 마다 서버의 장바구니와 recoil의 상태를 동기화 시켜주어야 하는데, 이러면 POST를 할때 마다 GET을 해야할까요? 이건 어쩔수 없는 문제일까요?
selector로 초기 데이터를 받아와도 될까요? 보통은 그렇게 쓰는 것 같은데, recoil 공식 문서에는 다음과 같이 나와있어서 무언가 초기에 서버에서 받아오는 상태는 파생된 상태라고 보기 어려워서 껄끄러운 부분이 생겼습니다.

Selector는 파생된 상태(derived state)의 일부를 나타낸다.
파생된 상태를 어떤 방법으로든 주어진 상태를 수정하는 순수 함수에 전달된 상태의 결과물로 생각할 수 있다.

etc

배포 사이트

배포 사이트

시퀀스 다이어그램 - 장바구니 물품 수량 변경

컴포넌트 설계도

손으로 그려 다소 깔끔하지는 않는 점 사과드립니다 😥

상품 목록 페이지	상품 아이템

감사합니다!!

Co-authored-by: 2yunseong <2yunseong@users.noreply.github.com>

sunhpark42

안녕하세요 윤생, 리뷰가 늦었습니다. 죄송합니다. 🥲

ErrorBoundary 를 사용해보신 시도가 좋아요.👍 왜 서버 요청에 관련된 부분을 Recoil을 사용해야 하는지가 의문이라서, 리뷰에 함께 질문남겨놓았습니다. 수고하셨습니다~!

recoil을 처음써보았는데 철학에 맞게 사용했는지 잘 모르겠네요!

는 저도 안써서, 이 부분은 피드백을 드릴 수 없을 것 같습니다. 죄송합니다. 😭

나중에 장바구니를 서버와 통신을 한다면, 장바구니의 상태를 관리할 때 마다 서버의 장바구니와 recoil의 상태를 동기화 시켜주어야 하는데, 이러면 POST를 할때 마다 GET을 해야할까요? 이건 어쩔수 없는 문제일까요?

이 부분은 이제 선택의 문제인데요, 서버의 장바구니와 리코일에서 관리하고 있는 상태가 항상 같을 필요가 있는지를 먼저 고민해 볼 수 있습니다. 동기화가 정말로 중요한 페이지라고 한다면 필요한 동작이겠지만, 그렇지 않다면 어느정도는 프론트 로직을 신뢰하는 식의 방법도 있습니다.
동기화가 정말로 중요하다면 말씀 주신 POST할 때마다 GET을 하는 방식도 있을거고, 저는 그렇게 선호하는 방식은 아닌데, POST의 응답이, 업데이트 된 장바구니 목록일 수도 있어요. 이 부분은 늘, 서버와 어떻게 할 지에 달린 문제라고 생각합니다.

페이지 관련

의도한 동작일지는 모르겠지만, 장바구니에 아이템의 수량이 저장되어있다는 것을 알기가 어려운 것 같아요. 똑같은 상품을 담고, 수량을 추가한다거나 할 때 아이템의 개수가 다시 1부터 시작하거나, 이미 담긴 아이템인 경우, 수량을 추가하더라도 장바구니 수량 알림 숫자가 변경되지 않아서 당황스러울 수도 있을 것 같더라구요. 이런 부분에 대해서도 한 번 고민해주시면 좋을 것 같아요~!

sunhpark42 · 2023-05-14T14:33:06Z

src/atoms/cartState.ts

+export const totalCartCount = selector({
+  key: "totalCartState",
+  get: ({ get }) => {
+    const cart = get(cartState);
+    return cart.length;
+  },
+});


totalCartCount 를 selector를 사용해서 관리하는 이유가 있을까요?

cartState는 장바구니 상태로, 현재 유저가 선택한 장바구니 품목과 수량을 가지고 있습니다.
totalCartCount는 이 cart로 부터 파생된 상태라고 생각해, selector로 관리하였습니다!

sunhpark42 · 2023-05-14T14:43:32Z

src/atoms/productState.ts

+export const products = selector({
+  key: "products",
+  get: async () => {
+    const response = await fetch("./products.json");
+    if (!response.ok) throw new Error();
+    const data = await response.json();
+    return data;
+  },
+});


selector로 초기 데이터를 받아와도 될까요? 보통은 그렇게 쓰는 것 같은데, recoil 공식 문서에는 다음과 같이 나와있어서 무언가 초기에 서버에서 받아오는 상태는 파생된 상태라고 보기 어려워서 껄끄러운 부분이 생겼습니다.

이 부분에 대해서 말씀 주신 것 같아요. 아쉽게도 제가 recoil을 사용하지 않아서, 정확히 답변드리기 어려운 부분이 있습니다. 🥲 https://recoiljs.org/docs/guides/asynchronous-data-queries/ 리코일이 제공하는 문서를 확인하면, selector를 asynchronous data를 다루는데 사용하고 있는 것으로 보여요. 이미 확인하셨을수도 있지만, 이 문서도 함께 참고해보시면 좋을 것 같습니다.

추가로 데이터를 받아오는 것과 별개로, products를 리코일에서 따로 상태관리를 진행하지 않으셨다고 이해했는데, 이 부분이 selector로 작성된 이유가 있을까요?

recoil 비동기 예시에서 atom에 대한 의존성(currentUserIDState)을 제거하고 데이터를 fetch해오는 로직만 작성하면 사용할 수 있지 않을까 해서 사용했습니다!

recoil 사이트 예제

const currentUserNameQuery = selector({ key: 'CurrentUserName', get: async ({get}) => { const response = await myDBQuery({ // currentUserIDState에 대해 의존성을 가지지만 제 코드는 가지고 있지 않습니다. userID: get(currentUserIDState), }); return response.name; }, }); function CurrentUserInfo() { const userName = useRecoilValue(currentUserNameQuery); return <div>{userName}</div>; }

사실 명쾌한 의도라고는 말 못하겠습니다.😭 미션에서 recoil을 적용해보려 하니, 비동기 데이터에도 리코일을 적용해 데이터를 받아왔습니다.

이 부분은 결국 관리하고자 하는 데이터(값)이 서버로 부터 받아와야하는 값인 경우에 이렇게 사용해라~ 라는 가이드 정도라서, 상태로 관리하고자 하는 값이 아니라면, 비동기 동작을 꼭 recoil 로 관리하지는 않아도 될 것 같아요.

sunhpark42 · 2023-05-14T14:46:57Z

src/components/ProductListPage/ProductItem.tsx

+  name,
+  price,
+}: ProductItemProps) {
+  const quantityRef = useRef<HTMLInputElement>(null);


컴포넌트 내에서 따로 사용되고 있는 부분이 없는 것 같은데, ref를 선언해주신 이유가 있을까요?

useAddCart Hook으로 합칠수도 있겠네요..! ref까지 hook에 넣어줘야할까 라는 생각이 들어서 컴포넌트에 선언했습니다!

sunhpark42 · 2023-05-14T14:51:42Z

src/components/common/ErrorBoundary.tsx

+    if (this.state.hasError) {
+      return <h1>Something went wrong.</h1>;
+    }


문구가 보다 더 명확하게 나타나면 더 좋을 것 같습니다~ 이건 step2 진행하실때 어떻게 하면 사용자에게 더 유용한 에러메세지를 전달할 수 있을지를 고민해보시면 좋을 것 같아요:)

sunhpark42 · 2023-05-14T14:53:43Z

src/components/common/QuantityCounter.tsx

+    if (!isForwardedRef<HTMLInputElement>(quantityRef)) return;
+    if (!isRefCurrent<HTMLInputElement>(quantityRef.current)) return;


이부분에 대한 처리가 추가로 되어있는 이유가 있을까요?

먼저 forwardRef는 타입이 다음과 같이 분리됩니다.

type ForwardedRef<T> = ((instance: T | null) => void) | MutableRefObject<T | null> | null
여기서는 MutableRefObject<T | null> 를 사용하고 싶어 isForwardRef<HTMLInputElement>()로 타입을 좁혀줍니다.

그다음으로는 current를 사용하고 싶어, MutableRefObject<T | null> 에서 current에 대한 타입을 좁혀주었습니다. (current의 타입이 T 이여서, null 검사를 해주어야 했습니다.)

이부분에 대한 처리가 추가로 되어있는 이유가 있을까요?

타입검사를 && 연산으로 한번에 넣어주니 에러가 발생하였습니다! 아마 타입스크립트가 quantityRef 타입을 모르는 상태에서 current를 추론하려다보니 그랬을것이란 생각이 들었습니다.

sunhpark42 · 2023-05-14T14:58:59Z

src/hooks/useAddCart.ts

+
+    return [
+      ...prevCart.slice(0, index),
+      cartItem,


로직상으로는
A상품 1개 담기 => A상풍 5개 담기 하면 총 6개가 아니라, 5개가 담기게 될 것으로 예상이 되는데, 덮어씌우는 것이 의도된 동작일까요?

의도된 동작입니다! PR에 시퀀스 다이어그램이 미션의 (예상)명세입니다!
다음 미션에는 토스트메세지 등으로 사용자에게 알려주는 방식도 좋을 것 같네요!!

현재 명세와 다르지만 이런식으로 알려주는 것이 사용자에게 좋은 경험을 제공해줄 수 있겠네요!!

sunhpark42 · 2023-05-14T15:01:32Z

src/utils/validation.ts

+  if (e.target.value === "0") {
+    e.target.value = "1";
+  }


사용자의 동작을 변경해 주는 것도 좋지만, 0을 입력 할 때 무조건 1이 입력되고 있어서 사용자 입장에서는 당황스러운 포인트가 아닐까 합니다. 입력되는 경우는 많지 않겠지만, 사용자에게 해당 동작은 불가능하다는 메세지가 함께 노출되어도 좋을 것 같아요.

처음 생각은 0은 입력이 안되야 하니 1로 강제로 바꿔주자! 라고 생각했는데, 서니 코멘트를 보고 다시 생각해보니 당황스러운 포인트가 될 수도 있다고 생각해요. 잘못된 입력을 알려줄 수 있는 다른 수단을 고민해보아야겠네요! 감사합니다~~!

2yunseong · 2023-05-15T07:55:10Z

안녕하세요 서니! 리뷰 다시 요청 보냅니다!
반영할 것 보다 커멘트로 남긴 것이 많아서, 확인 부탁드려요.
제안해주신 UI/UX 관련한 개선사항은 step2의 구체화된 요구사항과 같이 반영해보도록 하겠습니다!

2yunseong

커멘트가 반영이 안되었었군요 ㅠㅠ 죄송합니다 😭

2yunseong · 2023-05-15T03:55:26Z

src/components/common/QuantityCounter.tsx

+    if (!isForwardedRef<HTMLInputElement>(quantityRef)) return;
+    if (!isRefCurrent<HTMLInputElement>(quantityRef.current)) return;


먼저 forwardRef는 타입이 다음과 같이 분리됩니다.

type ForwardedRef<T> = ((instance: T | null) => void) | MutableRefObject<T | null> | null
여기서는 MutableRefObject<T | null> 를 사용하고 싶어 isForwardRef<HTMLInputElement>()로 타입을 좁혀줍니다.

그다음으로는 current를 사용하고 싶어, MutableRefObject<T | null> 에서 current에 대한 타입을 좁혀주었습니다. (current의 타입이 T 이여서, null 검사를 해주어야 했습니다.)

이부분에 대한 처리가 추가로 되어있는 이유가 있을까요?

타입검사를 && 연산으로 한번에 넣어주니 에러가 발생하였습니다! 아마 타입스크립트가 quantityRef 타입을 모르는 상태에서 current를 추론하려다보니 그랬을것이란 생각이 들었습니다.

2yunseong · 2023-05-15T04:13:19Z

src/hooks/useAddCart.ts

+
+    return [
+      ...prevCart.slice(0, index),
+      cartItem,


의도된 동작입니다! PR에 시퀀스 다이어그램이 미션의 (예상)명세입니다!
다음 미션에는 토스트메세지 등으로 사용자에게 알려주는 방식도 좋을 것 같네요!!

현재 명세와 다르지만 이런식으로 알려주는 것이 사용자에게 좋은 경험을 제공해줄 수 있겠네요!!

2yunseong · 2023-05-15T05:29:07Z

src/utils/validation.ts

+  if (e.target.value === "0") {
+    e.target.value = "1";
+  }


처음 생각은 0은 입력이 안되야 하니 1로 강제로 바꿔주자! 라고 생각했는데, 서니 코멘트를 보고 다시 생각해보니 당황스러운 포인트가 될 수도 있다고 생각해요. 잘못된 입력을 알려줄 수 있는 다른 수단을 고민해보아야겠네요! 감사합니다~~!

2yunseong · 2023-05-15T05:31:02Z

src/components/ProductListPage/ProductItem.tsx

+  name,
+  price,
+}: ProductItemProps) {
+  const quantityRef = useRef<HTMLInputElement>(null);


useAddCart Hook으로 합칠수도 있겠네요..! ref까지 hook에 넣어줘야할까 라는 생각이 들어서 컴포넌트에 선언했습니다!

2yunseong · 2023-05-15T05:37:01Z

src/atoms/cartState.ts

+export const totalCartCount = selector({
+  key: "totalCartState",
+  get: ({ get }) => {
+    const cart = get(cartState);
+    return cart.length;
+  },
+});


cartState는 장바구니 상태로, 현재 유저가 선택한 장바구니 품목과 수량을 가지고 있습니다.
totalCartCount는 이 cart로 부터 파생된 상태라고 생각해, selector로 관리하였습니다!

2yunseong · 2023-05-15T06:58:47Z

src/atoms/productState.ts

+export const products = selector({
+  key: "products",
+  get: async () => {
+    const response = await fetch("./products.json");
+    if (!response.ok) throw new Error();
+    const data = await response.json();
+    return data;
+  },
+});


recoil 비동기 예시에서 atom에 대한 의존성(currentUserIDState)을 제거하고 데이터를 fetch해오는 로직만 작성하면 사용할 수 있지 않을까 해서 사용했습니다!

recoil 사이트 예제

const currentUserNameQuery = selector({ key: 'CurrentUserName', get: async ({get}) => { const response = await myDBQuery({ // currentUserIDState에 대해 의존성을 가지지만 제 코드는 가지고 있지 않습니다. userID: get(currentUserIDState), }); return response.name; }, }); function CurrentUserInfo() { const userName = useRecoilValue(currentUserNameQuery); return <div>{userName}</div>; }

사실 명쾌한 의도라고는 말 못하겠습니다.😭 미션에서 recoil을 적용해보려 하니, 비동기 데이터에도 리코일을 적용해 데이터를 받아왔습니다.

sunhpark42

안녕하세요 윤생, 커멘트 남겨주신 것들 확인했습니다. step1은 여기서 마무리 할게요! 수고하셨습니다~

NaveOWO and others added 30 commits May 9, 2023 14:27

init project

f10ba54

Co-authored-by: 2yunseong <2yunseong@users.noreply.github.com>

docs : 기능구현목록 작성

c2b3061

style : style 초기세팅

61a9f2b

style : 아이콘 추가

e4af48d

style : themeProvider 설정

ee9258e

feat : header 컴포넌트 마크업

b09dd98

feat : productItem 컴포넌트 마크업

6eaa3c1

chore : 파일 경로 변경

2efdc8d

Co-authored-by: 2yunseong <2yunseong@users.noreply.github.com>

feat : products mock데이터 생성

8d219da

feat : productList 컴포넌트 구현

e654108

feat : products 데이터를 받아오는 커스텀 훅 생성

508dcbe

feat : quantityCounter 컴포넌트 구현

a8f0525

Co-authored-by: 2yunseong <2yunseong@users.noreply.github.com>

feat : cartState recoil변수 생성

17f9131

feat : 장바구니 header 상태 동기화

92ea94f

feat : mockData 생성

e09f37a

Co-authored-by: 2yunseong <2yunseong@users.noreply.github.com>

feat : input value 수량 조절 기능 구현

be5e652

style : 불필요한 코드 제거

9103c5e

style : 이미지 추가

1513763

Co-authored-by: 2yunseong <2yunseong@users.noreply.github.com>

style : 스타일 변경

bed8f9c

feat : 장바구니에 중복된 아이템 수량 변경 로직 구현

89002df

feat : React.Suspense적용

8a3db63

Co-authored-by: 2yunseong <2yunseong@users.noreply.github.com>

feat : ErrorBoundary적용

2afa12f

Co-authored-by: 2yunseong <2yunseong@users.noreply.github.com>

feat : 로딩 컴포넌트 구현

4264933

feat : dataFetching 커스텀 훅 제거

eab58b1

refactor : 불필요한 조건부 랜더링 삭제

f9c1584

refactor : selector 네이밍 변경

c3a56f7

refactor : 유효한 숫자범위인지 판별하는 로직 분리

b43ece9

feat : counter 변경 로직 분리

5cfd152

Co-authored-by: 2yunseong <2yunseong@users.noreply.github.com>

feat : 장바구니 추가 로직 커스텀훅 분리

f9890a4

Co-authored-by: 2yunseong <2yunseong@users.noreply.github.com>

feat : 에러메세지 상수화

19a0321

NaveOWO and others added 6 commits May 11, 2023 14:17

feat : 라우팅 설정

2c192be

feat : mock데이터 업데이트

8f66acb

style : 코드 스타일 정리

3d850a6

refactor : 카운터에 유효 하지 않은 입력값 제어

972e1a0

test : cypress e2e test

21f6d02

style : 코드 스타일 정리

83ddb4e

Co-authored-by: 2yunseong <2yunseong@users.noreply.github.com>

2yunseong changed the base branch from main to 2yunseong May 11, 2023 08:19

2yunseong added 6 commits May 13, 2023 19:17

refactor: any 타입 제거

049ebc1

refactor: html 템플릿 변경

5eee8d6

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

7accc68

chore: cypress 버전 변경

818fa93

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

4bc8d6c

refactor: action 상수화

7dbbbce

sunhpark42 self-requested a review May 14, 2023 14:20

sunhpark42 self-assigned this May 14, 2023

sunhpark42 requested changes May 14, 2023

View reviewed changes

refactor: ref를 hook에 포함

539604a

2yunseong added 4 commits May 15, 2023 20:02

refactor: 불필요한 fragment tag 제거

4b39052

refactor: key name 일관성 지키기

6f7c587

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

785be8b

test: custom command로 코드 축소

a340084

2yunseong commented May 17, 2023

View reviewed changes

sunhpark42 approved these changes May 17, 2023

View reviewed changes

sunhpark42 merged commit 1540fda into woowacourse:2yunseong May 18, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

2yunseong commented May 11, 2023 •

edited

Loading

sunhpark42 left a comment

sunhpark42 May 14, 2023

2yunseong May 15, 2023

sunhpark42 May 14, 2023

sunhpark42 May 14, 2023

2yunseong May 15, 2023

sunhpark42 May 17, 2023

sunhpark42 May 14, 2023

2yunseong May 15, 2023

sunhpark42 May 14, 2023

sunhpark42 May 14, 2023

2yunseong May 15, 2023

sunhpark42 May 14, 2023

2yunseong May 15, 2023

sunhpark42 May 14, 2023

2yunseong May 15, 2023

2yunseong commented May 15, 2023

2yunseong left a comment

2yunseong May 15, 2023

2yunseong May 15, 2023

2yunseong May 15, 2023

2yunseong May 15, 2023

2yunseong May 15, 2023

2yunseong May 15, 2023

sunhpark42 left a comment

		if (!isForwardedRef<HTMLInputElement>(quantityRef)) return;
		if (!isRefCurrent<HTMLInputElement>(quantityRef.current)) return;

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

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

Conversation

2yunseong commented May 11, 2023 • edited Loading

필수 요구사항

상품목록 페이지

mock 데이터 활용

상품 목록 조회 - 예상 응답 값

전역 상태 관리

테스트 도구 선정

프로그래밍 요구사항

Readability

Reusability

Performance

궁금한 점

etc

배포 사이트

시퀀스 다이어그램 - 장바구니 물품 수량 변경

컴포넌트 설계도

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

recoil 사이트 예제

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 commented May 15, 2023

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

recoil 사이트 예제

sunhpark42 left a comment

Choose a reason for hiding this comment

2yunseong commented May 11, 2023 •

edited

Loading