[장바구니 미션 Step 1] 코난(윤정민) 미션 제출합니다. #170
Conversation
|
이거 원래 이런건가요..? |
서버가 불안정한 것 같아요..😱 |
There was a problem hiding this comment.
코난 안녕하세요~
api server 가 잘 동작하지 않는거 같았는데, 오늘 걍 리뷰해야지 하니까 또 잘 되네요? ㅎㅎ
저도 Recoil에 익숙하지는 않아서, 많이 소통하면서 저도 많이 배워갔으면 좋겠네요~
아래는 PR본문의 질문에 대한 답변입니다~~
ErrorBoundary와 개별 에러 정책
ErrorBoundary는 그냥 도구입니다.
비동기 상황일 때 try-catch문의 catch문을 선언적으로 사용할 수 있게 된거랑 비슷한거죠
지금 에러를 받아서 그냥 fallback을 그리는 용도로 사용되는건, 에러를 어떻게 핸들링 할지에 대한 정책이 없기 때문입니다.
각 ProductItem을 ErrorBoundary로 감싼 다음에, 에러가 나면 그냥 아무것도 렌더링 안하겠다 라는 정책을 가질 수도 있고, StyledThumbnail 에서 img를 prefetch하는 과정에서 에러가 나면 default 이미지를 렌더링하겠다 라는 정책을 가질 수도 있습니다.
그리고 최상단에 앱이 터지는걸 막기 위한 ErrorBoundary도 당연히 필요하다고 생각합니다~
(흰화면이 보여지는게 프론트엔드 최고의 장애죠..)
레퍼런스로는 https://jbee.io/react/error-declarative-handling-2/ 추천드립니다~~
src/atoms/cart.ts
Outdated
|
|
||
| export const cartState = atom<Cart[]>({ | ||
| key: 'CartListState', | ||
| default: getLocalData('CART'), |
There was a problem hiding this comment.
localStorageEffect가 있는데 default 값으로 localStorage에서 로드한 값을 넣어줘야 하나요?
There was a problem hiding this comment.
AtomEffect에 대한 이해도가 부족했던 것 같습니다.
저는 atom이 먼저 default로 초기화되고 그 후부터 effect가 적용된다고 생각하였는데,
default를 초기화할 때 setSelf가 적용되는 것이었네요.
✍️ 수정 커밋 - 98adcd7
src/atoms/product.ts
Outdated
| export const fetchProductSelector = selector({ | ||
| key: 'FetchProductSelector', | ||
| get: async () => { | ||
| const response = await fetch(MOCK_API_URL); | ||
|
|
||
| if (!response.ok) throw new Error('foo'); | ||
|
|
||
| const products = await response.json(); | ||
|
|
||
| return products as Product[]; | ||
| }, | ||
| }); |
There was a problem hiding this comment.
selector가 아니라 atom 이 더 맞지 않을까요? (궁금)
There was a problem hiding this comment.
atom의 default 값에 Promise가 가능하긴 합니다.
const fetchProductState = atom({
key: 'FetchProductState',
default: fetch(MOCK_API_URL).then((response) => response.json()),
});하지만 atom은 상태를 나타내는 역할이어서, 이렇게 되면 AtomEffect를 걸어야 다시 fetch를 시도할 것입니다.
selector는 순수함수와 같은 기능을 하는 역할이기 때문에 비동기 fetch는 selector가 더 어울리는 것 같습니다.
useEffect(AtomEffect)를 걷어낸다는 점에서 장점이기도 한 것 같아요
There was a problem hiding this comment.
일단 저는 리코일알못이라는 점을 말씀드립니다 ㅎㅎ
const ProductList: React.FC = () => {
const products = useRecoilValue(fetchProductSelector);
// 생략- ProductList가 리렌더링 되면 fetchProductSelector의 get이 다시 호출되어 api 호출을 다시 하게 되나요?
즉, refetch를 하려면 selector 에서는 어떻게 해야하는 건가요..?
If the user names were stored in some database we need to query, all we need to do is return a Promise or use an async function. If any dependencies change, the selector will be re-evaluated and execute a new query. The results are cached, so the query will only execute once per unique input.
https://recoiljs.org/docs/guides/asynchronous-data-queries#asynchronous-example
저는 selector를 사용하면 get 에서 의존하고 있는 다른 atom이 변화되면 다시 selector의 get이 호출되고,
그러면 refetch 가 이뤄진다고 생각했어요.
근데 현재는 의존하고 있는 다른 atom이 없고, refetch도 하지 않기 때문에, 그냥 atom의 default로 충분하지 않나? 라는 생각을 했었고용!
- atom의 default로 Promise값을 주면, 해당 atom이 사용되지 않더라도 바로 api 호출이 될거 같고,
selector의 get으로 Promise를 반환하는 함수를 전달하면 실제 selector가 사용될 때 api 호출이 되지 않을까 싶어요.
(간단히 테스트했을 때는 그런거 같아 보이긴 한데 확실하진 않네요 ㅎㅎ)
3.기능이 계속 추가하면, 분명히 refetch가 필요한 순간이 올거 같아요. 서버단에서 새로운 상품이 추가된다거나, 기존 상품이 품절되었다 같은 정보를 줄수 있으니까요. 그러면 selector가 더 편할거 같긴 한데, useResetRecoilState 같은 걸 사용하면 atom이 더 편하지 않으려나 싶기도 하고 그렇네요. (react-query에 너무 찌들어서.. 그냥 react-query 쓸거 같긴 합니다 ㅎㅎ)
src/components/Header/index.tsx
Outdated
| <StyledHeaderWrapper> | ||
| <StyledHeaderBox> | ||
| <StyledTitle>SHOP</StyledTitle> | ||
| <StyledCartWrapper> | ||
| <StyledCart>장바구니</StyledCart> | ||
| <StyledCartAmount data-cy="cart-amount">{totalAmount}</StyledCartAmount> | ||
| </StyledCartWrapper> | ||
| </StyledHeaderBox> | ||
| </StyledHeaderWrapper> |
There was a problem hiding this comment.
아직 Layout이나 Outlet을 구성하지 않아서 간단하게 css 수정하여 맞추어보았습니다.
✍️ 수정 커밋 - 31af335
| const useCart = (cartState: RecoilState<Cart[]>, product: Product) => { | ||
| const [cart, setCart] = useRecoilState(cartState); |
There was a problem hiding this comment.
cartState 는 그냥 import 해서 쓰면 될거 같은데,
굳이 인자로 넘겨 받는 이유는 무엇인가요?
(테스트 편의성 때문인가요?)
There was a problem hiding this comment.
테스트 편의성 때문인가요?
약간 찔리는 기분이 든다면 테스트 편의성 때문이 조금 더 높았다고 할 수 있을까요..? 😭
장바구니가 여러개일 경우를 생각해봤었는데 그럴 경우는 없을 것 같기도 하네요..
There was a problem hiding this comment.
이런 라이브러리를 쓰면 테스트가 번거로워질 수 밖에 없습니다 💦
그렇지만 라이브러리의 특정 함수를 인자로 전달하는 방식은 일반적으로 그다지 선호되지는 않는거 같습니다 ㅎㅎ
여유가 되면 https://recoiljs.org/docs/guides/testing 이문서를 보고 여러가지 시도해보시면 좋겠네요~
src/components/ProductItem/index.tsx
Outdated
| const { cart, addCart, updateCart, deleteCart } = useCart(cartState, product); | ||
| const productItemQuantity = cart.find((c) => c.product.id === product.id)?.quantity; | ||
|
|
||
| const [count, setCount] = useState(productItemQuantity || 0); |
There was a problem hiding this comment.
count가 불필요한 로컬 상태처럼 느껴지는거 같은데요..
useState(productItemQuantity || 0); 만 봐도 count 가 productItemQuantity의 파생상태로 느껴지는거 같아요!
There was a problem hiding this comment.
장바구니에 담긴 수량(count)을 cartState를 활용하는 파생상태(cartQuantityReadOnlyState)로 변경하였습니다.
✍️ 수정 커밋 - 754ca15
src/components/ProductItem/index.tsx
Outdated
| const limitInputNumber = (e: React.ChangeEvent<HTMLInputElement>) => { | ||
| if (e.target.value.length > 3) { | ||
| e.target.value = e.target.value.slice(0, 3); | ||
| } | ||
| }; | ||
|
|
||
| const handleCartAmountChange: React.ChangeEventHandler<HTMLInputElement> = (e) => { | ||
| limitInputNumber(e); | ||
| const newCount = Number(e.target.value); |
There was a problem hiding this comment.
event.target.value의 값을 직접 바꾸는건 어색한거 같습니다.
StyledCountInput을 controlled 로 사용하고 있으니 더더욱이요~
src/components/ProductItem/index.tsx
Outdated
| }; | ||
|
|
||
| const handleBlur: React.FocusEventHandler<HTMLInputElement> = (e) => { | ||
| if (Number(e.target.value) < 1) { |
src/components/ProductItem/index.tsx
Outdated
| updateCart(newCount); | ||
| setCount(newCount); | ||
|
|
||
| if (newCount === 0) deleteCart(); |
There was a problem hiding this comment.
newCount가 0이면 updateCart는 필요없는거 아닌가요..?
There was a problem hiding this comment.
// src/components/Counter/index.tsx
const handleCartPlus = () => {
if (cartQuantity === 0) {
addCart();
} else {
updateCart(cartQuantity + 1);
}
};
const handleCartMinus = () => {
if (cartQuantity === 1) {
deleteCart();
} else {
updateCart(cartQuantity - 1);
}
};위와 같이 add, delete, update를 나누어봤습니다.
src/components/ProductItem/index.tsx
Outdated
| const handleBlur: React.FocusEventHandler<HTMLInputElement> = (e) => { | ||
| if (Number(e.target.value) < 1) { | ||
| setCount(0); | ||
| deleteCart(); | ||
| } | ||
| }; |
There was a problem hiding this comment.
근데 count 를 0으로 바꿔버리면 바로 handleCartAmountChange 에 의해서 deleteCart 가 호출되어 버려서,
실제로 handleBlur는 호출되지 않는거 같은데요..?
There was a problem hiding this comment.
input에 사용자가 입력이 가능하도록 만들게 되어서 음수를 입력하는 경우(-)를 생각했었는데
다시 생각해보니 onChage 이벤트에서 처리해도 되는 문제네요!
|
아 말하는걸 깜빡했네요 전역상태관리가 필요한 데이터에 대한 인식은 잘 되어 있는거 같습니다. 한번 생각해볼건, Cart가 product가 아니라 product의 id 만 갖고 있어도 되지 않을까? 입니다. |
주요 변경사항
그리고 장바구니가 장바구니에 담긴 상품 갯수 총합에서 개인적인 생각웹페이지마다 다른 정책을 가지고 있어 재밌었습니다.
wallmart나 B마트(모바일) 등등 상품을 10~12개 정도만 담을 수 있게 제한되어 있고, 최대 갯수 제한이 있어 굳이 input 입력을 받지 않습니다. P.S. 요청 드리고 생각해보니 input을 입력할 수 없는 Counter를 적용하면서 상품은 최대 99개까지 담을 수 있게 만들었네요.. |
Step1 - Begin State Management
안녕하세요, 동동 😊
이번 step1은 전역상태관리에 대해 Recoil을 활용해 학습해보는 시간입니다!
핵심 기능은 장바구니에 상품을 추가하고, 헤더에서 장바구니에 들어간 상품 갯수의 합을 출력하는 것입니다.
학습 목표
필수 요구 사항
기능 구현 사항
궁금한 점
ErrorBoundary와 개별 에러 정책
ErrorBoundary를 구현하면서 페어와 함께 궁금했던 부분은 에러처리를 ErrorBoundary가 하는 것이 맞느냐입니다.
먼저 Suspense가 나오기 전 사용되던 패턴인 isLoading, hasError, data 3가지 상태를 가진 커스텀 fetch 메서드를 사용하여 Loading과 Error에 대해 처리하였습니다.
그 후 최근 React에서 선언적인 코드를 위해 지원한 Suspense를 활용해보면서 ErrorBoundary도 함께 적용하였습니다.
이 과정에서 ErrorBoundary는 단순히 감싼 컴포넌트 내부에서 던져진 Error를 받아 fallback을 그려줄 뿐이었고,
개별 에러 정책은 어떻게 처리해주어야 하는지 답을 내지 못했습니다.
에러 핸들링이 필요한 부분에 개별 에러 정책을 구현하다보면, ErrorBoundary는 어플리케이션이 터지지 않기 위한 용도로만 사용될 것 같아
ErrorBoundary가 필요한가에 대해서도 의문점이 들었습니다.
ErrorBoundary의 적절한 활용사례나 Suspense를 활용하는 경우 개별 에러 정책은 어떻게 적용할 것인지 조언 주시면 감사합니다!