[Step2.3] hippo: TanStack Query 적용하기 - #7
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Free Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughZustand 기반의 ChangesTanStack Query 마이그레이션
추정 코드 리뷰 노력🎯 3 (Moderate) | ⏱️ ~20 minutes Note 🎁 Summarized by CodeRabbit FreeYour organization is on the Free plan. CodeRabbit will generate a high-level summary and a walkthrough for each pull request. For a comprehensive line-by-line review, please upgrade your subscription to CodeRabbit Pro by visiting https://app.coderabbit.ai/login. Comment |
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
[배움]
useQuery, useMutation을 src/queries/ 폴더에 커스텀 훅으로 분리한 구조가 인상적이었습니다. 저는 컴포넌트 안에 직접 작성했는데, 이렇게 분리하면 같은 쿼리를 여러 컴포넌트에서 재사용할 때나 queryKey 변경 시 한 곳만 수정하면 된다는 장점이 있을 것 같습니다!
|
|
||
| export function useRestaurantsQuery() { | ||
| return useQuery({ | ||
| queryKey: ["restaurants"], |
There was a problem hiding this comment.
[논의]
["restaurants"]를 useRestaurantsQuery.js와 useAddRestaurantMutation.js에 각각 직접 작성하셨는데, 저는 RESTAURANTS_QUERY_KEY 상수를 별도 파일로 분리했습니다. 쿼리 훅이 한 폴더에 모여있다면 파일 내 상수로도 충분히 관리될 것 같은데, 상수 파일 분리 없이 인라인으로 작성하신 이유가 궁금합니다. 실무에서는 "몇 군데서 참조하냐"에 따라 파일 내 상수 vs 별도 파일로 나누거나, 쿼리가 다양해지면 restaurantKeys.all, restaurantKeys.detail(id) 같은 팩토리 패턴으로 발전시키기도 하더라고요.
There was a problem hiding this comment.
별도의 상수 파일로 두면 좋을 것 같네요. 반영하겠습니다!
| mutation.mutate( | ||
| { category, name, description }, | ||
| { | ||
| onSuccess: () => onClose(), | ||
| onError: () => alert("음식점 추가에 실패했습니다. 다시 시도해주세요."), | ||
| }, | ||
| ); |
There was a problem hiding this comment.
[논의]
onMutate에서 목록을 낙관적으로 업데이트하면서도 모달은 onSuccess(서버 응답 후)에서 닫히도록 구현하셨는데, 이 조합에서 목록에는 새 항목이 이미 보이는데 모달은 아직 열려있는 상태가 짧게 발생하지 않나요? 저는 낙관적 업데이트라면 모달도 onMutate에서 함께 닫는 게 일관성 있다고 판단해서 그렇게 구현했는데, 혹시 다르게 판단하신 이유가 있으신지 궁금합니다!
There was a problem hiding this comment.
지적하신 부분은 맞습니다. 하지만 의도가 있는 구현이었는데 서버 실패 시 유저가 폼을 다시 채울 수 있게 하려고 했습니다. onMutate에서 모달을 먼저 닫아버리면 실패 시 유저가 입력했던 내용이 사라집니다. 하지만 onSuccess에서 모달을 닫으면 실패했을 때 모달이 그대로 남아 있어서 유저가 바로 수정하고 다시 제출할 수 있습니다.
| onError: (_err, _newRestaurant, context) => { | ||
| queryClient.setQueryData(["restaurants"], context.previousRestaurants); | ||
| }, |
There was a problem hiding this comment.
[배움]
onError: (_err, _newRestaurant, context) 처럼 미사용 파라미터에 _ 접두사를 각각 붙이셨는데, 저는 (err, _, context)처럼 _ 단독으로 썼습니다. _err, _newRestaurant 방식이 파라미터 자리 의미(어떤 인자인지)를 유지하면서 의도적으로 사용하지 않음을 명시적으로 표현한다는 점이 더 명확한 것 같아서 좋은 것 같습니다!
| {isLoading && <p>불러오는 중...</p>} | ||
| {error && <p>{error}</p>} | ||
| {error && <p>{error.message}</p>} |
There was a problem hiding this comment.
[제안]
로딩과 에러를 JSX 인라인으로 처리하셨는데, early return 패턴으로 바꾸면 로딩/에러 상태일 때 빈 리스트가 함께 렌더링되는 걸 방지할 수 있습니다.
if (isLoading) return <p>불러오는 중...</p>;
if (error) return <p>{error.message}</p>;어떻게 생각하시나요?
There was a problem hiding this comment.
early return을 사용하는 방법이 있다는 것을 알려주셔서 감사합니다. 적용해보겠습니다!
|
|
||
| `onSuccess` 대신 `onSettled`를 쓴 이유는, 실패 후 롤백된 상태에서도 서버 데이터와 동기화가 필요하기 때문이다. `onSuccess`는 성공 시에만 실행되지만 `onSettled`는 성공/실패 상관없이 항상 실행된다. | ||
|
|
||
| ### useOptimistic (React 19) |
There was a problem hiding this comment.
[배움]
README에 useOptimistic (React 19)과 TanStack Query Optimistic Update를 비교한 섹션이 인상적이었습니다. 저는 이 주제를 따로 학습하지 않았는데, Server Actions 환경에서는 롤백 코드 없이 자동 처리된다는 점이 흥미로웠습니다. 현재 미션 구조(REST API + fetch)에서는 TanStack Query가 더 자연스럽다는 것도 배워갑니다!
개인 목표 달성 여부
useQuery,useMutation,QueryClient개념을 직접 사용하며 TanStack Query 구조를 익힌다.Summary by CodeRabbit
New Features
Bug Fixes
Documentation