短くまとめると
React に PR を送ったら、メンテナからの反応が渋そうだったので萎えていたら、メンテナが自分でブランチ切って修正してマージしていた。
教訓
- OSS ではメンテナの言葉に一喜一憂しない鋼の心が必要
- OSS では調査に時間がかかっても、できるだけすぐレスする
- OSS で PR を送る前に テストとリント(と React の場合はタイプチェック, Flow)を通す
PR を送る前からメンテナがマージするまでの流れ
普段自分がしていたこと
去年の12月になって、1年間業務で使っている Next.js の内部構造を知らないのも変だなと思い Next.js のソースコードを読むようになりました。その過程で、server とちょっと build と client が分かるようになったので、今年の 1月から issue を探して、そのバグ修正を行うようになりました。
ことの発端
今回のことの発端は、この issue でした。
Next.js の issue ですが、throw した カスタム Error もしくは object が [object Object] で表示されてしまうとのこと。
そこで自分なりにこの issue を調査することになったのですが、調べてみる(node_modules/next/dist/compiled/next-server/app-page.runtime.dev.js にデバッグコードを入れる)と、React Server Component で一回 server component を JSON 形式にする段階で エラーが [object Object] になっていることが分かりました。
React の PR を送る
ということで、React に PR を投げてみることにしました。
修正は簡単で、object が throw された時の条件分岐を加えただけです。
React の PR のフォーマットは初心者にも分かりやすく、
- prettier を通す
- lint を通す
- flow (Meta製のJavascriptタイプチェック) を通す
- テストを通す
- CLA を出す
ことを素直にしました。
いざメンテナのレビュー
いざ、メンテナのレビューの段階になると、結構手厳しい意見が...
大まかにいうと2点で、
- object から message を取っているだけでは、エラーとは言い切れない。object から楽観的に情報を取り出せないものか? react でも既存でそういう関数はあるけど
- そもそも カスタムエラーが object を返しているのはおかしい。何か実装ミスがあるのでは?
豆腐メンタルの自分にはどう対処すればいいか分かりません。
まず、object から楽観的に情報を取る関数が JSX をパースする機能が dev でしかないのですが、そもそも JSX がエラーフォーマットに入るかが分からない(入らないと思っていた)ので、新しく エラーフォーマットを作る関数を作るべきなのか?
次に、Next.js で issue を切った人の実装ミスなら、そもそも直してくれるか分からない。
そんなことで、調査が終わってから、1日悩んでいました。
1日寝かしたら
そうしたら、メンテナが新しいブランチを切って、修正しているではありませんか!
それも修正は新しい関数を作るのではなく、devではJSXを表示してくれる提示された関数を使っているだけという...
なんというか実装方針で考え過ぎていただけでした!本当に確認が大切ですね。
その後の教訓(再掲)
メンテナだって、利用者に嫌な顔はしたくないはずだから、自分で直せというだけではなくて、実装を変えてくれることもありました。メンテナの言葉だけでなく、コードの利便性が上がることを正々堂々主張してもいいのかもしれません。
それとメンテナの返信スピードも早かったんで、自分も即レスを心がけたいですね。
そんなことで、今後は鋼の心を養うことも怠らず、OSS で PR を送りたいと思いました。
現場からは以上です。