レビュー観点
以下は、よくあるレビュー指摘について観点をまとめたものです。
レビュー依頼の前に以下のような指摘に自身で対処してください。
参考で載せている記事・本もあわせて読んでください
詳細設計
- コンポーネント分割：初期表示時のデータの取得処理（API呼び出し）と、データを描画する処理のコンポーネントを分けているか
- 共通コンポーネントと同じ処理を個別実装していないか（共通コンポーネントの中身を確認してから実装しているか）
- エラー処理の要/不要を検討したか（不要と判断した場合、理由が説明できるか）
- 型では読み取れないような特別な処理を避けているか(例：number型変数で 0に特別な意味を持ったせたり、array型で0番目に特別な意味を持たせたりしていないか)
- 暫定対応がある場合に「仮実装」が最小限か。未確定の部分のみ適切な単位で切り出してメソッド化またはコンポーネント化した実装か
実装
自分で書いたコードに責任を持つ
- 自分で書いたコードでなくても担当した機能については責任を持つ
- コピペしたコードは、処理内容と、何故そのコードを採用したのかが説明できる
- 自分で書いたコードは「動くからOK」ではなく、使われているプログラミング言語（React、React Native、JavaScript、TypeScript）の文法を説明できる
書き方を統一する
- ファイル間、メソッド間、変数間、関連する別アプリで表記揺れや、書き方の違いをなくす
他の人がコードを見た時に、書き方が統一されていないと、書き方の違いについて書き手の意図があるのかと勘ぐり、処理自体を理解する妨げになります。 もし意図的な違いだとしても、変数名・メソッド名に違いがなかったり、コメントが無いは、読み手に伝わりません。
また、割れ窓理論の観点から、ささいな書き方の違いなども徹底して排除してください。 例えば実PJで100画面の場合、ファイル数も相当になり、プログラマー・レビューアーの人数も増えます。 そうなった時に、僅かな書き方の乱れが原因で、その後の機能改修などを経て乱れが拡大していきます。
変数名・関数名・コンポーネント名などの無意味な名前をつけない
実PJでは自分以外の他人と協力して開発します。そのため他人が読んで意味（理由）がわかる変数名を定義してください。
また実PJでは長期間にわたり開発を継続します。 たとえ自分が書いたコードでも6ヶ月後の自分はその時の考えを忘れています。 6ヶ月後の自分のためにも、意味（理由）がわかるような変数名を定義してください。
読み手に理由を明示する
- 他人が見た時に「なぜ？」と思う箇所にはコメントを付ける
- コードのみで読み手に意図を伝えるのが難しい場合はコメントを添える
- なんらかの理由で作業途中などの場合や、他の人が読んだら疑問に思いそうな部分には、
// TODOや// FIXMEなどのコメントを添える
プログラミングのお作用
- 同じ処理は共通化する
- マジックナンバーを使わない
- 値をハードコートしない
レビュー依頼前のチェック
- ローカル環境でlintエラーが無いことを確認してレビュー依頼してください
- GitLabのマージリクエスト、GitHubのプルリクエストなどのリモート環境のCIでlintエラー・テストエラー・ビルドエラーが無いことを確認してレビュー依頼してください
- developブランチの最新をfeatureブランチに取り込んだ上でレビュー依頼してください
- ブラウザで動作確認中、開発者ツールのコンソールでエラーが無いこと確認する、エラー残すなら残す理由を説明可能にしてからレビュー依頼してください
その他
実際のレビューでよくある指摘を具体的に列挙したものです。
基本
- 仮実装についてのコメントはTODOコメントを指定してください
- TODOコメントを残した状態でレビュー依頼しないでください、どうしても残す場合はTODOがストーリーなどで管理された状態にしてからRV依頼してください
- コメントアウトは理由がなければ削除、あるなら理由をコメントしてください
- コメントで処理内容を説明するより、コードを見て処理内容がかわるコードにしてください
- 共通部品で賄える部分は自作しないでください
- 一度しか参照しない変数は、変数定義せず、参照先で直接値を定義してください
- 一箇所でしか使っていないコンポーネントや処理は別ファイルに切り出したり共通化しないでください
- 複数回出てくる同じ処理は共通化してください
JavaScript・TypeScript
- 関数定義直後で引数を分割代入しており、分割代入元を一度も参照しない場合は、関数の仮引数の時点で分割代入してください
- if分の{}を省略するか否かは統一してください
- 数値は0の時falseと判断されることを理解した上で条件分岐を実装してください
- 型推測が有効な部分に明示的に型を定義しないでください
anyを使わないでくださいasは極力使わないでください。ジェネリクスや型推論で賄えるようにしてくださいawaitの意味をわかってから使ってください。await不要な箇所でawaitを定義しないでください- 変数の形式は、コンポーネントならパスカル、変数ならキャメル、定数ならスネーク全部大文字にしてください
- 関数は省略できるなら省略してください
- オブジェクトの定義で、keyとvalueが同じ名前なら省略形で書いてください
- if-elseで値を代入、戻り地返却するなら三項演算子を使ってください
- 配列のソート関数は、どんなソートかわかるように関数化してください
exportの意味理解して使ってください。外部から参照されないものはexportしないでください
React
- ステートは不用意に定義しないでください
- ステートの値を直接変更しないでください。セッター経由で変更してください
- 処理中でHooks未使用の関数は、カスタムフックではなく単なるutil関数にしてください
useXXXはカスタムフックの命名規則なので、それ以外の目的の場合、変数にuseXXXとしないでくださいuseEffectは極力使わないでください、最終手段として使ってください ※入力項目へのフォーカス目的では使って良い- コンポーネントに依存せずReactのフックなどを使わないような処理（定数定義）はコンポーネント外で定義するようにしてください
- カスタムフックだけのファイルの拡張子をtsxにしないでください
- 10行以上にわたるような大きなTSX部品のメモ化はやめてください。コンポーネント化してください
- 複数のステートを定義してお互いが依存しているならばReducerを導入してください
- 画面遷移はReact Routerの
history.pushを使ってください。window.location.hrefを直接書き換えないでください
MUI(UI)
divタグspanタグは使わずにMUIのBoxタグを使ってください- 登録・更新・削除などの処理が成功した場合は、スナックバーを表示してください、とりわけ操作者に伝えることがあればスナックバーではなくダイアログを表示してください
- スタイルでサイズ・間隔を指定する場合は
theme.spacingを使ってください。単位なしやpx、remは極力使わないでください - style属性は極力指定しないでください。間隔調整であればBoxタグかGridタグを使ってください
- style属性は極力指定しないでください。色の指定であればcolor属性を使ってください
- スタイルで色を指定する際は、値をハードコードせずtheme.palletを使ってください