[클린코드 리액트 3기 박재영] 장바구니 미션 Step2 #60
Conversation
- pr 피드백 반영
There was a problem hiding this comment.
안녕하세요! 재영님! 리뷰 늦게 해서 죄송합니다!
전반적으로 hook으로 분리가 잘 되어있어서 읽기에 편했어요!
리뷰할것도 자잘한거 밖에 없었습니다.
혹시 추가적인 기능구현도 하실 계획이실까요?
이 정도 퀄리티면 지금 머지해도 충분하다고 생각이 들어서요 ㅋㅋ
일단 다시 request드리고 재요청주시면 merge해도될거같습니당
의견
- 개인적으로는 api 에러처리가 되어있었으면 하는 마음이 들었네요!
(api에서 에러가나면 메인페이지에서 무한 렌더링이 발생하는것 같아서) - 장바구니에서 수량을 변경하면 새롭게 fetch하는거같아요 ㅋㅋ UI가 새로 불러와지네요.
답변
데이터를 어디에 저장할까
- 상품 데이터의 경우 많다면 엄청 많을수도 있겠다는 생각
- 비로그인 사용자의 상품 데이터는 구매하기 전까지 클라이언트만 알고 있으면 된다는 생각
- 상품 데이터가 그렇게 취약한 정보는 아닐거라는 생각
위 3가지 이유때문에 저라면 localStorage에 해버렸을거같아요! ㅋㅋ
낙관적 업데이트를 어떻게 처리할 지 고민입니다.
네네 말씀주신것처럼 프론트상태로 받아와서 저장해도 구현할 수 있고
mutate써서 후속동작으로 query data를 직접변경하는 방법도 있을거같아요!
| @@ -0,0 +1,38 @@ | |||
| import { CookieOptions } from './type' | |||
There was a problem hiding this comment.
요거 유틸함수 같은데, useCookieRepository 에서만 사용할 유틸이라 여기서 정의한것인가 생각했었는데
getCookie 가 다른 곳에서도 쓰이고 있네요!
저라면 util폴더에 놔뒀을것 같긴합니다!
아니면 재영님이 어떤 기준을 가지고 넣었다면 괜찮은거 같습니다 ㅎㅎ
There was a problem hiding this comment.
원래는 useCookieRepository 훅에서 쿠키 상태를 관리하려고 해당 폴더 하위에 만들어 두었었는데요 ㅎㅎ useCartListStore 훅에서 전역 상태로 관리하는것으로 변경하면서 해당 유틸 함수의 위치른 변경하지 않았었습니다. 말씀해주신대로 utils 하위로 수정해두었습니다!
| const cookie = document.cookie | ||
| .split('; ') | ||
| .find((row) => row.startsWith(`${name}=`)) | ||
| ?.split('=')[1] |
There was a problem hiding this comment.
쿠기에 '=' 값이 들어가도 정상적으로 동작할까 싶었느데, MDN 공식문서에 그렇게 되어있네요 흠..
| const cookieOptions = Object.entries(options).reduce((prev, [key, value]) => { | ||
| switch (key) { | ||
| case 'expires': | ||
| return (prev += `; expires=${(value as Date).toUTCString()}`) | ||
| default: | ||
| return (prev += `; ${key}=${value}`) | ||
| } | ||
| }, '') |
There was a problem hiding this comment.
prop을 직접 수정하는건 지양해보는게 어떨까요?
const newValue = ~~ 형식으로 새로운 값을 반환하는게
불변성을 지키는 측면 등등에서도 좋아보입니다 :)
There was a problem hiding this comment.
언급하신 prop이 options를 말씀하시는걸까요? 👀 options를 const newValue = ~~와 같은 형식으로 deep copy하는게 좋다는 말씀이신지 궁금합니다!
| export interface Store<T> { | ||
| getState: () => T | ||
| setState: (next: T | ((prev: T) => T)) => void | ||
| subscribe: (callback: () => void) => () => void | ||
| } | ||
|
|
||
| /** | ||
| * @summary 스토어를 생성한다. | ||
| */ | ||
| export const createStore = <T>(initialValue: T): Store<T> => { | ||
| let state = initialValue | ||
| const callbacks = new Set<() => void>() | ||
|
|
||
| const getState = () => { | ||
| return state | ||
| } | ||
|
|
||
| const setState = (next: T | ((prev: T) => T)) => { | ||
| state = typeof next === 'function' ? (next as (prev: T) => T)(state) : next | ||
|
|
||
| callbacks.forEach((callback) => callback()) | ||
| } | ||
|
|
||
| const subscribe = (callback: () => void) => { | ||
| callbacks.add(callback) | ||
|
|
||
| return () => callbacks.delete(callback) | ||
| } | ||
|
|
||
| return { | ||
| getState, | ||
| setState, | ||
| subscribe, | ||
| } | ||
| } |
| return unsubscribe | ||
| }, [store]) | ||
|
|
||
| return [state, store.setState] as const |
| foundProduct | ||
| ? setValue((prev) => ({ | ||
| ...prev, | ||
| cartList: [ | ||
| ...prev.cartList.filter((product) => product.id !== id), | ||
| { id, count: foundProduct.count + 1 }, | ||
| ], | ||
| })) | ||
| : setValue((prev) => ({ ...prev, cartList: [...prev.cartList, { id, count: 1 }] })) |
There was a problem hiding this comment.
개인적으로 삼항연산자는 식으로 쓰이기 때문에, 값으로 평가되는 곳에 사용하는게 목적에 맞다고 생각합니다.
간단하게 쓰이는 용도로 쓰셔도 되는데, 지금과 같은 곳에서는
if/else 처럼 '문'으로 쓰이도록 하는 편입니당 ㅎㅎ
개인차이라서 넘어가셔도 될거같아요!
There was a problem hiding this comment.
말씀하신대로 표현식과 문을 혼용해서 사용했었네요 ㅎㅎ 🤣 반영 했습니다!
| export const CART_MIN_COUNT = 1 | ||
| export const CART_MAX_COUNT = 20 | ||
|
|
||
| export const CART_COOKIE_KEY = 'CARTS' |
src/routes/cart/-useCartVM.ts
Outdated
| setSelectedProductList((prev) => { | ||
| const newMap = new Map(prev) | ||
|
|
||
| if (newMap.get(id)) { |
There was a problem hiding this comment.
말씀해주신게 더 의미가 적절할 것 같습니다. 감사합니다!
|
|
||
| <div className="number-input-container"> | ||
| <input | ||
| key={product.count} |
There was a problem hiding this comment.
ㅎㅎ; 해당 input은 상품의 개수를 변경할 수 있는 역할을 합니다. input이 controlled, uncontrolled로 다룰 수 있을텐데 먼저 controlled일 경우 value, onChange props를 내려줘야 하는데 onChange를 사용하게 될 경우 입력된 value를 변경하고 사용자가 입력을 멈췄다는 플래그가 추가로 필요할 것 같습니다. 그래서 defaultValue로 기본값을 할당하고 onBlur시 상품의 카운트를 바꾸는 uncontrolled 방식으로 input을 다루게 되었습니다. 그런데 defalutValue때문인지 상품의 갯수 증감시 리렌더링이 되지 않더라구요 🤔 그래서 key값으로 강제 리렌더링 되도록 했습니다.
| import { ProductCard } from '../ProductCard/ProductCard' | ||
|
|
||
| /** | ||
| * @summary 상품목록 컴포넌트 | ||
| */ | ||
| export const ProductList = () => { | ||
| const { data, isLoading, isError } = useQuery<ProductsListType>({ | ||
| const { ref, inView } = useInView() | ||
| const { data, status, isFetchingNextPage, hasNextPage, fetchNextPage } = useInfiniteQuery({ | ||
| queryKey: ['/posts'], |
There was a problem hiding this comment.
- pr 피드백 반영
- queryClient.invalidateQueries를 활용하여 백그라운드에서 데이터를 가져오도록 수정 - 액션 실행시데이터 호출을 트리거 하기 위해 데코레이터 함수로 액션 함수를 감쌈
|
안녕하세요 정혁님! 리뷰 남겨주셔서 감사합니다.😊 코멘트 달아주신 부분들 수정 혹은 댓글로 제 생각과 질문을 남겼습니다.
queryClient.invalidateQueries를 활용하여 백그라운드에서 데이터를 가져오도록 수정해보았습니다. 추후 낙관적 업데이트를 적용해 UX를 향상시켜볼 생각입니다.
낙관적 업데이트를 추가적으로 적용해볼 생각입니다. 이 부분은 개인 레파지토리에서 수행 하겠습니다ㅎㅎ |
안녕하세요 정혁님 😄 step2도 잘 부탁드립니다!
Pull Request Template
1. 가이드
1-1. 배포
docs
1-2. 로컬 환경
2. 리뷰 요청 ✍️
2-1. 어렵거나 아쉬웠던 점
2-2. 나누고 싶은 고민
2-3. 중점적으로 리뷰받고 싶은 부분
2-4. 새로운 시도 혹은 미션을 통해 도전한 점
@tanstack/react-router의 ScrollRestoration를 활용해 구현했습니다.2-5. 도식화된 자료 혹은 작성된 이미지 형태의 설계 구조
-
3. 질문있어요 🙋
4. 요구 사항 ✅
4-1. 필수 요구사항
필수 요구사항
상품목록
장바구니
4-2. 선택 요구사항 (심화)