Zenn
Open3

React知見

React
koukou

ListコンポーネントにList以外のことを入れない

よくよく考えてみたら、これは「単一責務にしよう」ってことに近いのかも。リストはリスト。見出しは見出し。ただ、タイトルまで含めて一つのリストとみなしたい場面なら、それもまた真なのかもしれない(?)。

※親Pageコンポーネントでtodosデータを取得し、propsとして渡す場合の話

こういうことしない(↓)

「Todo一覧」というタイトルを含んでしまっている
// 親からtodosが渡ってくる想定
export const TodoList: FC<Props> = ({ todos }) => {
  return (
    <div className={'mx-auto p-4 sm:w-[720px] space-y-3'}>
      {/* これをここに混ぜない方がいい(↓) */}
      <h1 className={'text-2xl font-bold py-4'}>Todo一覧</h1>
      {todos.length > 0 ? (
        todos.map((todo) => {
          return <TodoItem key={todo.id} todo={todo} />;
        })
      ) : (
        <p className={'text-gray-notice text-center'}>Todoはありません</p>
      )}
    </div>
  );
};

理由

  • Todo一覧取得途中にローディングを表示させたい場合、このコンポーネントを丸ごとSpinnerに置き換えてしまうと、「Todo一覧」というタイトルごと消してしまうことになる。

改善案
タイトル部分を外に出して以下のようにする

こうする
<Page>
  <div className={'mx-auto p-4 sm:w-[720px]'}>
    <Heading title={'Todo一覧'} />
    {/* loading中のときでも「Todo一覧」という文言は表示されたままになった!(ただしちょっと手続き的) */}
    {isLoading ? (
      <Loading />
    ) : (
      <TodoList todos={todos} />
    )}
  </div>
</Page>

現状の最善案
TodoListに必要なデータをTodoList自身に取ってこさせるようにして、それをSuspenseでラップする。
Suspenseを用いて以下のようにする

Suspenseﾂﾖｲ
<Page>
  <div className={'mx-auto p-4 sm:w-[720px]'}>
    <Heading title={'Todo一覧'} />
    {/* Loading状態まで宣言的に扱えるようになった!ｷﾓﾁｲ */}
    <Suspense fallback={<Loading />}>
      {/* todos propを渡すのではなく、TodoList自身の中でフェッチ処理をかけるようにする */}
      <TodoList />
    </Suspense>
  </div>
</Page>

ここからわかること。

  • Suspenseを上手く使うためには、コンポーネント自身が自分に必要なデータを自分で取ってくるということをする必要がある（それを上位コンポーネント側でSuspenseでラップする）。
  • この、「自分に必要なデータを自分で取ってくる」というのを上手く実現するためにuseSWRなどのデータフェッチ用フックを内部で使うことになる。このとき取得してきたデータの加工処理まで含めたい場合、useSWRなどでのデータ取得＋加工処理までをまとめてカスタムフックとして切り出し、そのコンポーネントの近くに置いておくと、ロジックとUIが分離して且つとても見やすくて気持ちがよい🙂
TodoList
├─index.ts // バレル用
├─TodoList.tsx // UI専門(JSX)
├─useTodoList.hook.ts // ロジック専門(カスタムフック)

ちなみに、最初の例も、TodoList自身でデータ取得を行っていれば、JSX内でそのまま分岐が書けるのでまだマシだったかも。結局Suspenseが一番見やすいに変わりはないが。

いや、書いてみて思ったけど三項演算子ネストしてる時点でダメだわ。🙅‍♀️

TodoList自身でデータ取得まで行なってる版の最初の例
export const TodoList: FC = () => {
  const { data: todos, error } = useSWR('api/todos', fetcher);
  const isLoading = !data && !error;

  return (
    <div className={'mx-auto p-4 sm:w-[720px] space-y-3'}>
      <h1 className={'text-2xl font-bold py-4'}>Todo一覧</h1>
      {isLoading ? (
        <Loading />
      ) : (
        {todos.length > 0 ? (
          todos.map((todo) => {
            return <TodoItem key={todo.id} todo={todo} />;
          })
        ) : (
          <p className={'text-gray-notice text-center'}>Todoはありません</p>
        )}
      )}
    </div>
  );
};
koukou

コンポーネントはできるだけコンパクトにする

1コンポーネントがやることは1つのこと(単一責務)。シンプルで読みやすい「関数」を目指す。
テストが書きやすいのは、値を受け取ってその値を用いて表示を行なうだけの純粋関数的な関数コンポーネント。

行数目安としては、あくまで感覚だが多くて50行程度を保つようにして、最大でも100行ぐらいに収まるようにすると比較的見易い。

以下の記事を読んで、自分ならこう書きそうだなというのを書いていて思った。

自分ならこう書きそう版
export const Profile: FC<Props> = ({ userId }) => {
  const { data: user } = useQuery(['user', userId], () => getUser(userId));

  return (
    <>
      {user ? (
        <div className={user.gender === '男性' ? 'blue' : 'pink'}>
          <p>ユーザー名：{user.name}</p>
          <p>性別：{user.gender}</p>
          <p>年齢：{user.age}</p>
        </div>
      ) : (
        <p>ユーザーはいません</p>
      )}
    </>
  );
};
koukou

[考える] コンポーネントの共通化

UIが似ているからといって、安易にコンポーネントを共通化するのは危険な思考だと思っている。
A, Bという類似したUIが想定される時、AというコンポーネントがUIとして為す役割と、BというコンポーネントがUIとして為す役割が、異なっているのであれば、たとえUIが似通っていたとしてもコンポーネントは分けた方がいいように思う。

そうは思う一方で、AとBはデザインが似通っているために、Aのデザインに修正が入る時は、おおかた似通っているBの方にも同じようなデザインの修正が入ることが想定される。
2つ程度であればまだいいが、これが3つ4つあるようなとき、本当にこの基準でのコンポーネント分割は正しいのだろうかとも思えてくる。
実装者本人であれば、どこでコンポーネントにわけているかは大体覚えているのでまだよいが、これを他人が保守していく時、抜け漏れを発生させずに修正をこなすのは、共通化した場合に比べて相対的に難易度が上がる。

A, B内で使う部品コンポーネントの部分を共通で使っていくのが、過度な共通化を避けつつ似たUIを実現する方法なんだろうか。
A,Bの大枠デザイン変更が起きた場合のみ、先ほど挙げた点に注意する必要は残る。

引き続き考える。

雑メモ。
最初に分けてから後で共通コンポーネント化するのと、最初に共通コンポーネントにしていてから、後で分けるのどちらがいいのだろう。
最初に分けるのは役割・ドメインの違いなどで分けるってことだから、あとから共通化したいみたいなのって少ないんじゃ？
最初に共通だと思っていたものが、役割等の違いで別コンポーネントにしたほうがいいことは確かにあるかも。