[PR 코드리뷰] 우아한테크코스 프론트엔드 레벨2 - 리액트 장바구니 미션

우아한테크코스 프론트엔드 과정 레벨2 <리액트 페이먼츠 미션>을 수행하며 우아한형제들의 @Vallista 님께 리뷰를 받을 수 있었다. 마치 1학년 때 졸업반 수업에 들어가서 첨삭받은 느낌을 받았다...! 여느 때보다 더 오랜 시간 피드백에 대해 고민하고 반영했는데, 그만큼 많이 배울 수 있었다.

 

미션 1단계 피드백

👉 PR 바로가기

작성한 컴포넌트
atoms(10): Template, Button, Checkbox, Line, List, UnderlinedText, TrashCanIcon, CartIcon, Upward/DownwardIcon
molecules(6): CheckoutBox, Confirm, Header, Navbar, QuantityStepper, RedirectNotice

1. 컴포넌트 분리의 미학

컴포넌트 분리와 같이 다양한 관점을 접할 수 있는 이 글을 읽어보자.

아토믹 디자인에서 Atoms, Molecules로 공통된 레이아웃 컴포넌트를 만들기 좋고, Organisms, Template 레벨은 프로젝트의 한정된 레이아웃을 뽑는데 좋다. Organisms 레벨부터는 비즈니스 로직를 보관하는 Container와 1:1 로 매칭시켜서 비즈니스 로직을 녹여내는 방법도 좋다.

Molecules는 비즈니스 로직을 직접 갖고 있기 보다, props로 의존성을 주입받는 것이 더 낫다. confirmAction과 같은 비즈니스 로직은 필요없는 상황도 존재한다. 직접 갖고 있다면 redux와 강한 결합이 되어 redux에 수정이 일어날 경우 해당 컴포넌트에도 영향을 미치게 되고 재사용성도 떨어진다.

/* 개선 전 */

export const Confirm = (props) => {
  const { isOpen, message, onApprove, onCancel } = props;

  return (
  ...
    <S.CancelButton onClick={onCancel}>취소</S.CancelButton>
  );
};

/* 개선 후 */

export const Confirm = (props) => {
  const { isOpen, message, onApprove, onCancel } = props;

  return (
  ...
    <S.CancelButton onClick={onCancel}>취소</S.CancelButton>
  );
};

2. 적절한 파일링 / 폴더링

./pages는 재사용하는 component가 아니기 때문에 ./components 밖에 있는 것이 더 자연스럽다. ./pages와 ./components 레벨을 분리하여 페이지에서 컴포넌트를 가져다가 레이아웃을 만든다는 흐름이 더 적절하다.

또, 지금은 양이 적어서 ducks 패턴을 사용하면서 파일 하나에 명세가 되지만, 추후에는 분리를 해도 좋겠다. (이 글에서 중간 스크롤 쯤의 이미지 참고)

테스트 코드에 포함된 mock 부분은 .mock.js 정도로 분리하는게 좋다.

3. 적극적인 커스텀훅 사용

커스텀 훅은 비단 공통된 로직만 분리한다는 개념뿐만 아니라, 컴포넌트의 재사용화를 위해 비즈니스 로직을 분리한다는 개념으로도 사용해도 좋겠다. 현재 코드에서는 Redux 상태와 레이아웃에 들어가는 비즈니스 로직이 엮여있는 부분을 분리해낼 수 있어 보인다.

4. API 요청 횟수 줄이기

캐싱을 사용해서 장바구니나 주문내역과 같이 다른 페이지에서 다시 상품목록으로 돌아올 때, 짧은 시간이라면 API Fetching을 하지 않도록 해보자. 이는 API 요청 횟수를 줄일 수 있는 방법이다(짧은 시간동안 상품 목록이 갑자기 전면 개편되거나 타임세일이 있지 않는 한). 더불어, API 요소가 달라진 부분이 없다면 갱신을 안시키는 로직이 추가되면 좋겠다. 도 들어가있으면 좋겠네요. 사용자 UX 상으로는 백그라운드에서 동기화가 되니, 상관없이 잘 이용할 수 있게 제공도 해주어야 겠지만요. swr 같은것도 써보면 좋겠습니다.

5. UX개선 - 로딩/타임아웃 처리

로딩을 만드는게 좋겠습니다~ API 로드가 길어지면 아무런 화면도 띄워지지 않아, 사용자 입장에서 버그라고 생각할 수 있을 것 같아요~ 또한 타임아웃때도 어떤 에러페이지가 나올 지 생각해보면 좋겠습니다!

6. prop-types 로 명세 보여주기

일부 컴포넌트에 prop-types가 누락되어 있다. prop-types는 props타입을 컨트롤해주기도 하지만, 명세를 알려주는 역할도 하기 때문에 반드시 적어주는 것이 좋다.

7. 렌더링  예외처리

컴포넌트 전반적으로 prop이 없는 상황을 예견하고 예외 처리를 해주자. 예를 들어 title이 undefined라면 의미 없는 태그가 렌더링 될 것이다. 조건부 랜더링 처리를 해주거나, 꼭 필요한 prop이라면 PropTypes를 isRequired로 설정해주자.

/* 개선 전 */

<S.Title>{title}</S.Title>

/* 개선 후 */

{title && <S.Title>{title}</S.Title>}

8. 크기가 0인 요소의 위치

Frustom Culling 측면에서 (0,0)은 화면 안에 렌더링이 되는 오브젝트로 인식된다. 따라서 화면 상에 표현하지 않아도 되는 요소라면 (-9999, -9999) 로 이동시켜 화면 밖으로 빼주는 것이 좋다.

9. 스타일에 필요한 수치 상수화

CSS in JS를 사용하는 이유 중 하나는 constant 로 따로 빼서 관리할 수 있다는 점이다. 색상 팔레트, 내비게이션 바의 높이, 디바이스의 크기 등 모두 상수화해서 관리하는 것이 좋다.