Skip to content
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

[2단계 - 상품 목록] 해리(최현웅) 미션 제출합니다. #56

Merged
merged 41 commits into from
Jun 13, 2024

Conversation

hwinkr
Copy link

@hwinkr hwinkr commented Jun 8, 2024

안녕하세요 브콜~ 🍀 그동안 잘 지내셨나요?

스텝 2 요구 사항을 완성하여 리뷰 요청 보내봅니다~ ✨

이번에도 잘 부탁드립니다!

배포 주소

🤔 학습 목표와 관련된 고민

이번 스텝 2에서는 다음과 같은 학습 목표들이 있었습니다.

React Query를 사용하여 서버 상태를 관리할 수 있다.
API 연동 과정에서 발생하는 다양한 에러 상황에 대응하고 사용자에게 피드백을 제공할 수 있다.

상품 목록
상품 목록에서 담기 버튼을 누르면, 수량을 조절할 수 있다.
2. 장바구니
장바구니 담기: 상품을 장바구니에 담는 기능을 구현하고 장바구니 상태를 업데이트한다
장바구니 모달: 장바구니 버튼을 클릭하면 모달로 장바구니에 담은 목록을 확인할 수 있어야 한다.
장바구니에서도 수량을 조절할 수 있어야 한다.

✨ React Query를 사용하여 서버 상태를 관리할 수 있다.

위 학습 목표를 읽은 후, 제가 스텝 2에서 해결해야 하는 문제에 React-Query 라이브러리를 '잘' 녹여내는 것으로 개인적인 목표를 설정했습니다.

해당 라이브러리는 이전에 사용해본 적이 없어, 막연했지만 수업 자료를 활용하고 점진적으로 저의 코드에 React-Query를 녹여내보려고 했던 것 같습니다.

예를 들어, 무한 스크롤 기능을 활용해서 상품 목록을 보여줘야할 때,

우선 스텝 1의 useFetch + useProducts의 코드를 useQuery를 사용하는 것으로 변경하고

=> 의도대로 동작하는지 확인.
=> 동작할 경우, useInfiniteQuery를 사용하는 것으로 점진적 리팩터링
=> 의도대로 동작하는지 확인 (무한 스크롤 기능 확인)
=> 동작할 경우, Suspense의 기능을 활용해 보기 위해서 useSuspenseInfiniteQuery를 사용하는 것으로 점진적 리팩터링
=> 의도대로 동작하는지 확인

위 순서대로 제가 해결해야 하는 문제에 React-Query의 기능들을 녹여내보려고 했던 것 같습니다...!

그리고. Raect-Query를 '잘' 사용하기 위해서는 클라이언트의 상태와 서버의 상태를 잘 구분하는 것이 중요하다는 것을 수업을 들으면서 알게되었고, 이번 미션에서의 클라이언트 상태와 서버의 상태를 구분지어 보기로 했습니다.

구분지을 때, 가장 도움이 됐던 기준과 생각은 '데이터의 주도권을 누가 들고있는가?' 였던 것 같아요 ㅎㅎ

아래 내용은 본격적으로 미션을 진행해 보기 전 이번 미션에서 클라이언트와 서버 상태를 나름대로 구분해봤던 내용입니다.

1. 클라이언트

  • 사용자가 현재 선택한 상품 카테고리, 가격 정렬 옵션

앱 내부에서 사용자의 특정 행동으로 부터 변할 수 있고 변경이 UI에 바로 반영되어야 하는 데이터가 클라이언트 상태가 아닐까 싶습니다.

그리고. 사용자가 현재 선택한 상품 카테고리라던가 가격 정렬 옵션은 서버로 부터 넘어오는 데이터가 아니기 때문에 즉, 클라이언트에서 Harry라는 사용자의 현재 카테고리, 가격 정렬 옵션을 서버에 GET 요청을 보내서 받아오는 것이 아니기 때문에 클라이언트 상태로 보는게 맞다고 판단했고 useState 훅을 사용해서 관리하기로 했습니다.

클라이언트에서 상품 목록 필터링 옵션 상태의 주도권을 가지고 있고, 출처도 클라이언트이다! 라고 정리했습니다.

클라이언트에서 관리하고 있는 카테고리, 정렬 옵션 상태에 따라서 상품 목록 GET 요청을 보내는 url이 달라지고,
서버는 url이 어떻게 달라지는지에 대해서 관심이 없으며 오직, 요청한 url에 따라서 클라이언트가 원하는 데이터를 넘겨줄 뿐이라고도 한 번 정리해 봤습니다 ㅎㅎ

2. 서버

  • 사용자의 장바구니에 담긴 상품의 현재 수량

사용자가 현재 장바구니에 담아둔 상품의 수량은 서버의 상태로 봐야한다고 생각했습니다.

서버에서 사용자의 상품 수량 데이터를 가지고 있을 것이며,

상품 수량을 변경하는 행동 자체는 클라이언트에서 트리거 되지만, 이 트리거된 행동(+ 버튼을 눌렀을 경우)이 결국 서버에 patch 요청을 보내서 데이터베이스에서 관리하고 있을 사용자의 상품 수량을 변경하기 때문이라고 정리해 봤습니다..

즉, 행동은 클라이언트에서 발생하지만 변경되는 데이터의 주도권은 서버이며, 해당 데이터의 주인도 서버이다! 라고 정리해 봤습니다.

위와 같은 생각들로

  • 서버에서 넘어오는 상품 목록
  • 사용자의 장바구니에 담긴 상품 목록

또한 서버의 상태로 보고, 해당 상태들은 React-Query가 제공하는 훅인 useSuspenseInfiniteQuery, useMutation으로 관리했습니다.

✨ API 연동 과정에서 발생하는 다양한 에러 상황에 대응하고 사용자에게 피드백을 제공할 수 있다.

APi 연동 과정에서 어떤 에러가 발생할 수 있을까를 고민해보았습니다.

  1. 인증된 사용자가 아닐 때,
  • 장바구니에 상품을 담으려고 하는 경우
  • 장바구니에 담긴 상품 수량을 변경하려고 하는 경우
  • 장바구니에 담긴 상품을 삭제하려고 하는 경우
  1. 네트워크 OFF 상태일 때, 클라이언트에서 페칭을 요청하는 경우

  2. 서버에서 알 수 없는 예외를 던지는 경우 (500)

위와 같은 경우들이 있다고 판단했고, 이에 대한 예외를 스텝 1처럼 토스트 메시지로 사용자에게 피드백을 전달해줄 수 있도록 구현했습니다.

🙋‍♂️ 질문

  1. 스텝 1에서는 사용하지 않았던 라이브러리인 React-Query의 사용성

스텝 1에서는 사용해보지 않았었고, 우테코 크루가 되기 전에도 사용해 본 적이 없던 라이브러리인 React-Query를 도입해 보려고하니 너무 막연하더라구요.

위에서 설명드린 것처럼, 점진적으로 적용해보려고 했으나, 과연 적절하게 적용한 것인지에 대한 피드백을 받아보고 싶습니다 ㅎㅎ

  1. 토스트 메시지를 활용한 예외 피드백

이번 미션에서는 토스트 UI 하나를 활용해서 사용자에게 예외 피드백을 전달해 주었습니다.

현업에서는 더 다양한 예외들이 있을 것이라고 예상되는데, 어떤 예외에 어떤 UI를 사용하는지, 이에 대한 기준이 있는지 궁금합니다 ㅎㅎ

사용자 친화적인 예외 메시지를 전달해주기 위해서, 제가 어떤 부분을 개선해볼 수 있을지에 대한 피드백을 받아보고 싶어요.

이번에도 잘 부탁드립니다. 코드에 대한 설명이나, 궁금한 점은 코멘트로 남겨두겠습니다. 남은 주말 잘 보내세요~ 🙇‍♂️

hwinkr added 29 commits June 7, 2024 00:22
src/apis/cart-item.ts Outdated Show resolved Hide resolved
src/hooks/products/useFetchProducts.ts Outdated Show resolved Hide resolved
src/hooks/products/useFetchProducts.ts Outdated Show resolved Hide resolved
src/hooks/products/useProductsOption.ts Outdated Show resolved Hide resolved
Copy link

@Tanney-102 Tanney-102 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

안녕하세요 해리!
마지막 미션 정말 고생 많으셨습니다 ㅎㅎ
suspense query까지 사용하셨군요~
근데 왜 Suspense는 사용하지 않으셨는지 궁금해요!(제가 못 찾은 걸지도!)

더 사용자 친화적인 예외 메시지를 전달해주기 위해서, 제가 어떤 부분을 개선해볼 수 있을지에 대한 피드백을 받아보고 싶어요.

rq 관련하여 몇가지 코멘트 드렸습니다!

이번 미션에서는 토스트 UI 하나를 활용해서 사용자에게 예외 피드백을 전달해 주었습니다.
현업에서는 더 다양한 예외들이 있을 것이라고 예상되는데, 어떤 예외에 어떤 UI를 사용하는지, 이에 대한 기준이 있는지 궁금합니다 ㅎㅎ
더 사용자 친화적인 예외 메시지를 전달해주기 위해서, 제가 어떤 부분을 개선해볼 수 있을지에 대한 피드백을 받아보고 싶어요.

에러의 중요도, 종류, 복잡도 등에 따라 UI를 구분하는 편인 것 같아요.
뭔가 의도대로 동작하지 않았음을 나타내는 건 일반적으로 토스트로 표현하고 사용자가 꼭 인지하고 넘어가야 하는 내용, 추가적인 액션이 필요한 내용들은 모달을 보여준디거나, form submit의 에러는 form의 각 input의 표현한다거나 하는 식이에요!
모달 하나를 보여주더라도 단순한 Alert이 있을 수 있고 Confirm 동작이나 특별한 form을 가지고 있을 수도 있죠..! 강제 업데이트 같이 정말 중요한 내용이이라면 모달 닫기를 아예 못하게 하기도 하구요.

다음 레벨에서 서버와 함께 프로젝트를 진행하실 때 이런 에러에 대해 미리 고민을 많이해보시면 좋을 것 같아요. 어떤 에러를 정의할 것인지, 에러의 수준은 몇가지로 정의할 것인지, 이러한 수준들을 어떤 필드로 표현할 것인지, 각 수준에 따라 혹은 에러 종류에 따라 어떤 UI를 보여줄 것인지 등등!

const queryClient = new QueryClient({
defaultOptions: {
queries: {
staleTime: 1000 * 60 * 10,

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

오호 staleTime의 기준이 궁금해요!
정답을 정해두고 하는 질문이 아니라 이렇게 커스텀 옵션을 주신 것에 대한 생각이 궁금해서!!

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

오호, 아직 React-Query를 많이 사용해보지 않아서 staleTime에 대한 명확한 기준이 없었어요...!

10분 정도면 상품 목록앱에서 사용하는 데이터에 대한 staleTIme이 적당하지 않을까...막연하게 정한 것 같습니다.

브콜은 상품 목록 앱에서 staleTime이 어느 정도 되어야 적당하다고 생각하시는지 묻고 싶습니다!! ㅎㅎ

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

커머스 성격에 따라 다른 것 같긴하네요! 새로운 상품의 노출이 얼마나 중요한지에 따라..!
일반적으로 10분정도면 적당해 보이긴 하네요!

src/apis/cart-item.ts Outdated Show resolved Hide resolved
src/apis/cart-item.ts Outdated Show resolved Hide resolved
<Styled.Container>
{products.map((product, index) => (
<ProductItem
key={`${product.id}-${index}`}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

index가 들어가야하나요?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

서버에서 중복된 id가 있는 상품이 있었기 때문에 index를 사용해서 구분할 수 있도록 했습니다

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

아하.. 중복이 있었군요

Comment on lines +16 to +18
onSuccess: () => {
queryClient.invalidateQueries({ queryKey: [QUERY_KEYS.cartItems] });
},

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍


const { data, error, isLoading } = useQuery({
queryFn: fetchCartItem,
queryKey: [QUERY_KEYS.cartItems],

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

배열에 넣을 필요는 없어보입니다~

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

쿼리키는 간단한 구조, 고정된 순서를 필요로 하는 경우에는 배열 자료구조를 활용해서, 고유한 식별자를 생성하는 것으로 알고 있는데 브콜께서는 어떤 자료구조로 쿼리키를 관리했으면 좋겠다고 생각하셨나요?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

예전 버전에서 queryKey로 string도 허용했던 걸 생각하고 드렸던 코멘트네요! 최신 버전에서는 배열로 작성하신 게 맞는 것 같습니다. 혼란을 드려 죄송해요ㅜ

src/hooks/cart-items/useFetchCartItem.ts Outdated Show resolved Hide resolved
Comment on lines 22 to 26
queryClient.setQueryData([QUERY_KEYS.cartItems], (prevData: CartItem[]) => {
const nextCartItem = prevData.map((cartItem) => {
return cartItem.product.id === productId ? { ...cartItem, quantity } : cartItem;
});

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mutate 이후 직접 setQuery를 하는 것도 나쁘지 않습니다.
근데 server state인 만큼 정합성을 위해 최신 데이터를 한 번 더 fetch 하는 방법도 고려해봄직 합니다!

Copy link

@Tanney-102 Tanney-102 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

안녕하세요 해리! 끝까지 고생많으셨습니다.
더 반영할 내용은 없어보여 이만 머지하도록 하겠습니다!

@Tanney-102 Tanney-102 merged commit 753b980 into woowacourse:hwinkr Jun 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants