[클린코드 리액트 3기 김대현] 장바구니 미션 Step1

[pengooseDev]

1. 리뷰 요청 ✍️

안녕하세요 은현님 :) 다른 일정들이 겹쳐, 우선 PR을 생성하고 피드백주신 방향대로 코드를 수정하고자 합니다.

> Project Link

---

2-1. 어렵거나 아쉬웠던 점

현재 주어진 API 중, 상품 리스트만을 MSW가 모킹하고 있습니다. 받아온 데이터를 기반으로 ViewModel(CartManager)이 비즈니스 로직을 처리하는 방식입니다. API 모킹이 제대로 들어가지 않았는데, handler 작성 후 onClick쪽에 api 호출부만 추가하면 완성될 것 같습니다. (시간이 된다면 추가하겠습니다)

2-2. 나누고 싶은 고민

이번 미션 주제인 react-query과는 조금 다른 방식의 상태 관리 도구에 대한 고민입니다.

전역 상태관리도구의 문제점은 추상화 해야하는 위치가 정해져있지 않습니다. 그렇기에 View 내부에서 getter와 setter를 추상화하여 View와 비즈니스 로직의 의존성이 발생하는 문제점을 종종 목격해왔습니다.
redux는 setter를 객체 내부에서 추상화하도록 강제하지만, getter(selector)의 추상화 위치와 dispatcher 호출 위치가 자유로워 이전과 같은 문제가 발생할 가능성이 높습니다.
그 문제를 해결하기위해, 추상화 방식이 상대적으로 자유로운 아톰 기반 상태관리도구 jotai를 사용하여 추상클래스를 만들고 getter와 setter의 구현방식을 강제하는 방식을 도입해보았습니다.

@pengoose/jotai

1. AtomManager

selector와 action을 추상화 할 객체에 상속받는 추상클래스입니다.

import { Atom, WritableAtom, atom } from 'jotai';

export abstract class AtomManager<T> {
  public initialState: T;
  protected atom: WritableAtom<T, any, void>;

  constructor(initialState: T) {
    this.initialState = initialState;
    this.atom = atom(this.initialState);
  }

  // field의 타입을 강제하기보단 원하는 데이터 형태만 반환하는 것이 더 나아보임. 따라서 Atom<T[K]>에서 Atom<any>로 변경
  abstract selectors: {
    [K in keyof Partial<T>]: Atom<any>;
  };

  abstract actions: {
    [key: string]: WritableAtom<T | null, any, void>;
  };
}

2. useManager

AtomManager의 인스턴스를 전달받아 이에 대한 각 selectors와 actions에 useAtomValue, useSetAtom을 각각 래핑한 뒤, 타입 추론을 통해 Atom이 가진 제네릭 타입을 기반으로 selector와 action의 type을 할당하여 반환합니다.

export const useManager = <T extends AtomManager<any>>(manager: T) => {
  /**
   * @description
   * - atomManager의 selectors에 useAtomValue을 래핑하여 반환합니다.
   * - 각 Atom의 type을 추론하여 반환합니다.
   */
  const selectors = Object.fromEntries(
    Object.entries(manager.selectors).map(([key, atom]) => [
      key,
      createUseSelector(atom)(),
    ])
  ) as {
    [P in keyof T['selectors']]: T['selectors'][P] extends Atom<infer V>
      ? V
      : never;
  };

  /**
   * @description
   * - atomManager의 actions에 useSetAtom을 래핑하여 반환합니다.
   * - 각 Atom의 type을 추론하여 반환합니다.
   */
  const actions = Object.fromEntries(
    Object.entries(manager.actions).map(([key, actionAtom]) => [
      key,
      createUseAction(actionAtom)(),
    ])
  ) as {
    [P in keyof T['actions']]: T['actions'][P] extends WritableAtom<
      any,
      infer U,
      void
    >
      ? U[0] extends undefined
        ? () => void
        : (param: U[0]) => void
      : never;
  };

  return { selectors, actions };
};

생각되는 장점

• 1. 상태관리 컨벤션 확립 (하나의 ViewModel 객체 내부에서 getter와 setter가 존재)
• 2. ViewModel => hook(useManager) => Event => View 순서대로 로직을 분리하면 View에서 깔끔하게 비즈니스 로직을 걷어낼 수 있습니다.
• 3. DI 구조를 이용한 설계가 가능합니다. 따라서, 도메인이 커질수록 여러개의 AtomManager로 분리하거나 ServiceLayer를 추가적으로 추상화하여 비즈니스 로직을 깔끔하게 관리할 수 있습니다.

생각되는 단점

해당 방식으로 사용하도록 추상화된 라이브러리가 아니기 때문에, 의존성이 있는 도메인이 커질수록 re-rendering에 대한 고민을 해야합니다. 극단적인 상황이 아니라면, 아직까지 큰 문제가 되진 않아보이지만 jotai 자체에 PR을 생성하여 해당 방식의 최적화 로직을 구현해야합니다.

2-3. 중점적으로 리뷰받고 싶은 부분

1. AtomManager 및 useManager에 대한 은현님의 생각과 추가적인 개선 방향을 피드백 받고싶습니다.
2. View 컴포넌트에서 onClick 로직을 걷어내기위해 components/Event 컴포넌트로 아래와 같이 분리해보았습니다.

import { Event } from '@/components/container/Event/Event';
import { Product } from '@/components/product/Product';
import { useNavigate, useCart } from '@/hooks';
import { Product as ProductData } from '@/types';
import { useQuery } from '@tanstack/react-query';
import { QUERY_CONFIG } from '@/api/queryConfig';

export const List = () => {
  const { moveDetail } = useNavigate();
  const { add } = useCart();
  const { data: listData } = useQuery(QUERY_CONFIG.PRODUCT.GET);

  return (
    <section className="product-container">
      {listData?.map((product: ProductData) => {
        const { id, name, price, imageUrl } = product;

        return (
          <Product key={id}>
            <Event.onClick
              onClick={() =>
                moveDetail(id, {
                  state: {
                    product,
                  },
                })
              }
            >
              <Product.Image src={imageUrl} alt={name} />
            </Event.onClick>

            <Product.InfoContainer>
              <Product.Info name={name} price={price} />
              <Event.onClick onClick={() => add({ product })}>
                <Product.Image src={'assets/svgs/cart.svg'} alt="장바구니" />
              </Event.onClick>
            </Product.InfoContainer>
          </Product>
        );
      })}
    </section>
  );
};

위처럼 구현할 경우, 합성 컴포넌트로 Product 관련 컴포넌트를 관리해야한다는 점이 강제되긴 하지만, onClick을 view에서 걷어낼 수 있다고 생각해서 도입해보았습니다. view에서 action을 걷어내는 더 좋은 방법이 있는지 알고싶습니다.

감사합니다! :)

---

구현된 페이지

상품의 생명주기에 대한 작업을 우선적으로 진행했습니다.
상품 목록 => 상품 디테일 => 장바구니 => 주문
(주문 목록 페이지는 작업하지 않았습니다.)

[devhyun637]

안녕하세요 대현님!
리뷰가 많이 늦어져서 정말 죄송합니다. 🙇‍♀️
수강기간에 상관없이 편할때 다시 리뷰요청 주시면 다음부터는 빠르게 리뷰 드리겠습니다.

일단 코드가 잘 분리가 되어있어서 코드를 쉽게 파악할 수 있었고, 어떤 식으로 코드를 짜셨는지 README나 PR에 자세하게 써주셔서 어떤 부분들을 고민하면서 코드를 작성하셨는지 알 수 있어서 정말 좋았습니다. 중점적으로 받고 싶다고 남겨주신 부분에 대해서 코드 중간중간 간략히 의견을 남겨드렸는데, 추가적으로 더 궁금하신 부분이 있다면 부담없이 질문 남겨주시길 바랍니다.

[devhyun637]

생각하신 디자인이 이런 형태가 맞을까요?

[devhyun637]

여기서 말하는 View는 기본 컴포넌트를 말하는 걸까요? ex) Product

[devhyun637]

추가한 다음에 snackbar, alert 등으로 인터렉션에 대한 안내가 있으면 좀 더 좋을 것 같아요. 지금은 추가를 해도 장바구니에 들어가지 않으면 추가가 되었나? 이 생각이 들어서요 :)

[devhyun637]

상품이 아에 없는 경우에도 체크가 되어있던데, 이건 기획일까요?

[devhyun637]

선택 해제보다는 '전체 선택'이 더 자연스러운 UX일것같은데 어떠신가요?

[devhyun637]

axios가 devDependecy인가요?

[devhyun637]

이 밑에 주석들은 아직 구현전이라서 주석처리를 해두신걸까요?

[devhyun637]

Event를 View와 분리하는 과정에서의 고민을 README에 정리해두신 내용을 바탕으로 잘 읽었습니다 👏
다소 아쉬운 점은 children의 onClick를 활용하는 방법이면 더 좋았을 것 같은데, 전부 button으로 렌더링된다는 것입니다.

view에서 action을 걷어내는 더 좋은 방법이 있는지 알고싶습니다.

지금처럼 시도해보는 방법도 좋은 것 같고, 저는 코드를 보면서 "view에서 action을 왜 분리해야 하나요?"를 질문드리고 싶었습니다. 컴포넌트에서 '비즈니스' 관련된 부분을 hook으로 분리하는 것은 익숙한데, 컴포넌트 자체에 기본적으로 기대하는, 이벤트는 분리할 필요가 있을까? 하는 의문이 들었습니다. 예를들어 아이콘의 경우, 클릭할 수 있는 아이콘들은 IconButton(ex. MUI IconButton)을 만들어서 clickable하다는 것을 나타내는 것처럼요

대현님의
<Event.onClick onClick={클릭이벤트}>{아이콘}</Event.onClick> 과 제가 생각한<IconButton onClick={클릭이벤트}>{아이콘이미지}<IconButton> 중 어느 것이 더 낫다는 건 없습니다. 다만, 처음 코드를 본 사람들이 어떤 것을 더 가독성있게 느낄지 혹은 어떤 것이 더 재사용성이 좋을지에 대한 관점(ex. Event 컴포넌트를 언제 사용할 수 있는지?)에서 스스로 생각을 정리해보시면 "view에서 action을 걷어내는 더 좋은 방법"을 찾으실 수 있을거라 생각합니다.

[devhyun637]

여러 페이지를 쉽게 이동할 수 있는 이 퀵링크 너무 좋은 것 같아요 👏👏
div 태그대신에 시멘틱한 태그를 활용해보는 건 어떨까요?

[devhyun637]

AtomManager 및 useManager에 대한 은현님의 생각과 추가적인 개선 방향을 피드백 받고싶습니다.

지금의 방향성도 저는 많은 고민을 하고 짜신 것 같아서 괜찮은 것 같습니다.
또한, 추가적인 개선 방향도 성능관련해서 고민을 하고 계신것 같아서 개선을 하고 싶다면 성능 쪽을 고려해보는 것은 어떨지 의견을 드립니다.

++ 다른 관점의 의견)
이번 미션의 필수 요구사항이 '조타이 상태관리 사용'이었을까요?
저는 개인적으로 어떤 부분에서 클라이언트 상태 관리가 필요해서 Tanstack-query 외에도 jotai로 상태관리를 하려고 했던 것인지가 궁금합니다.