Reviewing React Frontend

reviewing-react-frontend

Reactフロントエンドのコード変更をレビューします。アーキテクチャ、Reactベストプラクティス、型安全性、UX、パフォーマンスに焦点を当てます。

いつこのスキルを使うか

以下の場合に使用:

• React/TypeScriptコードを含むプルリクエストやブランチのレビュー
• mainブランチへのマージ前のフロントエンド実装分析
• コンポーネント構造と状態管理の評価
• Reactパターンと規約への準拠確認

用語定義

レビューで使用する重要な概念の定義:

ディレクトリ構造

• features/: 特定の機能（ドメイン）に関連するコード。他の機能から直接インポートしてはならない

  • 例: features/auth/, features/history/, features/search/
  • 判断基準: 削除しても他の機能が動作し続けるなら features/
  • 含むもの: pages, components, hooks, contexts, types（その機能専用）

• shared/: 複数の機能で共有されるUI/ロジック。プロジェクト固有だが汎用的なコード

  • 例: shared/components/AppHeader, shared/hooks/useAuth, shared/utils/formatDate
  • 判断基準: 3つ以上の機能から使われる、または将来的に再利用が見込まれる
  • 判断基準: プロジェクトのドメイン知識が含まれている（例: AppHeader, AuthContext）
  • 含むもの: components, hooks, contexts, utils, types（アプリケーション共通）

• lib/: 完全に汎用的なユーティリティ。他のプロジェクトでもそのまま使える

  • 例: lib/supabaseClient.ts, lib/logger.ts, lib/apiClient.ts
  • 判断基準: プロジェクト固有の知識を含まない
  • 判断基準: npm パッケージ化できるレベルの汎用性
  • 含むもの: 外部サービスのクライアント、汎用ユーティリティ関数

判断フロー:

1. 特定の機能でのみ使う → features/
2. 複数機能で使うがプロジェクト固有 → shared/
3. 完全に汎用的で他プロジェクトでも使える → lib/

状態管理のレベル

• ローカル状態: useState/useReducerで1つのコンポーネント内のみで使用

  • 判断基準: 他のコンポーネントが知る必要がない状態

• リフトアップ状態: 親コンポーネントで管理し、Propsで子に渡す

  • 判断基準: 兄弟コンポーネント間で共有が必要な状態

• グローバル状態: Context/Redux等で広範囲に共有

  • 判断基準: 多数のコンポーネントツリーを跨いで共有が必要な状態（認証、テーマなど）

単一責任の原則

コンポーネント/関数が1つの明確な責務のみを持つ

• 判断基準:
  • コンポーネント名が「〜And〜」を含まない
  • 1つのコンポーネントが50行以下（目安）
  • 説明に「〜と〜をする」という接続詞が不要

レビュープロセス

このチェックリストに従って体系的にレビュー:

1. アーキテクチャ & 設計

• コンポーネントが単一責任の原則に従っている（上記定義参照）
• features/とshared/が適切に分離されている（上記定義参照）
• 状態管理が適切なレベルで行われている（上記定義参照）
• カスタムフックが3つ以上の場所で再利用可能、または再利用が見込まれる
• 同一ドメインのファイルが同一features/配下にグループ化されている

2. Reactベストプラクティス

• 不要な再レンダリングがない

  • 判断基準: 親の再レンダリング時に影響を受けないコンポーネントはReact.memoでラップ
  • 判断基準: 高コスト計算（100ms以上）はuseMemoでメモ化
  • 判断基準: 子コンポーネントに渡すコールバックはuseCallbackでメモ化

• useEffectの依存配列が正確で完全

  • 判断基準: ESLint exhaustive-deps警告がない
  • 判断基準: useEffect内で使用するすべての外部変数が依存配列に含まれる

• Keyプロパティが安定していて一意

  • NG: 配列のインデックスをkeyに使用
  • OK: データのID、UUID、安定したユニーク値

• 条件付きレンダリングが適切なパターンを使用

  • 判断基準: null/undefined表示は&&演算子、分岐は三項演算子
  • 判断基準: 複雑な条件分岐は早期returnで表現

• イベントハンドラーがレンダリング毎に新しい関数を作成していない

  • NG: onClick={() => handleClick(id)}
  • OK: onClick={handleClick} または useCallback使用

• useEffectで適切なクリーンアップが実装されている

  • 判断基準: setTimeout/setIntervalにはclearTimeout/clearInterval
  • 判断基準: イベントリスナー登録にはremoveEventListener
  • 判断基準: Subscription（WebSocket等）にはunsubscribe

3. 型安全性

• any型の使用が正当化されている

  • 許容される例: 外部ライブラリの型定義が不完全、動的な型が本質的に必要
  • 判断基準: any使用箇所にコメントで理由を記載

• 型アサーション（as）が必要かつ安全

  • 判断基準: DOM要素の型（as HTMLInputElement）など、TypeScriptが推論できない場合のみ
  • NG: エラー回避のためのas any

• Propsインターフェースが完全で正確

  • 判断基準: すべてのPropsに型定義がある
  • 判断基準: オプショナル（?）と必須が正確に区別されている
  • 判断基準: デフォルト値がある Props は型定義でオプショナル

• APIレスポンス型がバックエンド契約と一致

  • 判断基準: types/supabase.types.ts等の共有型定義を使用
  • 判断基準: 手動で型定義する場合、バックエンドのスキーマと照合

• 共有型がtypes/ディレクトリに定義されている

  • 判断基準: 2つ以上の機能で使用される型はtypes/に配置
  • 判断基準: 機能固有の型はfeatures/*/types/に配置

4. ユーザー体験

• 非同期操作中にローディング状態が表示される

  • 判断基準: 500ms以上かかる操作にはローディングインジケータ
  • 判断基準: ボタンクリック後の処理中は disabled 状態

• エラーメッセージがユーザーフレンドリーで実行可能

  • NG: "Error: 500" "Unknown error"
  • OK: "保存に失敗しました。もう一度お試しください"
  • 判断基準: 技術的詳細はログに記録、ユーザーには対処法を提示

• アクセシビリティ属性が適切に設定されている

  • 判断基準: クリック可能な非button要素には role="button"
  • 判断基準: インタラクティブ要素には aria-label（視覚的ラベルがない場合）
  • 判断基準: フォーカス可能な要素には tabIndex（0 or -1）

• キーボードナビゲーションが機能する

  • 判断基準: onClick がある要素には onKeyDown でEnter/Spaceキー対応
  • 判断基準: モーダルはEscキーで閉じられる
  • 判断基準: Tab順序が論理的

• レスポンシブデザインが適切

  • 判断基準: モバイル（< 768px）、タブレット（768-1024px）、デスクトップ（> 1024px）で表示確認
  • 判断基準: 横スクロールが発生しない

• フォーカス管理が適切

  • 判断基準: モーダル表示時にフォーカスがモーダル内に移動
  • 判断基準: モーダル閉じる時に元の要素にフォーカスが戻る

5. パフォーマンス

• N+1 APIコール問題がない

  • NG: リスト表示でmap内でAPIコール
  • OK: 一括取得、またはuseEffect外でまとめて取得

• 大きなリスト（100件以上）で仮想化を使用

  • 判断基準: react-windowやreact-virtualized等を使用
  • 判断基準: または無限スクロール+ページネーション

• 画像最適化と遅延読み込み

  • 判断基準: <img loading="lazy"> 属性を使用
  • 判断基準: 画像サイズが適切（WebP形式推奨、width/height指定）
  • 判断基準: ファーストビュー外の画像は遅延読み込み

• バンドルサイズへの影響が許容範囲

  • 判断基準: 新しい依存関係追加時に webpack-bundle-analyzer で確認
  • 判断基準: 大きなライブラリ（> 100KB）は動的インポート検討

• 高コスト計算がメモ化されている

  • 判断基準: 複雑なフィルタリング/ソート処理はuseMemo
  • 判断基準: 100ms以上かかる計算は測定して判断

6. テスト

• コンポーネントに対応するテストファイルがある

  • 判断基準: ComponentName.tsx に対して ComponentName.test.tsx が存在
  • 例外: 単純なプレゼンテーショナルコンポーネント（10行以下）

• ハッピーパスとエラーケースをカバー

  • 判断基準: 正常系1つ + 異常系（エラー、空データ等）1つ以上

• モックが適切（実装の詳細をテストしていない）

  • OK: APIレスポンスをモック
  • NG: useState内部の値をモック

• テスト出力にact()警告がない

  • 判断基準: 非同期処理は waitFor() でラップ
  • 判断基準: ユーザーイベントは userEvent.setup() + async/await

• 重要なユーザーフロー向けの統合テスト

  • 判断基準: ログイン、検索、データ保存など主要機能に統合テスト

7. コード品質

• 命名規則が一貫している

  • コンポーネント: PascalCase（UserProfile）
  • 関数/変数: camelCase（handleClick, userName）
  • 定数: UPPER_SNAKE_CASE（API_ENDPOINT）
  • ファイル: コンポーネントはPascalCase.tsx、それ以外はkebab-case.ts

• 本番コードにconsole.log/debugger文がない

  • 判断基準: Grep で console.log 検索して0件
  • 例外: console.error/warn は許容（エラーログ用途）

• コメントが「なぜ」を説明している

  • NG: // ユーザー名を取得 ← コードを読めばわかる
  • OK: // APIの制限により2回に分けて取得 ← 理由を説明

• デッドコードや未使用のインポートがない

  • 判断基準: ESLint no-unused-vars 警告がない
  • 判断基準: IDEの「未使用」警告がない

• Lint と Format チェックが通過

  • 判断基準: npm run lint (Biome) が0エラー
  • 判断基準: npm run fmt (Biome formatter) 適用済み

出力形式

以下の構造でレビューフィードバックを提供:

## 🎯 概要
[全体的な評価: 承認、変更要求、または議論が必要]

## ⚠️ 発見された問題

### 重大
- [セキュリティ問題、致命的なバグ、データ損失リスク]

### 重要
- [パフォーマンス問題、UXの問題、アクセシビリティギャップ]

### 軽微
- [コードスタイル、最適化の機会、あったら良い改善]

## 💡 推奨事項
- [具体的な改善提案]
- [検討すべき代替アプローチ]

## 📝 詳細コメント
[行単位またはファイル単位の詳細なフィードバック]

使用するツール

1. Grep: パターン検索（例: any型、console.log、TODOコメント）
2. Read: diffで指摘された特定のファイルを調査
3. Bash: git diff mainを実行して変更を確認
4. Glob: すべてのテストファイルを見つけてカバレッジを確認

フラグすべきアンチパターン

• コンポーネント間での状態の重複
• 3階層以上のPropsドリリング
• 多数の依存関係を持つ大きなuseEffect
• 正当な理由のないインラインスタイル
• 定数の代わりにハードコードされた文字列
• エラー処理のためのエラーバウンダリの欠如

コンテキスト質問

レビュー開始前に必要に応じて明確化:

• これは新機能かリファクタリングか？
• 焦点を当てるべき特定の懸念事項はあるか？
• ブラウザ/デバイスのサポート要件は？
• 満たすべきパフォーマンスベンチマークはあるか？