[레벨2 - 미션3] 2단계 장바구니 피드백

Nine·2022년 6월 20일
1

장바구니 2단계 PR

🎯 이번 피드백을 통해 도메인을 적절히 분리하는 방법에 대해 배운 것 같아요.

🎯 페이먼츠 미션에서 생각하지 못했던 커스텀훅의 재활용성에 대해서도 고민해보게 되었던 시간이였습니다.

🔨 리뷰를 살펴볼까요?

필요 없는 건 숨기는 스토리북

어떤 컴포넌트를 스토리롤 만들지 생각해봅시다🤔🤔

  1. 전반적으로 UI 구성에 필요한 값들만 Controls 영역에 노출되어 있어야 Controls 이용이 용이할 것 같아요.

  2. 다만 컴포넌트라고 무조건 스토리를 만드는 것은 비추입니다. UI와 연관이 영역만을 테스트합시다.
    예를 들어 ErrorPendingBoundary, children을 적극 활용하는 Header같은 컴포넌트들은 스토리 (UI테스트)를 만드는 이유가 있을까요?


스토리북에서의 API 모킹?

storybook에서 API를 mocking 해야 될지 말지 여부에 대한 판단은 storybook의 사용범위를 어떻게 설정하느냐에 따라 달라질 것 같아요.

모든 페이지가 storybook의 대상이라면 컴포넌트에 필요한 데이터를 mocking하는게 한계가 있으니까 결국 실제 API를 호출하게 되겠죠.

먼저 생각을 해봅시다. storybook의 적절한 사용범위는 (특히 CDD를 위한) 컴포넌트별로 💪 잘게 쪼개진 공통 컴포넌트의 UI 카탈로그 정도라서 그런 경우라면 API의 호출이 대부분 불필요하겠죠.

⌛ 결국 이 부분은 Storybook을 어느 범위까지 활용하느냐에 따라 달라지는 부분이라 그 부분에 대한 고민이 먼저 선행되야 될 것 같아요.


도메인이 존재하는 컴포넌트?

최대한 page 단에서만 도메인 로직을 작성 하다보니 물론 비즈니스 로직이 제거된 일부 컴포넌트들의 재사용성이 높아지기는 했지만 page 컴포넌트들의 파일의 길이가 너무 길어지고 전달되는 props도 너무 많아지는 느낌이 드네요.

page 하위에서 비즈니스 로직을 적절히 위임한 컴포넌트들로 조금은 더 쪼개지는게 좋을 것 같아요.

현재처럼 page 내부에서만 스토어에 접근한다면 store로 전역으로 관리될 필요 없이 그냥 해당 page 내에 state로 정의해서 props로 전달하는게 더 나은 경우도 보이고요.

(맞죠? 사실 어디서든 접근하기 위해 store를 만든 것인데 이렇게 page단위에서 전부 props로 내려주면 쓰는 의미도 없어진다고 생각이 드네요.)

현재의 요구사항 정도라면 page 내부에 적절히 도메인별로 분리된 component가 있고 그런 component 들에서 비즈니스 로직이 제거된 공통 컴포넌트들 정도의 3개의 계층으로 분리해 보면 좋을 것 같습니다.


전역에서 관리하는 API LOADING, SUCCESS, FAILURE

😀 리뷰어님의 생각

전체적으로 PENDING, SUCCESS, FAILURE가 store 마다 관리되고 있어요.
이 값들은 fetch를 하는 동안과 끝난 이후에 짧은 시간 동안만 사용되는 값들입니다.
이런 값들을 꼭 전역으로 관리할 필요가 있을까요? createAsyncThunk를 활용해서 개선해보면 어떨까요?

😀 나의 생각

전역으로 관리하기에는 "굳이?" 라는 의문이 드는 점을 크루들에게 정말 많이 물어봤는데 의견이 갈리더라구요.

기존 구조: store에 error,pending 관리 -> 비동기 처리를 위한 thunk 사용 이 더 좋은 구조라는 생각을 했어요.

바꾼 구조: useFetch 커스텀훅을 만들었기에 그 안에 있는 error, pending을 개별적으로 사용하는것이 더 좋다는 생각이 들었어요.

현재 문제점:

현재는 page에서 하나의 get 요청을 하고 있지만, 만약 페이지 내부 컴포넌트들에서 각각 api요청을 여러번 한다면 공유하고 있는 error, pending이 겹치기 때문에 문제가 발생할 수 있다 -> store내부에서 api 요청마다 error,pending을 따로 만들어줘야하고 이는 곧 store가 너무 복잡해질 수 있어요.

그래서 여러 방법을 시도해봤는데요~

  1. useFetch에서 {error, pending, data, fetch}를 가져와서 각 컴포넌트 내부에서 실행시키고, error,pending,data 을 useEffect dependencies로 계속 지켜보면서 fetch작업이 끝남을 확인하고 dispatch를 해준다.

    • 하지만 너무 복잡했어요. 하나의 fetch를 위해 최소 2번의 useEffect가 사용되어야하는데 2번만 해도 useEffect가 4개가 되더라구요... 이건 아니다!
  2. 원래 제작했던 thunk 함수의 인자로 useFetch의 fetch를 넘겨주자!

    • 훨씬 간단해질수 있었는데, 훅 규칙에 의해 useFetch를 Thunk함수 내부에서 쓸 수가 없기에 가장 최신화된 값(useFetch의 data)을 넘겨주는 시점이 맞지 않더라구요.
  3. 마지막 방법으로 useFetch의 기능을 확장하여 후행 처리를 했어요.

    • useFetch의 fetch 메서드가 원래 받고 있던 매개변수에서 onSuccess를 추가했습니다.
 axios[method](API_URL, body)
  .then((response) => {
    setPending(false);
    response.data && setData(response.data);
    onSuccess(response.data); // 이렇게 추가했습니다!🎯
  })
  .catch((error) => {
  	...
  }

이렇게 onSuccess를 받아서 지금은 제가 원하는 dispatch 작업을 해줬습니다. 굳이 thunk를 사용할 필요가 없어서 thunk 또한 삭제했습니다.

onSuccess에는 dispatch뿐만 아니라 무엇이든 올 수 있기 때문에 구현은 못했지만 snackBar 호출 같은 기능들이 들어올 수도 있을 것 같아요.


응답코드 잘 파악하기

    const isInCart = cart.some(({id}) => id === Number(productId));

    if (isInCart) {
      return res(ctx.status(505));

장바구니에 없는 productId의 요청에 505 error는 적합하지 않죠.

  • HTTP error code는 규약이므로 상황에 맞는 적절한 error 코드로 개선해보면 좋을 것 같아요.

  • 협업 미션에서도 백엔드 크루들이 이 부분에 굉장히 많은 공을 들이더라구요. 신경쓰도록 합시다!


Styled-Component가 확장하는 컴포넌트의 범위?

const ItemNameLink = styled((props) => <Link {...props} />)

<Link>는 react-router-dom에서 제공하는 컴포넌트죠.

이런 Link를 styled component로 쓰니까 ItemNameLink를 쓰는 곳에서 해당 컴포넌트가 react-router-dom의 Link라는걸 바로 볼 수 없어서 Link에서 사용할 수 있는 여러 가지 메서드나 용법들을 적용하지 못하고 a element로 오인하게 될 수도 있을 것 같아요.

이런 부분을 고려해서 분리하는건 어떨까요?

profile
함께 웃어야 행복한 개발자 장호영입니다😃

0개의 댓글