はじめに
最近チームに React 18 を布教することの多い osuzu です。
普段の業務で、ペアプロ時に設計意図を伝えたり、コードレビューで都度自分の意図を伝えたりしてきました。
今回、これまでのチーム開発の経験やドキュメントに目を通す中で、自分が良いと考えている設計やコードレビューの観点を言語化することが出来てきたので、筆を執ってみました。
この記事はコードレビューの観点をチーム内へ知見共有するために書きましたが、社内に閉じる必要もない内容のため、Zenn でオープンに公開することにしています。
設計部分はプロジェクト（チーム）に依存していることが多く参考にしにくい部分もあるかもしれませんが、この記事がコードレビューや設計ガイドラインのような形で少しでも参考になれば幸いです。
記事の対象外
コードレビューそのものの基準や観点は取り扱いません。下記記事など適宜参考に。
Google How to do a code review
プロジェクト構成の観点
私達のプロジェクトは下記のようなディレクトリ構成になっています。
my-project/
├ src/
│ ├ components/
│ │ ├ features/
│ │ │ └ [複数のPageから参照されうるComponent]
│ │ ├ layout/
│ │ │ └ [すべてのPageから参照されうるComponent]
│ │ ├ page/
│ │ │ └ [routerのRouteに渡されるComponent]
│ │ └ ui/
│ │ └ [UIを構成する Component。Global State を扱う hooks に対する依存 NG]
│ ├ entries/
│ │ └ [React SPAのrootとなるhtmlやtsx。Context.ProviderやLayout Component(Global Error Boundary, Suspense Fallback含む)の構造を定義]
│ ├ lib/
│ │ └ [React に一切依存しない汎用関数]
│ ├ public/
│ │ └ [静的ファイル]
│ ├ recoil/
│ │ └ [Recoil の API を wrap した Actions が定義された hooks。Recoil は Selectors を通して State の update も可能だが、API が Recoil 独自のためこのディレクトリで吸収する]
│ ├ router/
│ │ └ [React-RouterのAPIをwrapしたActionsが定義されたhooks。routesの定義ファイルもここ。paramsやsearch paramsをvalidationして型安全にするといった役割を持つ]
│ └ swr/
│ └ [SWRのAPIをwrapしたActionsが定義されたhooks。immutableなresourceの明示やmutateの依存関係などの責任も持つ]
├ package.json
├ tsconfig.json
└ vite.config.js
- コロケーションを重視したプロジェクト構成にしています。
- そのため test.tsx を置く場合は component や hooks と同じ場所に。
依存関係の観点
ディレクトリやライブラリの提供する hooks, component に対し下記画像のルールを持たせています。（画像の文字が小さいので拡大推奨）
- ディレクトリごとに依存関係のルールを持たせています。
- 逆方向への依存禁止。一層上への依存のみ OK。
- 一つの Component に結びつく hooks は、
components/featuresディレクトリの中にuse~.tsとして配置しています。- hooks を hooks ディレクトリに切り出さない。関心ごとのコロケーションを重視する。
スタイルの観点
スタイルに関しては静的解析で担保しており、コードレビューでの指摘は行っていません。
そのため eslint では react-hooks や import 含めて recommended を設定するなど、書き方がブレにくい strict なルールを設定します。
React Component の観点
Component の粒度
1 つの Component がもつ状態を少なくする
例えば下記のような Component が存在した場合、
import { useState } from "react";
const Page = () => {
const [count, setCount] = useState(0);
return (
<div>
<button onClick={() => setCount((prev) => prev + 1)}></button>
<p>{count}</p>
<OtherComponent />
</div>
);
};
こうなっていた方が良いです。
import { useState } from "react";
const NewComponent = () => {
const [count, setCount] = useState(0);
return (
<>
<button onClick={() => setCount((prev) => prev + 1)}></button>
<p>{count}</p>
</>
);
};
const Page = () => {
return (
<div>
<NewComponent />
<OtherComponent />
</div>
);
};
この修正で count の state に変更があった場合に、 <OtherComponent /> を memo 化せずとも再レンダリングを避けることができます。
Component が十分に小さくなっているかについては下記のような観点をチェックしています。
- 一つの Component に結びつく hooks の数を少なくできているか
- 特に data fetch の hooks を呼ぶ場合、Component 1 つに resource が 1 つまでとした方が分かりやすい
- 状態を持った Component に子が存在する場合
- その状態は props として渡されるべきものか
- Component をまたぐ State をすべて Recoil 等で管理するのは大変で可読性も悪くなる。階層が浅くて共通機能でないものは props リレーしても良いが、Page や Feature にとっての Global State である認識をもつ
- そうでない場合、まずは状態をもった部分を対象に分割することを考える
- それが難しいなら子を memo 化できないか検討する
- その状態は props として渡されるべきものか
分割した Component をどこに所属させるか
書き始めの段階では基本的に Page でどんどん書いていき、共通機能（複数の Page から呼ばれる Component）が見えた段階で Features に切り出します。
その際 Props だけで構成され、Global State の hooks に依存がなく props のみで構成される Component は、 UI として切り出します。
useEffect の利用ガイド
そもそも useEffect とは
useEffect はクラス記法時代にあった React Component のライフサイクルではありません。
Render 一つ一つに対する副作用を定義し、props と state に応じて React tree 以外のものをシンクロするのが useEffect です。
useEffect をなるべく避ける
筆者の方針として、「useEffect を利用するのはなるべく避けたい」と考えています。
これは、「props と state に応じて React tree 以外のものをシンクロするのが useEffect」と先ほど述べましたが、
React tree に対して useEffect を使うことで、宣言的な React に副作用をもたらしバグの温床になりえます。
そのため使っていいケース以外での利用は、極力避けられないかをレビュー時にも強く考えています。
useEffect を使って良いケース
Component を React 外部のシステムと同期させる
setInterval のような Web API を元に React State を制御したり、外部サーバーにログを送信するなどといったケースが当てはまります。
※ ただし data fetch は useSWR や useFetch といった hooks を通した方が開発者の考慮は減って良いです。useEffect は低レイヤーな hooks で難しく、レースコンディションといった考慮すべき問題が知られています。
異なる State Management 同士を組み合わせる
例えば SWR で fetch した data を元にして Recoil の State を初期化したり、 React Router の location の変化を元にして Recoil の State を変化させるといったケースです。
useEffect の代替手段
イベントハンドラに置き換える
ユーザーイベントを処理する目的で Effect は NG。ハンドラに渡す Callback で処理してください。不要な Render を避けたい場合に関数は useCallback でメモ化可能です。
useMemo で置き換える
レンダリング用にデータを変換するために Effect を使わずに useMemo で計算しましょう。
useEffect の依存を減らすオプション
useEffect は関数の第２引数に依存関係を渡すことになりますが、 eslint で react-hooks/exhaustive-deps のルールを有効にするように推奨されています。
これは依存する props や state をすべて書くべきというルールですが、これに対して嘘を付きたくなる状況では別の選択肢が優れていたり、依存を減らす手段があります。
関数を useEffect の中で定義する、もしくはコンポーネント外に Hoisting する
依存関係の中に関数が存在する場合、その関数自体を useEffect 内で定義できるか検討します。
また関数自体が React Component に依存しない形で定義できる場合は、コンポーネント外に定義する(Hoisting する)ことで依存が減らせるか検討します。
useReducer を使う
props を useEffect の依存から削りたい場合に、 useReducer を用いることで必要最低限の Render に対する Effect の実行となります。
アップデートするロジックとそれらを宣言的に記述する表現を分離します。
※ 以下でも記載しますが、現在 beta の useEffectEvent が実装されるとそちらで実装するのが一般的になるのではと考えています。
useEffectEvent を使う
useEffectEvent (旧 useEvent) は元々 useCallback が 最適化にならないケースを解消するために用意されていますが、 useEffect の依存を減らす目的でも大きな効果があります。
React 18 時点では、 react-hooks/exhaustive-deps を守るために不必要な Effect 関数の実行が起きてしまったり、 eslint の rule を disabled にしたりといった状況が出てくると思いますが、そうした箇所を comment しておき useEffectEvent が実装された後にリファクタできるようにしておくと良いと思います。
Custom hooks の切り方
Component 間でロジックを共有したい場合に、 まず hooks にする必要がないものは純粋な関数として定義して利用しましょう。
関数を Hooks にする必要(React から提供される hooks を利用する必要)があって、かつ共通ロジックにしたいのは Global State を扱うケースだと思います。
私のアプリケーションでは下記の 3 つのディレクトリで Custom Hooks を管理しています。 hooks としてディレクトリを切らないのはコロケーションのためです。
- router (router にまつわる hooks)
- recoil (Global State Management にまつわる hooks)
- swr (data fetch & cache にまつわる hooks)
非同期処理とエラーハンドリング
Suspense と Error Boundary の Component で宣言的に管理します。
Suspense に関しては、なるべく data fetch に依存する Component 単位で細かく Suspense していきます。
※ ただし layout shift が発生する場合は考慮なしで fallback しない方が良いケースもあります。
Error Boundary に関しては、resource 単位で Global として Error を扱うのか、特定範囲の Component に閉じて fallback して良いのか検討できると良いです。
※ Error に関してはアプリケーションごとに対応が違うので、 必ず Error Boundary で fallback すべきとは思っておらず、 Toast を出すだけに留めたいみたいなケースもあると思います。
まとめ
以上が、執筆時点で私がチーム開発時に考えている設計やコードレビュー時の観点になります。
私自身、実装やレビューをしていく中で他に注意すべき観点が出てくるはずなので、この記事は今後もメンテ予定です。
意見や解釈が異なる箇所など、記事のフィードバック頂けたらとてもありがたいです！
参考
Discussion