-
Notifications
You must be signed in to change notification settings - Fork 51
[클린코드 리액트 3기 한재성] 장바구니 미션 Step1 #46
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
base: han-d-peter
Are you sure you want to change the base?
Conversation
|
|
||
| export const queryClient = new QueryClient(); | ||
|
|
||
| enableMocking().then(() => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
mocking을 안 하고 API를 직접 사용하는 경우에는 어떻게 표현되어야 할까요?
더 정확히는, 배포하는 시점에는 API를 직접 호출해서 사용한다고 했을 때 이런 코드가 어떤 모습이 되어야할까요?
- 개발할 때는 목킹 사용
- 배포할 때는 API 사용
JunilHwang
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
안녕하세요 재성님!
장바구니 미션에서 다시 만나게 되었네요 ㅎㅎ
컴포넌트를 설계할 때는 일관성과 이벤트에 대한 정의가 제일 중요하다고 생각해요!
지금은 shared에 계층 구분 없이 ( primtive와 primtive가 아닌 것들 ) 컴포넌트들이 만들어져 있어서 이를 구분 해주면 어떨까 싶은 생각이 들었어요.
그리고 Page 컴포넌트들의 경우 비즈니스 로직이 컴포넌트 내부에 그대로 표현되어있다보니, 이를 함수 혹은 훅으로 분리하면 어떨까 싶은 생각이 들었습니다!
훅을 정의할 때 재사용을 기준으로 정의하기보단, 관심사 분리를 기준으로 적용할 수도 있답니다 ㅎㅎ
그 다음에 전체적으로 타입스크립트의 장점을 활용하지 못하고 있다는 느낌을 받았어요.
똑같은 타입인데 계속 재정의해서 사용한다거나
api관련 부분은 아예 반환하는 값이 any로 취급되고 있어서 개발할 때는 실제로 어떤 타입이 있는지 작성자가 아닌 다른 사람은 알기 힘들지 않을까 싶네요 ㅠㅠ
그리고 query에서 mock 폴더를 직접 참조하고 있는데요, mock을 참조할 수 있는건 mock 밖에 없어야 하지 않을까 싶어요!
실제로 우리가 어플리케이션을 만들고 관리할 때는 mock을 참조하면 안 되니까요 ㅎㅎ
나머지 부분은 상세 리뷰를 참고해주세요!
부지런히 나아갑시다 ㅎㅎ 화이팅!!
src/main.tsx
Outdated
| } | ||
| } | ||
|
|
||
| export const queryClient = new QueryClient(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
queryClient는 다른 곳에서 정의해서 가져오는게 좋을 것 같아요!
여기서 export 하는 경우 많은 파일들이 main.tsx를 참조하게 될 것 같네요!?
| onSuccess() { | ||
| alert("카트 이동 성공"); | ||
| }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
onSuccesss는 되도록 지양하면 좋을 것 같아요!
| @@ -0,0 +1,26 @@ | |||
| import { useMutation, useQuery } from "@tanstack/react-query"; | |||
| import cartsRepository from "./carts.repository"; | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
API를 관리하는거라서, Repository라는 표현은 조금 어색한 것 같아요!
clients나 remotes는 어떨까요?
| import { CART_URI } from "../../../../mocks/api/carts"; | ||
| import { cartsScheme } from "./carts.type"; | ||
| import { Product, Response, responseScheme } from "../types"; | ||
|
|
||
| class CartsRepository { | ||
| async getCarts() { | ||
| try { | ||
| const response = await fetch(CART_URI.carts.uri); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
mock 폴더를 직접 참조하는건 좋지 못한 방법 같아요.
URI를 mock이 아닌 다른 곳에서 관리하면 어떨까요?
| } catch (error) { | ||
| throw new Error("getCarts api 의 오류."); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
catch가 꼭 필요할까요? 없으면 어떤 일이 발생할까요?
| const [countValue, setCountValue] = useState(0); | ||
|
|
||
| useEffect(() => { | ||
| onChange && onChange(value ?? countValue); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
onChange?.(value ?? countValue);이렇게 표현하면될 것 같네요!?
| useEffect(() => { | ||
| onChange && onChange(value ?? countValue); | ||
| }, [countValue, value]); | ||
|
|
||
| function up() { | ||
| if (countValue >= max) { | ||
| setCountValue(max); | ||
| } else { | ||
| setCountValue(countValue + 1); | ||
| } | ||
| } | ||
|
|
||
| function down() { | ||
| if (countValue <= min) { | ||
| setCountValue(min); | ||
| } else { | ||
| setCountValue(countValue - 1); | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
그런데 이 경우에 up / down 으로 인하여 render 함수가 실행되고,
그 다음에 useEffect 내부의 함수 때문에 render가 다시 한 번 발생할 수 있을 것 같습니다.
그래서 이 때는 up/down 내부에서 onChange를 호출하는게 더 좋지 않을까 싶어요!
| function add(e: MouseEvent) { | ||
| e.preventDefault(); | ||
| e.stopPropagation(); | ||
| onAdd && onAdd(); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
실제로 이 컴포넌트 내부에서 "증가" 라는 행위가 일어나면 onAdd 라는 이름이 어울릴 것 같은데, 그런게 아니라서
onAddClick 처럼 표현하면 어떨까 싶습니다!
| <div className={cardLeftSection}> | ||
| <div> | ||
| <div> | ||
| <input type="checkbox" onChange={check} {...{ checked }} /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
함수를 래핑하지 않고 그대로 onChange에 onCheck를 넘겨줘도 동작할 것 같네요
<input type="checkbox" onChange={onCheck} {...{ checked }} />요로코롬!?
| weight = 400, | ||
| }: Text) { | ||
| return ( | ||
| <div style={{ color }} className={IconButtonStyle({ as, weight })}> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
div가 아닌 다른 태그를 사용하고 싶다면, 태그를 바깥에서 주입하고 싶다면 어떻게 하면 좋을까요?
1-1. 배포
배포된 페이지가 있다면 링크를 남겨주세요 (삭제)
LMS
CSS
1-2. 로컬 환경
install
yarn
run
yarn dev
test
yarn test
2. 리뷰 요청 ✍️
2-1. 어렵거나 아쉬웠던 점
tanstack router 를 처음 사용해봤는데 라우팅 설정과 라우팅에 따른 공통 layout 설정 등 기본적인 사용 패턴에 대한 고민이 있었습니다.
zod를 사용해 외부에서 들어오는 api에 대한 타입검증을 설정했습니다만 어느 단계에서 처리하면 좋을지에 대한 고민이 있었습니다. 저는 직접적으로 fetch 를 사용해 응답을 받아오는 단계에서 사용했습니다.
2-2. 나누고 싶은 고민
2-3. 중점적으로 리뷰받고 싶은 부분
컴포넌트 설계나 구조에 대해서 중점적으로 리뷰 받아보고 싶습니다.
2-4. 새로운 시도 혹은 미션을 통해 도전한 점
기간이 한정되어 있는 상태에서 vanilla extract와 zod, tanstack router 를 처음 사용해봤는데
언제나 새로운 기술을 만날 수 있기 때문에 빠르게 학습하고 적용하는 능력이 중요하겠다는 생각이 들었습니다.
2-5. 도식화된 자료 혹은 작성된 이미지 형태의 설계 구조
3. 질문있어요 🙋
4. 요구 사항 ✅
예시를 참고하여 체크 박스를 활용해주세요 (삭제)
4-1. 필수 요구사항
[ o ] Storybook 상호 작용 테스트
[ o ] MSW를 활용한 API mocking
4-2. 선택 요구사항 (심화)
반응형 레이아웃을 구현한다.