Skip to content

[5팀 김유현] Chapter 2-2. 디자인 패턴과 함수형 프로그래밍#31

Open
yuhyeon99 wants to merge 51 commits intohanghae-plus:mainfrom
yuhyeon99:main
Open

[5팀 김유현] Chapter 2-2. 디자인 패턴과 함수형 프로그래밍#31
yuhyeon99 wants to merge 51 commits intohanghae-plus:mainfrom
yuhyeon99:main

Conversation

@yuhyeon99
Copy link

@yuhyeon99 yuhyeon99 commented Aug 4, 2025

과제 링크

https://yuhyeon99.github.io/front_6th_chapter2-2/

과제의 핵심취지

  • React의 hook 이해하기
  • 함수형 프로그래밍에 대한 이해
  • 액션과 순수함수의 분리

과제에서 꼭 알아가길 바라는 점

  • 엔티티를 다루는 상태와 그렇지 않은 상태 - cart, isCartFull vs isShowPopup
  • 엔티티를 다루는 컴포넌트와 훅 - CartItemView, useCart(), useProduct()
  • 엔티티를 다루지 않는 컴포넌트와 훅 - Button, useRoute, useEvent 등
  • 엔티티를 다루는 함수와 그렇지 않은 함수 - calculateCartTotal(cart) vs capaitalize(str)

기본과제

  • Component에서 비즈니스 로직을 분리하기
  • 비즈니스 로직에서 특정 엔티티만 다루는 계산을 분리하기
  • 뷰데이터와 엔티티데이터의 분리에 대한 이해
  • entities -> features -> UI 계층에 대한 이해
  • Component에서 사용되는 Data가 아닌 로직들은 hook으로 옮겨졌나요?
  • 주어진 hook의 책임에 맞도록 코드가 분리가 되었나요?
  • 계산함수는 순수함수로 작성이 되었나요?
  • Component에서 사용되는 Data가 아닌 로직들은 hook으로 옮겨졌나요?
  • 주어진 hook의 책임에 맞도록 코드가 분리가 되었나요?
  • 계산함수는 순수함수로 작성이 되었나요?
  • 특정 Entitiy만 다루는 함수는 분리되어 있나요?
  • 특정 Entitiy만 다루는 Component와 UI를 다루는 Component는 분리되어 있나요?
  • 데이터 흐름에 맞는 계층구조를 이루고 의존성이 맞게 작성이 되었나요?

심화과제

  • 재사용 가능한 Custom UI 컴포넌트를 만들어 보기
  • 재사용 가능한 Custom 라이브러리 Hook을 만들어 보기
  • 재사용 가능한 Custom 유틸 함수를 만들어 보기
  • 그래서 엔티티와는 어떤 다른 계층적 특징을 가지는지 이해하기
  • UI 컴포넌트 계층과 엔티티 컴포넌트의 계층의 성격이 다르다는 것을 이해하고 적용했는가?
  • 엔티티 Hook과 라이브러리 훅과의 계층의 성격이 다르다는 것을 이해하고 적용했는가?
  • 엔티티 순수함수와 유틸리티 함수의 계층의 성격이 다르다는 것을 이해하고 적용했는가?

과제 셀프회고

과제를 하면서 내가 제일 신경 쓴 부분

명확한 책임 분리를 통한 예측 가능한 코드 작성

이번 과제를 진행하면서 가장 중점을 둔 부분은 함수와 컴포넌트의 단일 책임 원칙(SRP)을 지키는 것이었습니다. 특히, 작은 유틸리티 함수부터 시작하여 코드의 예측 가능성을 높이고자 노력했습니다.

구체적인 사례: formatPrice 함수의 분리

  • 문제점
    기존 formatPrice 함수는 isAdmin이라는 boolean 플래그를 받아 관리자용 포맷과 사용자용 포맷을 분기 처리했습니다.
// 기존 방식
export const formatPrice = (price: number, isAdmin: boolean = false): string => {
  if (isAdmin) {
    return `${price.toLocaleString()}원`; // 관리자용
  }
  return `₩${price.toLocaleString()}`; // 사용자용
};
  • 개선점
    이 방식은 함수를 사용하는 쪽에서 항상 isAdmin 플래그를 신경 써야 하고, 함수의 이름만으로는 어떤 결과가 나올지 예측하기 어렵다는 단점이 있었습니다.
    이 문제를 해결하기 위해, 각기 다른 책임을 가진 formatPriceForAdminformatPriceForShop 두 개의 명시적인 함수로 분리했습니다.
// 개선된 방식
export const formatPriceForAdmin = (price: number): string => {
  return `${price.toLocaleString()}원`;
};

export const formatPriceForShop = (price: number): string => {
  return `₩${price.toLocaleString()}`;
};
  • 결과
    이렇게 분리함으로써, 함수를 사용하는 AdminPage.tsxShoppingPage.tsx에서는 더 이상 조건부 로직에 대해 고민할 필요 없이 필요한 함수를 직접 가져와 사용할 수 있게 되었습니다.
    코드의 가독성과 예측 가능성이 향상되었고, 각 함수의 책임이 명확해져 유지보수가 용이해졌습니다.

과제를 다시 해보면 더 잘 할 수 있었겠다 아쉬운 점

페이지 컴포넌트의 과도한 책임

리팩토링을 통해 많은 부분을 개선했지만, 페이지 레벨 컴포넌트(ShoppingPage.tsx, AdminPage.tsx)가 여전히 너무 많은 책임을 가지고 있다는 점이 아쉽습니다.
컴포넌트 분리가 더 세밀하게 이루어졌다면 좋았을 것 같습니다.

구체적인 사례: ShoppingPage.tsx의 복잡성

  • 문제점
    ShoppingPage.tsx는 상품 목록, 장바구니, 쿠폰 적용, 검색 등 쇼핑 페이지의 거의 모든 상태와 로직을 총괄하고 있습니다.

    • useProducts, useCart, useCoupons, useNotifications 등 여러 커스텀 훅을 동시에 사용하여 데이터를 가져옵니다.
    • 검색어 상태(searchTerm, debouncedSearchTerm)와 그에 따른 필터링 로직을 직접 포함하고 있습니다.
    • completeOrder와 같은 주요 비즈니스 로직 콜백 함수를 페이지 컴포넌트 내에서 정의하고 있습니다.
  • 아쉬운 점
    이로 인해 ShoppingPage는 30줄이 훌쩍 넘는 상태 관리 로직과 100줄이 넘는 JSX를 가지게 되어 비대해졌습니다.
    예를 들어, 상품 목록을 보여주는 부분과 장바구니 부분을 각각 별도의 컨테이너 컴포넌트(ProductListContainer, CartContainer)로 분리했다면,
    ShoppingPage는 이들의 레이아웃만 담당하게 되어 훨씬 간결해졌을 것입니다.
    각 컨테이너가 필요한 데이터만 주입받도록 설계하여 관심사를 분리하지 못한 점이 가장 아쉽습니다.


리뷰 받고 싶은 내용이나 궁금한 것

  1. formatPrice 함수 분리 방식에 대한 피드백

    • 현재 formatPriceForAdminformatPriceForShop으로 역할을 분리한 방식에 대해 어떻게 생각하시는지 궁금합니다.
      SRP를 잘 적용한 사례라고 볼 수 있을까요?
      혹은, 이렇게 작은 기능은 하나의 함수에 type과 같은 옵션을 받아 처리하는 것이 더 나은 설계였을지, 실무에서는 어떤 패턴을 더 선호하는지 궁금합니다.
  2. 거대 페이지 컴포넌트의 효과적인 분리 전략

    • ShoppingPage.tsx와 같이 여러 도메인의 상태(상품, 장바구니, 쿠폰)를 조합해야 하는 페이지 컴포넌트를 리팩토링하는 더 좋은 전략이 궁금합니다.
    • 제가 아쉬운 점으로 꼽았듯이, 각 섹션(상품 목록, 장바구니)을 컨테이너 컴포넌트로 분리하고 필요한 훅을 각각의 컨테이너에서 호출하는 방식이 괜찮은 접근일까요?
      이 경우, 컴포넌트 간에 상태 공유가 필요할 때(예: 상품을 장바구니에 담는 상호작용)는 어떻게 처리하는 것이 가장 효율적인지에 대한 조언을 구하고 싶습니다.

App.tsx에 혼재되어 있던 계산 관련 함수들을 utils/calculators.ts 파일로 분리

- getMaxApplicableDiscount
- calculateItemTotal
- calculateCartTotal
- `formatPrice` 함수를 `src/basic/utils/formatters.ts`로 이동
- `validateProductForm` 함수를 `src/basic/utils/validators.ts`로 생성 및 분리
- `src/basic/hooks/useLocalStorage.ts` 파일 생성 및 로컬 스토리지 관리 로직 이동
- `App.tsx`에서 `products`, `cart`, `coupons` 상태에 `useLocalStorage` 훅 적용
- 기존 `localStorage` 관련 `useEffect` 훅 제거
Hook 분리 (`useProducts`, `useCart`, `useCoupons`):
* 상품, 장바구니, 쿠폰의 상태(state)와 관련 함수(CRUD, 수량 변경 등)를 App.tsx에서 각 useProducts, useCart, useCoupons Hook으로 이전했습니다.
타입 및 유틸리티 개선:
* types.ts: UI를 위한 ProductWithUI 타입을 추가하고, initialCoupons 데이터를 정의했습니다.
* formatters.ts: formatPrice 함수가 재고 상태에 따라 'SOLD OUT'을 표시하도록 기능을 개선했습니다.
- `src/basic/components/ui/Button.tsx` 및 `src/basic/components/ui/Notification.tsx` 파일 생성
- `App.tsx` 내의 기존 버튼 및 알림 UI를 새로 분리된 `Button` 및 `Notification` 컴포넌트로 대체
- `Button` 컴포넌트에 `icon` prop을 추가하여 유연성 향상
- `src/basic/components/ProductCard.tsx` 파일에 상품 설명, 할인 정보, 재고 상태 표시 추가
- `src/basic/components/CartItem.tsx` 파일에 할인율 표시 추가
- `ShoppingPage.tsx`: 상품 목록, 장바구니, 쿠폰 적용, 결제 로직 등 쇼핑몰 관련 기능을 담당합니다.
- `AdminPage.tsx`: 상품 및 쿠폰 관리 등 관리자 관련 기능을 담당합니다.
- `App.tsx`는 이제 `isAdmin` 상태에 따라 두 페이지 컴포넌트를 렌더링하는 역할만 수행합니다.
- `addNotification` 함수가 필요한 하위 컴포넌트 및 훅으로 올바르게 전달되도록 수정했습니다.
formatPrice 함수의 매개변수 구조 변경에 따라 AdminPage.tsx에서 잘못된 인자(product.id)를 전달하던 문제를 해결했습니다.
- `vite.config.ts`를 수정하여 `src/advanced` 프로젝트의 GitHub Pages 배포를 위한 `base` 경로, 빌드 출력 디렉토리(`dist/advanced`), 그리고 진입점(`index.advanced.html`)을 설정
- GitHub Actions 워크플로우 파일 `deploy.yml`을 추가하여 `main` 브랜치 푸시 시 `src/advanced` 프로젝트를 빌드하고 GitHub Pages(`gh-pages` 브랜치)로 자동 배포하도록 구성
 `CartItem.tsx` 파일에서 컴포넌트 이름과 `types.ts`의 `CartItem` 인터페이스 이름이 충돌하여 발생하던 `Individual declarations in merged declaration 'CartItem' must be all exported or alllocal` 오류를 해결했습니다.
Copy link

@JunilHwang JunilHwang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

안녕하세요! PR 정리와 리팩토링 고생 많으셨습니다. 요청하신 흐름(변화 시나리오 → 해당 시나리오에서 발생할 변화 → 이를 바탕으로 응집도/결합도 평가 및 개선안 제시)에 맞춰 종합 피드백과 상세 피드백을 마크다운 형식으로 정리했습니다. PR 파일 전체를 훑어보며 확인한 주요 키워드, 문제 및 개선안(AS-IS / TO-BE 코드 예시 포함)을 포함합니다.

요약:

  • 전반적 구조 개선(모노 App → pages/components/hooks/atoms 분리)은 잘 진행됨
  • Jotai 기반 상태관리 전환/분리(이후의 유지보수/교체를 고려한 추상화)는 일부 준비되어 있으나, 상태 라이브러리 변경 대비를 위한 “어댑터 레이어”·“핵심 도메인(순수 함수) 분리”가 더 필요
  • Jotai API(특히 write-atom 호출 방식) 사용에 혼선이 보입니다(다수의 set(addNotificationAtom, msg, type) 호출 — Jotai 표준 호출은 단일 payload 전달입니다). 이 부분은 코드 버그로 이어질 가능성이 큽니다.

목차

  1. 종합 피드백
  • 키워드(문제/장점)
  • PullRequestBody에 적어둔 회고/집중한 부분에 대한 인사이트 + 질문
  • PR에서 리뷰 받고 싶은 내용(빈칸)에 대한 답변/권장점검사항
  1. 상세 피드백 (개념 정의 → 문제 정의 → AS-IS → TO-BE)
  • 결합도 / 응집도 정의(제안된 규칙 포함)
  • 주요 문제 항목별 상세 개선안 및 코드 스니펫
  1. 우선순위 액션 아이템

  1. 종합 피드백
  • 핵심 키워드 (PullRequestFiles 기반)

    • 모듈화: components/pages/ui / hooks / atoms / utils(참조) 분리 — 장점
    • 상태관리 결합: Jotai 전용 API(원시 atom 호출 방식)에 대한 의존성 잔존 — 위험
    • 알림 API 불일치: addNotificationAtom 사용 방식(여러 인자 전달) — 잠재적 버그
    • 도메인/유틸 분리: calculate*, format* 같은 순수함수로 분리하려는 시도 — 긍정적
    • Packagability: 도메인(entities + calculators)과 UI/adapter 분리하면 패키지화 용이
    • 테스트: 기존 통합 테스트 유지·수정하려는 노력 관찰(테스트 코드 정리됨)
  • PullRequestBody(과제의 핵심취지/학습포인트)에 대한 인사이트

    • 좋은 점
      • 과제 취지(엔티티 vs UI 분리, 훅/순수함수 분리 등)를 의도적으로 반영하여 리팩터링 했음(entities 훅/atoms / UI 컴포넌트). 실무에서의 권장 패턴을 잘 따름.
      • UI 컴포넌트(Button, Notification)와 엔티티 컴포넌트(CartItem, ProductCard)를 분리한 점이 재사용성과 테스트 편의성을 높임.
    • 더 생각해볼 거리 (질문형 인사이트)
      • "도메인 로직(할인 계산, 총액 계산 등)을 완전히 순수함수로 분리했을 때 어떤 이득/단점이 있을까?" — (이득: 테스팅과 패키징 편의, 단점: 상태 동기화/캐싱 고려 필요)
      • "현재 훅(useCart, useProducts 등)은 Jotai 기반으로 구현되어 있는데, 외부에서 이 훅의 내부 구현(예: atom 대신 redux)을 바꿔야 한다면 가장 덜 번거로운 경로는 무엇일까?" — (답: 훅을 어댑터처럼 유지하고 내부에서 state adapter를 교체할 수 있게 하는 레이어 분리)
      • "UI 컴포넌트는 현재 상태벤트 핸들러를 props로 받고 있는데, 엔티티 훅들의 반환 인터페이스는 얼마나 '표준화' 되어 있는가?" — (표준화된 API 반환은 라이브러리 교체 시 교체 범위를 줄여줌)
    • 권장 질문(팀에게 던져볼 만한 질문)
      • 이 프로젝트를 라이브러리(패키지)로 배포한다면 어떤 단위(도메인 vs adapter vs UI)로 분리해서 배포할 것인가?
      • Jotai를 대체할 때 테스트/동작이 깨지는 주요 영역(예: 알림 비동기 제거, localStorage atomWithStorage 동작) 은 어디인가?
  • PullRequestBody의 "리뷰 받고 싶은 내용" (빈칸)에 대한 답변 / 권장점검

    • "테스트가 통과하는지" — 우선 로컬에서 vitest / npx vitest run 으로 전체 테스트 실행 권장. (Jotai multi-arg 사용 부분 때문에 일부 테스트 실패 가능성)
    • "라이브러리 변경 대비" — useCart/useProducts/useCoupons/useNotifications 훅이 외부에 안정적인 인터페이스를 제공하므로, 내부 어댑터만 교체하면 됨. 그러나 현재 addNotificationAtom 호출 방식 등 내부 API 불일치로 인해 교체에 오류 발생 가능.
    • 권장: CI에서 vitest, lint 돌리는 과정 확립(현재 package.json에 스크립트 있음). PR에 테스트 결과/빌드 상태 캡처 첨부 권장.

  1. 상세 피드백

A. 개념 정의 — 응집도 / 결합도(요청하신 규칙으로 정의)

  • 응집도(cohesion) 정의 (제안 규칙)

    • 변경에 대한 파일/코드의 추가·수정·삭제 경로가 얼마나 짧은가(짧을수록 응집도가 높다)
    • 라이브러(패키지)로 분리할 때 매끄럽게 떼어낼 수 있는가(즉, 의존성이 명확히 경계화되어 있는가)
    • 하나의 모듈이 '하나의 책임'을 갖고 관련된 함수/데이터만 모아두었는가
  • 결합도(coupling) 정의

    • 모듈 간 연결이 인터페이스를 통해 이뤄지는가 (낮은 결합도), 또는 내부 구현(라이브러 API 등)에 직접 의존하는가 (높은 결합도)
    • 예: 안좋은 예시는 addNotification를 직접 인자로 받아 특정 함수 내부에서 고정명으로 호출하는 경우. 좋은 예시는 onSuccess/onError 같은 일반화된 콜백 인터페이스를 받는 것

B. 문제 목록(정의 → AS-IS → TO-BE / 코드 예시 포함)

문제 1 — Jotai write-atom 호출 형태 불일치(심각/버그 위험)

  • 문제 정의
    • 여러 곳에서 set(addNotificationAtom, '...', 'error') 형식으로 호출하고 있습니다. Jotai의 write-atom(두 번째 인자)은 하나의 payload만 받는 것이 표준입니다. (write 함수가 (_get, set, payload)로 한 인자만 받음)
    • 현재 코드 대부분이 set(addNotificationAtom, message, type)처럼 2개 인자를 전달하고 있어 동작하지 않거나 예기치 않은 동작을 초래할 수 있습니다.
  • AS-IS (cartAtoms 중 일부 발췌)
    • cartAtoms.ts (발췌)
      set(addNotificationAtom, '재고가 부족합니다!', 'error');
  • 이유(왜 문제인지)
    • write-atom의 update 매개변수는 하나입니다. 위 코드는 message에 '재고가 부족합니다!'가 들어가고 type은 undefined가 됩니다. 결과: notification type 누락, setTimeout/clear 동작은 되더라도 타입 기반 스타일링/로직에 버그 발생.
  • TO-BE (권장 변경)
    • 두 가지 방식 제안:
      1. payload를 객체로 전달:
        // atoms/notificationAtoms.ts (writer)
        export const addNotificationAtom = atom(
          null,
          (_get, set, payload: { message: string; type: 'error'|'success'|'warning' }) => {
            set(notificationAtom, payload);
            setTimeout(() => set(notificationAtom, null), 3000);
          }
        );
        
        // 호출부
        set(addNotificationAtom, { message: '재고가 부족합니다!', type: 'error' });
      2. 헬퍼 훅을 통해 숨기기(권장: API 안정화)
        // hooks/useNotifications.ts
        export const useNotifications = () => {
          const notification = useAtomValue(notificationAtom);
          const setNotif = useSetAtom(addNotificationAtom);
          const addNotification = (message: string, type: 'error'|'success'|'warning' = 'success') => {
            setNotif({ message, type });
          }
          return { notification, addNotification };
        }
        
        // 호출부 (기존 코드들)
        const { addNotification } = useNotifications();
        addNotification('재고가 부족합니다!', 'error');
      • 이유: 훅으로 캡슐화하면 내부 상태 라이브러리를 바꿔도 호출부(components/hooks/atoms)가 바뀌지 않음

문제 2 — 훅과 atom 사이의 결합: 훅이 Jotai에 너무 직접적으로 종속됨(교체 비용 증가)

  • 문제 정의
    • 현재 useCart/useProducts/useCoupons 훅은 내부적으로 Jotai atom을 그대로 노출하거나 Jotai setter를 반환합니다. (예: useSetAtom(updateQuantityAtom) 반환)
    • 이 경우 Jotai → Redux/Zustand 등으로 바꿀 때 훅 내부만 바꾸면 되지만, write-atom 호출 규약 문제(위)와 맞물리면 예상치 못한 수정이 커집니다.
  • AS-IS (useCart)
    export const useCart = () => {
      const cart = useAtomValue(cartAtom);
      const addToCart = useSetAtom(addToCartAtom);
      const removeFromCart = useSetAtom(removeFromCartAtom);
      const updateQuantity = useSetAtom(updateQuantityAtom);
      const getRemainingStock = useAtomValue(getRemainingStockAtom);
      const calculateItemTotal = useAtomValue(calculateCartItemTotalAtom);
      return { cart, addToCart, removeFromCart, updateQuantity, getRemainingStock, calculateItemTotal };
    }
  • TO-BE (권장)
    • 훅은 "행위(behavior) 인터페이스"만 노출하고 내부는 어떤 스토어를 쓰든 동일한 시그니처를 유지하도록 캡슐화.
    • 예:
      // hooks/useCart.ts (인터페이스 유지)
      export type UseCartReturn = {
        cart: CartItem[];
        addToCart: (product: ProductWithUI) => void;
        removeFromCart: (productId: string) => void;
        updateQuantity: (productId: string, qty: number) => void;
        getRemainingStock: (p: ProductWithUI) => number;
        calculateItemTotal: (item: CartItem) => number;
      };
      
      export const useCart = (): UseCartReturn => {
        // 내부에서 jotai, zustand 등 어떤 스토어든 사용
        // 외부는 변경 없음
      };
    • 이 패턴을 적용하면 state 라이브러리 교체 시 훅 구현부만 바꾸면 됨(컴포넌트 불변)

문제 3 — 응집도 관점: 할인/계산 관련 유틸(순수함수)이 어디에 있는가?

  • 문제 정의
    • PR에서 calculateCartTotal / calculateItemTotal 등은 utils로 분리되어 사용하는 형태로 보입니다(정상). 다만 현재 atoms 내에서 utils에 의존하고 있고, utils의 함수 시그니처가 명확해야 외부에서 재사용·패키징 가능.
  • AS-IS
    • atoms/couponAtoms.ts:
      import { calculateCartTotal } from '../utils/calculators';
      const currentTotal = calculateCartTotal(cart, selectedCoupon).totalAfterDiscount;
  • 권장 TO-BE
    • 도메인(패키지)로 추출: src/domain/cart.ts (types + 순수 계산 함수) — tests만 해당 디렉토리로 의존
    • 예:
      // domain/cart.ts (pure)
      export function calculateItemTotal(item: CartItem, allCart: CartItem[]): number { ... }
      export function calculateCartTotal(cart: CartItem[], coupon?: Coupon): { totalBeforeDiscount:number, totalAfterDiscount:number } { ... }
    • atoms/adapter는 위 순수함수만 호출(결국 분리되어 패키지화 가능)

문제 4 — 응집도/모듈화: 파일 배치와 패키징 고려

  • 문제 정의
    • 현재 파일 구조는 개선된 편(components / hooks / atoms / utils)이지만, 'domain' vs 'state-adapter' vs 'ui' 경계가 완전히 분리되어 있지 않음
    • 패키지로 추출할 때 핵심은 domain(엔티티 + 순수함수)만 떼어내는 것
  • 권장 구조(예)
    • src/
      • domain/ <-- package로 떼기 좋음 (types, calculators, initial-data)
        • cart.ts
        • product.ts
        • coupon.ts
        • index.ts (export)
      • state/
        • jotai/ <-- 현재 atoms 구현체 (jotai에 특화)
        • redux/ <-- (나중에 다른 라이브러리 구현체를 둘 수 있음)
        • adapter.ts <-- export hook들을 여기서 재노출 (useCart, useProducts 등)
      • ui/
        • components/
        • pages/
      • hooks/ <-- UI가 사용하는 훅(최상위 훅들은 adapter를 통해 state에 접근)
      • utils/
  • 이 구조를 따르면 'domain' 패키지만 추출하면 UI와 상태 라이브러리를 떼어낼 수 있어 응집도와 패키징 편의성이 높아짐

문제 5 — 일부 세부 구현/버그 가능성 (테스트/동작 관련)

  • Notification: setTimeout 내부에서 클로저나 중복 알림 처리 고려 필요(동시에 여러 알림 발생 시 last only 로 표시되는 현재 형태가 의도된 것인지 확인)
  • atomWithStorage 와 localStorage 초기화 로직은 잘 되어 있지만, 원시값 undefined 처리 방식(예: cart 비어있을 때 삭제) 확인 필요
  • 테스트 코드에서 문자열 비교 등 DOM 기반 테스트는 fragile(테스트 깔끔화 권장)

  1. 개선 제안(우선 순위별)

긴급(빠르게 수정 필요)

  1. addNotificationAtom 호출 방식 통일 및 훅 캡슐화

    • 모든 set(addNotificationAtom, msg, type) → set(addNotificationAtom, {message,type}) 또는 useNotifications().addNotification(...) 으로 변경
    • (AS-IS → TO-BE 코드 예시 위에 있음)
  2. Jotai write-atom 호출 가이드 주석 또는 util 함수 추가

    • 팀원들이 혼동하지 않도록 토큰형 doc 주석 추가

중간 우선
3. useCart/useProducts 등 훅 인터페이스 명세화(타입 선언)

  • UseCartReturn 같은 타입을 선언하여 내부 구현 유연성 확보
  1. 도메인(domain) 코드(계산기, 타입, 초기데이터) 분리
    • domain 패키지로 옮기고, atoms/hooks는 domain을 소비하도록 변경

장기(리팩토링 단계)
5. state-adapter 레이어 도입

  • state/jotai/* 와 state/zustand/* 처럼 대체 라이브러리별 구현을 만들어 같은 훅 시그니처로 노출
  • 예: hooks/useCart.ts는 state/adapter/index.ts에서 적절한 구현(useJotaiCart/useZustandCart)을 export 하도록 함
  1. Notification queue 또는 다중 알림 처리 개선
    • 현재는 single notificationAtom에 덮어쓰기 방식. 다수 알림을 동시에 보여줄 경우 queue/배열 사용 권장

구체적 AS-IS / TO-BE 예시 (주요 항목)

A) addNotification API 변경 예시

AS-IS (문제 코드)

  • atoms/notificationAtoms.ts
    export const addNotificationAtom = atom(
      null,
      (_get, set, message: string, type: 'error' | 'success' | 'warning') => {
        set(notificationAtom, { message, type });
        setTimeout(() => {
          set(notificationAtom, null);
        }, 3000);
      }
    );
    // 호출부 예: set(addNotificationAtom, '재고가 부족합니다!', 'error');

TO-BE (수정안 1: payload 객체)

  • atoms/notificationAtoms.ts
    export type NotificationPayload = { message: string; type: 'error'|'success'|'warning' };
    export const addNotificationAtom = atom(
      null,
      (_get, set, payload: NotificationPayload) => {
        set(notificationAtom, payload);
        setTimeout(() => set(notificationAtom, null), 3000);
      }
    );
    // 호출부
    set(addNotificationAtom, { message: '재고가 부족합니다!', type: 'error' });

TO-BE (권장안 2: 훅 캡슐화)

  • hooks/useNotifications.ts
    export const useNotifications = () => {
      const notification = useAtomValue(notificationAtom);
      const setNotif = useSetAtom(addNotificationAtom);
      const addNotification = (message: string, type: 'error'|'success'|'warning' = 'success') => {
        setNotif({ message, type });
      };
      return { notification, addNotification };
    };
    // 호출부는 항상 addNotification(...) 사용

B) useCart 인터페이스 표준화 예시

AS-IS

export const useCart = () => {
  const cart = useAtomValue(cartAtom);
  const addToCart = useSetAtom(addToCartAtom);
  const removeFromCart = useSetAtom(removeFromCartAtom);
  const updateQuantity = useSetAtom(updateQuantityAtom);
  const getRemainingStock = useAtomValue(getRemainingStockAtom);
  const calculateItemTotal = useAtomValue(calculateCartItemTotalAtom);

  return { cart, addToCart, removeFromCart, updateQuantity, getRemainingStock, calculateItemTotal };
};

TO-BE (명확한 타입 + 내부 캡슐화)

export type UseCartReturn = {
  cart: CartItem[];
  addToCart: (p: ProductWithUI) => void;
  removeFromCart: (id: string) => void;
  updateQuantity: (id: string, qty: number) => void;
  getRemainingStock: (p: ProductWithUI) => number;
  calculateItemTotal: (item: CartItem) => number;
};

export const useCart = (): UseCartReturn => {
  // 내부는 state-adapter가 담당
  // 반환 시그니처는 변경하지 않음
};

C) 패키지화(모듈화) 제안 예시 — domain 패키지 분리

  • domain/index.ts
    // exports for package
    export * from './types';
    export * from './calculators';
    export * from './initialData';
  • state/jotai/* 는 domain을 import 해서 atom을 구성
  • UI는 domain의 타입과 계산 함수만 참조하고 state는 훅(adapter)를 통해 주입

  1. PR 파일별 간단 코멘트(핵심 파일들)
  • src/advanced/App.tsx

    • 장점: 모노리스 제거, 페이지 컴포넌트로 위임
    • 이슈: useNotifications() 사용부에서 onClose를 주석처리 해둔 부분(주석에 "Handled by atom's setTimeout") → Notification 컴포넌트의 onClose prop이 아무 동작도 하지 않음. 만약 사용자가 수동으로 닫는 UI를 원하면 훅에 명시적 clear 함수(예: clearNotification()) 추가 권장.
  • src/advanced/atoms/*

    • 장점: 도메인별 atoms 분리(제품/쿠폰/카트/노티)로 응집도 좋음
    • 이슈: addNotificationAtom 호출 방식(여러 인자 전달) → 전 파일에서 동일한 패턴 사용 → 통합 변경 필요
  • src/advanced/hooks/*

    • 장점: 훅이 atoms를 캡슐화하여 컴포넌트가 직접 atom을 건드리지 않음(일부는 여전히 setAtom 반환)
    • 개선: 훅이 외부에 노출하는 시그니처를 명확히 타입화
  • components (CartItem, ProductCard, pages)

    • 전반적으로 좋은 분리. Button/Notification 컴포넌트 잘 만듦.
    • Button의 variant 스타일이 하드코딩되어 있음(테마/토큰별로 분리하면 더 좋음).
  • src/advanced/tests/*

    • 테스트 코드도 포맷/정리 되어 있음. 다만 addNotification의 자동 사라짐 테스트가 setTimeout 기반인데, jest/vitest fake timers를 활용하면 테스트가 더 안정적.

  1. 우선순위 액션 아이템(간단 체크리스트)
  1. (필수) addNotificationAtom API 통일 / useNotifications 훅으로 캡슐화 → 코드베이스 전역 교체
  2. (권장) useCart/useProducts 등 훅의 반환 타입 정의 및 문서화
  3. (권장) domain(계산 함수, 타입, initial data) 폴더로 이동 — 패키지화 준비
  4. (권장) Notification을 배열 기반으로 확장할지(다중 알림) 판단 → UX 요구에 맞게 구현
  5. (권장) CI에서 vitest/ESLint/Prettier 실행 확인, failing test/ lint fix

마무리 및 질문(후속 요청)

잘하셨습니다 — 구조 개선, 컴포넌트 분리, 훅 분리 등 목표와 과제 취지를 잘 반영하셨습니다. 다만 "상태 라이브러리 변경" 시 가장 큰 걸림돌은 현재 Jotai-specific 패턴(특히 write-atom 호출 방식)과 훅이 Jotai API에 다소 노출된 부분입니다. 훅을 통해 상태 라이브러리와의 인터페이스를 표준화하면 변경 비용을 많이 줄일 수 있습니다.

제가 도와드릴 수 있는 다음 작업(선택):

  • addNotificationAtom → payload / 훅 캡슐화로 전역 일괄 리팩토링 PR 패치 생성
  • domain 추출(예: src/domain으로 이동) + 예제 패키지화 구조 제안 및 코드 스니펫 제공
  • Jotai → Zustand / Redux / Tanstack Query 로 전환하는 어댑터 샘플(예: useCart의 zustand 구현 예시)
  • CI 설정(vitest / lint) 확인 및 실패 원인 상세 디버깅

몇 가지 확인 질문:

  1. addNotificationAtom를 "single notification" 방식(현재)으로 유지할지, 아니면 다중 알림 큐(여러 개 동시에 표시)로 변경할지 UX 요구가 있으신가요?
  2. 향후 상태 라이브러리 교체를 실제로 진행하실 계획이 있나요? (있다면 우선순위가 어떻게 되나요 — 예: Zustand, Redux, TanStack 등)
  3. domain 패키지 분리 시 public API(어떤 함수/타입을 노출할지)에 대한 선호가 있으신가요? (예: 계산 함수만, 혹은 initialData까지 포함)

원하시면 'AS-IS → TO-BE' 자동 패치(PR용) 형태로 변경사항(diff)을 제가 제안해 드릴 수 있습니다. 어느 부분부터 적용해볼까요?

Comment on lines +36 to +39
useEffect(() => {
const count = cart.reduce((sum, item) => sum + item.quantity, 0);
setTotalItemCount(count);
}, [cart]);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이 로직이 그냥 useCart 안에 있었어도 좋았을 것 같아 남겨둡니당

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants