はじめに
こんにちは！株式会社アルファドライブの佐藤です。
この度、タイトルにもあるように2000行台（最大2415行）の巨大なReactコンポーネントをいくつかリファクタリングしました。この作業には大量の変更が生まれます。2月の私のmainブランチへのコミットは11905 (+5683, -6222)行でした。結論、ペアレビューの時間を設けることで約1ヶ月でこの膨大な変更をやり切ることができたので、今回はそこに至るまでの試行錯誤を記録したいと思います。
背景
私たちが扱うプロダクトには、2000行を超えるようなReact クラスコンポーネントがいくつか存在していました。このような巨大なコンポーネントができてしまった理由は、このプロダクトは外部に委託して開発された過去があり引き渡された時点で適切にコンポーネント分割がされていなかったこと、引き渡された後も開発メンバーが少なくリファクタリングに工数を割けなかったことが挙げられます。
そんな中、Create React AppからNext.jsへの移行を最終目的としてフロントエンドの基盤改善プロジェクトが立ち上がりました。その際に、React Routerといったライブラリのバージョンを上げる必要があり、4つのクラスコンポーネントを関数コンポーネントに置き換える必要がありました。それらのコンポーネントは頻繁に利用される画面のコンポーネントであるにも関わらず、非常に難解なコードになっていました（一つのコンポーネント内に50を超えるstate, 60を超えるメソッド, 250を超える// ts-expect-errorを想像してください）。また、工数が最近まで足りていなかったのもあり、古くからあるこれらのコンポーネントにはテストが存在していませんでした。そのため、関数コンポーネント化のような大掛かりな修正をするには、いかに動作を担保するかという問題がありました。
やり方1 機械的置き換え
一番の目的は関数コンポーネント化だったため、最初はファイルごとにプルリクを出すイメージで、機械的に置き換える方法を模索しました。
React公式のマイグレーションドキュメントなどを参考に、Reduxを使用している本プロダクトに合わせた移行手順書を作成しました。実際にはコード例なども用意しましたが、以下は簡単なイメージです。
- コンポーネントの宣言をクラスから関数に置き換える
- 状態管理を
useStateで置き換える - ライフサイクルメソッドを
useEffectで置き換える - refを
useRefで置き換える - 関数にconstをつける。関数やstateの
this.を消す - 該当コンポーネントの機能を洗い出し、動作チェックリストを作る
しかし、このような機械的な置き換えはうまくいきませんでした。
第一に、数千行もあるようなファイルで6の動作洗い出しをやるのが辛すぎます。機械的な置き換えから逸れて自身の可読性のためにコードの書き方を修正したくなりますが、このマイグレーション作業はやり切るまでコンパイルエラーになるため、途中で何をやってるかわからなくなり頭がおかしくなります。差分も莫大になるため、レビューは不可能に近いです。
やり方2 コンポーネント分割
そこで、先ほどの手順書の0番目の項目として、
classコンポーネントが膨大でレビューが困難になる場合は、機能の網羅が可能なサイズでコンポーネント分割する
という項目を加えました。これによって、プルリクごとに動作や影響を列挙できるようになりました。一定の変更ごとにコンパイルに成功し動作を確認できるので、少しずつマージできる安心感がありました。今後の運用を考えても、コードが適切なサイズで分割されることは明らかにメリットです。
作業がしやすくなった私は、毎日2,3のプルリクを上げまくりました。しかし、レビュー担当の方はなかなかレビューしてくれません。「どれを見ればいいの？」「ちょっと多すぎる」という具合です。
やり方3 ペアレビュー
プルリクが溜まると、私自身も修正があった時の手戻りが気になったりレビューリクエストが多すぎて申し訳なくなったりして作業を進めづらくなりました。 そこで、先輩エンジニアのカレンダーを1時間ほど抑えてペアレビューの時間を設けました。
ペアレビューをすると、今までの滞りが嘘のようにPRをマージしていけました。PRが5件くらい溜まっているから1時間かかるかなと思ったら、30分くらいで終わることもありました。今まででは考えられないレビュー効率の良さだったので、2,3日に一回ペアレビューの時間を設定し、リモートワーク中心のチームですが何度か出社して隣のソファに座りながらレビューをしました。何度かペアレビューをして、次のようなメリットを感じました。
- 大量に差分があっても「このコードはここを移動しただけです」「この変更はこういう目的です」のように意図を伝えることでレビュワーがコードを理解しやすい
- レビュワーが気軽に疑問点の確認や指摘ができ、その場で解決する
- 「後々修正するので一旦マージしたいです」みたいな空気感が伝わる
- 「疲れたから今日はここまでにしよう」みたいな空気感が伝わる
- レビューしてくれることへの感謝を伝えたり、逆に「いいことしてるよ」みたいな声をかけられたりするのでお互いにストレスが軽減される
特に、後半の気持ちの伝達は大きかったです。プルリクを跨いで後で修正する旨が伝わったり、まだレビュー依頼中のプルリクが残っていてもその日のレビューの辞め時が伝わりました。大量のプルリクエストを作ることでレビューしてもらうのが申し訳ない気持ちになって作業の手が重くなった時期もあったのですが、レビュワーの励ましの言葉で作業に自信を持つことができました。
まとめ
ペアレビューのおかげで、今のところデグレ無く、何箇所かバグを治した上で、巨大なコンポーネントをリファクタリングできました。今回のような膨大な作業に限らず、多少の説明が必要なレビューに関してはペアレビューの方が効率よく、そして気持ちよくマージしていけるのではないかと思いました。今後、ペアレビューを実践すべきシチュエーションの解像度を高めて、チームの開発フローも改善していきたいです！
