코드 리뷰2

App.js

 

const rootStore = new RootStore(); .... +    <Provider modelView={rootStore} KanbanStore={rootStore.kanban}> +      <HashRouter> +        {/* GNB에 추가된 LIST 임시로 화면을 보여드리기위해 추가됬습니다.        적용 시 삭제 부탁드립니다. */}

• App.js 에서 글로벌 상태 모델을 생성하고 을 제공했으면 합니다.
  
  • 이름을 MVVM 의 modelView 로 통일
• 다음에 KanbanStore 를 추가적으로 매핑을 할 수는 있는데...

 

components/kanban/ChannelCategories.js

 

-    const { match } = this.props; +    const { channelType, channelId } = this.props;      this.state = { -      selectedChannelType: match.params.channelType, -      selectedChannelId: match.params.channelId -        ? match.params.channelId -        : match.params.channelType, +      selectedChannelType: channelType, +      selectedChannelId: channelId || channelType,      };

 

• props 는 반드시 decomposition 을 사용했으면 합니다.
• 라우터에 매핑되는 컴포넌트는 하나만 두었으면 합니다.
  • 여기서는 하나의 URL 에 여러 컴포넌트들이 매핑이 되고 있음

 

 

+  static getDerivedStateFromProps(nextProps) { +    return { +      selectedChannelType: nextProps.channelType, +      selectedChannelId: nextProps.channelId || nextProps.channelType, +    }; +  }

• componentWillRecieiveProps 대신에 getDerivedStateFromProps 를 사용합니다.
  • Deprecated 되었음
  • 원래 아귀먼트는 nextProps, prevProps  두 개를 받는 함수임.
  • 새로 마운트하지 않고 Props 가 변경되었을 때 호출됨.

 

const renderchannelCategoryInfo = channelCategoryInfos.map( -      (channelCategoryInfo, index) => ( +      (channelCategoryInfo) => (          <ChannelCategory -          key={`${selectedChannelId}_${index}`} +          key={`${channelCategoryInfo.channelType}`}

• Html 목록 아이템을 생성할 때는 배열 인덱스를 이용하지 않습니다.

 

 

+ChannelCategories.defaultProps = { +  channelId: "",  };

• Props 에 대한 검증은 가능하면 추가하고(PropTypes), 필요하면 디폴트 값도 설정했으면 합니다.
  
  • 설정 안 하면 린트 에러

 

 

components/kanban/ChannelCategory.js

 

 

+      return ( +        <li key={channel.channelId}>            <Link              key={channel.channelId}              to={() => { -              if (this.props.channelType === channel.channelType) { -                return `/kanban/${channel.channelType}`; -              } else { -                return `/kanban/${this.props.channelType}/${channel.channelId}`; -              } +              return channel.channelId === channelType +                ? `/kanban/${channelType}` +                : `/kanban/${channelType}/${channel.channelId}`;              }} -            onClick={() => this.handleSelectedChannelClick(channel)}            >              {channel.channelName}            </Link>

• Link 에서 onClick 은 뺐으면 합니다.

 

 

+      <li +        className={ClassNames({ +          top: true, +          [ClassByChannelType[channelType]]: true, +          on: selectedChannelType === channelType, +        })} +      >

• className 은 객체로 class 를 설정할 수 있는 것이 핵심

 

 

components/kanban/KanbanBoard.js

 

 

+@inject("modelView") +@observer  class KanbanBoard extends Component {    constructor(props) {      super(props);      this.state = { -      channelCnt: "", -      channelsBrief: [],

• 어느 시점에 모델을 바인딩할 지 선택해야함.
  
  • 여기서는 URL 과 바인딩 된 페이지 아래에서 처리를 하고 있음
• 쓰지 않는 코드는 제발 삭제 부탁

 

 

async componentDidMount() { ... +    const { modelView, channelType } = this.props; +    await modelView.kanban.fetchChannelsBrief({ +      channelType, +    });    }

• 서버로 부터 모델을 업데이트하는 코드는 componentDitMount 와 getDerivedStateFromProps 에서

 

 

render() { +    const { modelView, channelType } = this.props; +    const { data } = this.state;      const MemberPopup = PopupWrapper(Members); ... +      return (        <main className="main kanban-main">          <div className="kanban"> -          {this.state.channelsBrief?.map((value, index) => { +          {modelView.kanban.channelsBrief?.map((value) => {

• 글로벌 스토어에 대한 접근은 modelView 를 이용하여서 접근

 

repositories/KanbanRepository.js

 

class KanbanRepository { -  getTasksBrief(props) { +  static getTasksBrief(props) {      const params = {        userId: props.userId,

• 메소드를 일반 메소드로 할 것인지 static 메소드할 것인지 잘 구분

 

routes/Kanban.js

 

 

+function Kanban() { +  return ( +    <div> +      <Switch> +        <Route +          exact +          path="/kanban/:channelType" +          component={KanbanChannelListPage} +        /> +        <Route +          path="/kanban/:channelType/:channelId" +          component={KanbanChannelDetailPage} +        /> +      </Switch> +    </div> +  );

• routes 를 단순화 시킴
  
  • URL 에 하나의 페이지가 매핑되는 형식으로 구조 수정

 

stores/KanbanStore.js

 

 

this.channelCategoryInfos = data.dto.channelList.map(        (channelCategoryInfo) => { +        const mappedCategoryInfo = { ...channelCategoryInfo }; ... -        channelCategoryInfo.data = channelCategoryInfo.data +        mappedCategoryInfo.data = mappedCategoryInfo.data            ? [...defaultCategoryInfoData, ...channelCategoryInfo.data]            : [...defaultCategoryInfoData];   -        return channelCategoryInfo; +        return mappedCategoryInfo;        }

• map 을 하는데, 자기 스스로를 업데이트하는 코드가 있다?

 

+  @asyncAction +  async *fetchChannelsBrief(props) { +    this.channelsBrief = []; + +    const { data } = yield KanbanRepository.getChannelsBrief(props); +    this.channelCnt = data.dto.channelCnt; +    this.channelsBrief = data.dto.channelsBrief +      ? [...data.dto.channelsBrief] +      : [];    }

• 배열인 경우 왜 상태를 항상 [] 으로 초기화를 하고, 다시 설정하는 지?
• 왜 비동기 액션에서 왜 generator 를 쓰고  있는 지?

 

 

-KanbanStore = new KanbanStore(); -  export default KanbanStore;

• 스토어나 컴포넌트를 특별한 목적이 아니면 내부에서 new 를 하지 않았으면 함.

 

stores/RootStore.js

 

 

import autobind from "autobind-decorator"; import AuthStore from "./AuthStore"; import KanbanStore from "./KanbanStore"; @autobind class RootStore {    constructor() {       this.auth = new AuthStore(this);       this.kanban = new KanbanStore(this);    } } export default RootStore;

• 멀티플 스토어 엮는 방법 제안

 

stores/AuthStore.js

 

 

import { observable } from "mobx"; import autobind from "autobind-decorator"; @autobind class AuthStore { @observable userId = "8c0a49be-d9de-4a70-99dd-3bc3de71d49a"; constructor(parentStore) { this.parentStore = parentStore; } } export default AuthStore;

• 현재 사용자의  인증 정보 담음.