W3 - 리팩토링 | 상품 조회 이력

yisu.kim·2021년 8월 18일

React WeCode mrcamel refactoring rootimpact wanted

Pre Onboarding

목록 보기

10/16

📍 원티드 프리온보딩 과정에 참여하여 개발한 미스터카멜 기업과제를 리팩토링해보았습니다.

리팩토링

리팩토링 저서로 유명한 Martin Fowler는 리팩토링을 이렇게 정의한다.

Refactoring is a disciplined technique for restructuring an existing body of code, altering its internal structure without changing its external behavior.
Refactoring isn't a special task that would show up in a project plan. Done well, it's a regular part of programming activity.

by Refactoring - Martin Fowler

리팩토링은 특별한 태스크가 아니라 일상적인 프로그래밍 업무의 일부임을 마음에 새기며 다만, 이번에는 수업시간에 배운 내용에 한해 사후 리팩토링을 진행해보았다.

🔗 GitHub

리팩토링 대상을 선정한 동기

수업시간에 소개된 React 디버깅 방법에 대한 영상을 보고 유용한 기능을 발견했다. 바로 Highlight updates when components render. 옵션!

옵션을 키고 어떤 과제를 리팩토링 해볼까하며 하나씩 살펴보는데...

아... 이거 원인이 뭐지? 왜 페이지 전체가 계속 리렌더링되는 걸까? 멘토님께서 수업시간에 렌더링을 줄이는 것이 핵심이라고 계속 강조하셨기 때문에 당황스러웠다.

그래서 이 문제는 성능향상과 좀 더 관련이 있지만, 대상 과제로 선정해 리팩토링할겸 문제를 해결해봐야겠다고 생각했다.

리렌더링을 줄이기 위한 여정

👩‍🏫 김예리 멘토님 수업

React 성능향상을 위한 몇 가지 방법
DOM 조작이 CPU리소스를 가장 많이 사용하거나 브라우저에 부하를 주기 때문에, rerender 안 되게 하는 팁이 대부분이다!

끊임없는 리렌더링의 원인 - setInterval

원인을 찾아 헤매던 중 조회 목록 페이지에서 다른 페이지로 이동하면 다음과 같은 경고가 뜨는 걸 발견했다.

컴포넌트가 언마운트되어도 비동기 작업이 취소되지 않아서 메모리 누수(!)가 발생한다고 친절하게 알려주고 있다. 이렇게 경고 메시지가 뜨면 참 좋다. 검색해볼 수 있으니까.

클라이언트 데이터를 사용하기 때문에 비동기 함수를 쓰는 게 없을텐데? 어쨌든 클래스 컴포넌트니까 componentDidMount와 componentDidUpdate를 찾아보기로 했다.

빙고! setInterval 함수가 사용되었다.

// pages/RecentList/RecentList.js
componentDidMount() {
  this.getRecentList();
  setInterval(this.update, 1000); // 대표적인 비동기 함수
}

다른 코드도 살펴보니 과제 요구사항 중 00시 기준으로 최근 조회이력과 관심 없는 상품목록 초기화하는 기능을 구현하기 위한 것이었다.

일단 에러부터 해결하자. 경고 메시지의 추천대로 componentWillUnmount에서 비동기 작업을 구독 취소해주면 된다.

// pages/RecentList/RecentList.js
componentDidMount() {
  this.timerID = setInterval(this.update, 1000);
}

componentWillUnmount() {
  clearInterval(this.timerID);
}

📢 이하 코드와 영상은 리팩토링 전 코드를 수정한 상태입니다. 이미지도 기업 특성에 맞춰 상품 사진으로 교체했습니다.

첫 번째 시도 - PureComponent

setInterver 함수로 인해 페이지 전체가 리렌더링된다는 사실을 알게 되었다.

먼저 리렌더링을 막기 위해 수업시간에 배운 React.PureComponent를 사용해보기로 했다.

부모가 rerender 하면, 자식 컴포넌트도 모두 rerender되기 때문에 Props 값이 변하지 않으면 다시 렌더하지 않는 PureComponent로 자식 컴포넌트를 변경하면 리렌더링을 막을 수 있지 않을까하는 생각에서였다.

Layout이라는 자식 컴포넌트를 PureComponent로 바꾸고 상태를 다시 확인해보았다.

// pages/RecentList/RecentList.js
class RecentList extends Component {
  ...
  render() {
    return (
      // 자식 컴포넌트
      <Layout menu={<Menu history={this.props.history} />}>
        ...
      </Layout>
    );
  }
}

// components/Layout.js
class Layout extends PureComponent { // 메모이제이션하는 PureComponent로 변경
  ...
}

안타깝게도 차이가 없었다.

자식 컴포넌트의 자식 컴포넌트를 PureComponent로 바꿔봐도 전체적으로 리렌더링이 계속되는 건 마찬가지였다.

두 번째 시도 - Clock 컴포넌트?!

구글링하면 다 나온다하는 심정으로 검색을 거듭하다가 무려 React 공식 문서 State와 생명주기에서 제공하는 setInterval를 사용하는 시계 예제를 찾게 되었다. 믿을 건 역시 공식문서밖에 없다.

예제를 보고 또 보다가 문득 엉뚱한 생각이 들었다. Clock 컴포넌트가 아무 것도 반환하지 않으면 어떨까? 다시 말해서 렌더링할 게 없다면?

그래서 다음과 같은 코드가 나오게 되었다.

// components/Clock.js
class Clock extends Component {
  state = {
    date: new Date(),
  };
  ...
  componentDidMount() {
    this.timerID = setInterval(() => this.tick(), 1000); // setInterval는 호출했지만
  }
  ...
  componentWillUnmount() {
    clearInterval(this.timerID);
  }

  tick() {
    this.setState({
      date: new Date(),
    });
  }

  render() {
    return null; // 이 컴포넌트는 아무것도 렌더링하지 않는다.
  }
}

export default Clock;

// pages/RecentList/RecentList.js
class RecentList extends Component {
  ...
  render() {
    return (
      <>
        // Clock 컴포넌트 추가
        <Clock handleStorageUpdate={this.getRecentList} />
        <Layout menu={<Menu history={this.props.history} />}>
          ...
        </Layout>
     </>
    );
  }
}

더이상 리렌더링되지 않고 날짜를 강제로 바꿔보면 기능도 정상적으로 동작하는 것을 확인할 수 있다.

그렇지만 이게 맞는 걸까?

사실 아무 값도 반환하지 않는 컴포넌트라는 개념이 무척 낯설어서 어쩌면 문제가 눈에 보이지만 않을 뿐 계속 존재하는 게 아닌가하는 걱정이 든다. 정상적인 방법이 아니라 꼼수를 쓴 느낌?

계속 개발을 해나가다보면 답을 찾을 수 있다고 생각하고 여기서 끝마친다.

리팩토링으로 넘어가서

수업에서 배운 내용들을 되짚어 보며 몇 가지 리팩토링을 진행했다. 어떻게 하면 가독성과 생산성을 고려할 수 있을까?

Constants 관리

App 컴포넌트만 봐도 라우트 정보가 상수로 정의되어 있지 않았다.

// src/App.js
class App extends Component {
  render() {
    return (
      <div>
        <Route exact path="/">
          <Redirect to="/product" />
        </Route>
        <Route exact path="/product" component={ProductListPage} />
        <Route path="/product/:productId" component={ProductDetailPage} />
        <Route path="/recent-list" component={RecentListPage} />
      </div>
    );
  }
}

상수를 정의하고 App 컴포넌트에서 가져다 쓰도록 했다.

// src/App.js
class App extends Component {
  render() {
    return (
      <Switch>
        <Route exact path={ROUTES.HOME}>
          <Redirect to={ROUTES.PRODUCT} />
        </Route>
        <Route exact path={ROUTES.PRODUCT} component={ProductList} />
        <Route
          path={`${ROUTES.PRODUCT}/:productId`}
          component={ProductDetail}
        />
        <Route path={ROUTES.RECENT_LIST} component={RecentList} />
        <Route component={NotFound} />
      </Switch>
    );
  }
}

그리고 코드를 전체적으로 살펴보며 상수를 모아 constants.js에 추가했다.

// utils/constants/constants.js
export const LOCAL_STORAGE = storage(localStorage);
export const STORAGE_KEYS = {
  INTEREST_LIST: "interestList",
  RECENT_LIST: "recentList",
  LAST_VISITED_DATE: "lastVisitedDate",
};

export const ROUTES = {
  HOME: "/",
  PRODUCT: "/product",
  RECENT_LIST: "/recent-list",
};

export const PRODUCT_DATA = productData.create();
export const UNIQUE_BRAND = productData.getUniqueBrand();

export const ORDER_BY = {
  VIEW: "view",
  PRICE: "price",
};

Utils 함수 관리

상품 데이터와 스토리지 관련 작업을 공통으로 사용하기 때문에 여기저기 흩어져 있는 작업들을 모아 관련 유틸 함수를 작성했다.

utils/productData.js
utils/storage/init.js
utils/storage/interestList.js
utils/storage/lastVisitedDate.js
utils/storage/recentList.js

Inline CSS 모두 제거

antd UI 라이브러리를 사용했는데 제공하는 컴포넌트의 스타일을 커스터마이징하기 위해 inline CSS가 작성되었다.

// pages/ProductDetail/ProductDetailPage.js
...
<Col sm={24} md={14} style={colStyle}>
  <div style={{ width: "100%" }}>
    <Row>
      <Col span={24}>
        <Card
          hoverable={true}
          bodyStyle={{ padding: "0" }}
          cover={
            <MainImgWrapper>
              <img
                alt="productImage"
                src={original_data[productId].imgUrl}
                style={mainImgStyle}
              />
            </MainImgWrapper>
          }
        ></Card>
      </Col>
    </Row>
  </div>
</Col>
...

리액트에서는 style 속성을 사용하는 것을 권장하지 않고 있다.

Some examples in the documentation use style for convenience, but using the style attribute as the primary means of styling elements is generally not recommended. In most cases, className should be used to reference classes defined in an external CSS stylesheet. style is most often used in React applications to add dynamically-computed styles at render time. See also FAQ: Styling and CSS.

by ReactJS

그래서 styled-components로 커스터마이징한 antd 컴포넌트를 만들었다.

// pages/ProductDetail/ProductDetailStyle.js
...
export const CustomCol = styled(Col)`
  text-align: ${({ textalign = "left" }) => textalign};
`;
...

컴포넌트 재사용 & 분리

페이지에서 공통으로 사용되는 레이아웃과 헤더를 각각 Layout, Header 컴포넌트로 만들어 재사용했다.

// components/Layout.js
class Layout extends Component {
  static propTypes = {
    menu: PropTypes.oneOfType([
      PropTypes.arrayOf(PropTypes.element),
      PropTypes.element,
    ]),
    children: PropTypes.oneOfType([
      PropTypes.arrayOf(PropTypes.element),
      PropTypes.element,
      PropTypes.node,
    ]),
  };

  render() {
    return (
      <>
        <Row gutter={[0, 16]} justify="center">
          <Col xs={22} sm={20} md={18} xl={16}>
            <Header menu={this.props.menu}></Header>
          </Col>

          <Col xs={22} sm={20} md={18} xl={16}>
            <div>{this.props.children}</div>
          </Col>
        </Row>
      </>
    );
  }
}

// components/Header.js
class Header extends Component {
  static propTypes = {
    menu: PropTypes.oneOfType([
      PropTypes.arrayOf(PropTypes.element),
      PropTypes.element,
    ]),
  };

  render() {
    return (
      <Row type="flex" justify="space-between" align="middle">
        <Col>
          <Link to={ROUTES.PRODUCT}>
            <img alt="logo" src="/logo.png" />
          </Link>
        </Col>
        <Col>{this.props.menu}</Col>
      </Row>
    );
  }
}