toshi-toma blog

主にフロントエンド、作業ログあとは色々なメモ ✍️ 🍅

eslint-plugin-reactへのコントリビュートに挑戦してみた ~1月の活動を振り返る~

eslint-plugin-reactへのコントリビュートに挑戦してみた

今年からある程度まとまった単位で活動をブログなどの記事としてアウトプットしておくことで、それを使った振り返りができるように意識しています。

2020年もはや1月が終わり、どんなことをしてたか振り返るとeslint-plugin-reactへのPR作成に時間を使っていたのでまとめを書きます。

eslint-plugin-react

GitHub - yannickcr/eslint-plugin-react: React specific linting rules for ESLint

  • eslint-plugin-reactはESLintのプラグインの1つで、React用のルールが用意されています
  • 有名なESLintのconfigであるeslint-config-airbnbなど、様々な場所で利用されています
  • 最近はReactを使うなら、TypeScriptを併用して使うことが多いので不要になっているルールも多い気がしています

OSSへのコントリビュート

去年からOSSへコントリビュートしていくモチベーションが上がっていたので、今年は去年以上にやっていくぞということで定期的にIssueを眺めていました。

OSSへのコントリビュート経験が少ないので、どう進めたらいいか分からなかったので1on1などの時間でOSS活動をやってるチームメンバーと話して、ある程度どんな風に進めるか分かってきました。

実際にやる時は

  • 自分も使ってるOSSを1つ選んで、最近のIssueやPRを眺めてみる。あまり色々なOSSを見ない。最初は1つに絞るくらいが良さそう
  • 「help wanted」がついているIssueを見て、自分にもなんとなくできそうかなーくらいに思えるものがあれば探究してみる

あたりを意識してやってみました。

そこで探究した結果、eslint-plugin-reactに2つPRを投げることができました。

[Fix]: Does not validate for LogicalExpression #2533

これは去年の12月にやっていた内容です。

対応したIssue

react/prop-types does not validate missing propTypes for functions in version 7.17.0 · Issue #2526 · yannickcr/eslint-plugin-react · GitHub

作成したPR

[Fix]: Does not validate for LogicalExpression by toshi-toma · Pull Request #2533 · yannickcr/eslint-plugin-react · GitHub

Issueを見て、何がbugなのか分かりやすかったので見てみました。
内容的には、以下のようなコードでreact/prop-typesのルールがチェックされていないというものです。

return (
    ordering.length > 0 && (
      <Paginator items={ordering} pageSize={10} />
    )
)

最初は、Issueに書いてある通り、prop-typesのコードを見にいってデバッグしてみました。 ただ、最初の起点になるコードで、そもそも該当のコードがチェック対象になっていないことが分かり、さらに上位のコードをデバッグしていきました。

その結果、コンポーネントを取得するutilコード側で、今回のケースが漏れていることが分かりました。

対応としてはLogicalExpressionもチェック対象に含めるようにしました。

ESLintのプラグインのテストはvalidとinvalidのケースを書くだけなのでテストを追加して終わりです。 ちょっと修正が入ったけど、さくっとマージされました。

[Fix] jsx-indent: Does not check indents for JSXText #2542

対応したIssue

react/jsx-one-expression-per-line · Issue #2467 · yannickcr/eslint-plugin-react · GitHub

作成したPR

[Fix] `jsx-indent`: Does not check indents for JSXText by toshi-toma · Pull Request #2542 · yannickcr/eslint-plugin-react · GitHub

これもIssueを見て、なんとなくできそうな気がしたので見てみました。

内容的には、以下のようにJSX内にplaneなテキストがある場合に、それがindentされないというものです。

return (
    <div>
text
    </div>
);

最初はIssueのタイトルにあるreact/jsx-one-expression-per-lineのコードを見に行ったのですが、デバッグしてみると、特にindentを考慮していないのが分かりました。 原因はreact/jsx-indentにあることが分かりました。

そこからはjsx-indentのコードをデバッグしていったのですが、今回の対象となるJSXTextノード用のコードが一切なかったので実装が必要でした。

また、JSXTextは他のNodeの場合と情報がだいぶ違うので自分で正規表現を使って現在のインデント数を計算したり、fix時の処理を実装する必要がありました。 ここでJavaScript正規表現について結構調べ直しました。

対応的には、JSXTextノード用のハンドラーを設定して、現在のインデント数を正規表現で取得。期待するインデント数と違う場合はreport関数に渡して、FixerFunctionでnodeのテキストを期待するインデント数でreplaceするということを行いました。

他のノードと形式が特殊すぎて、だいぶ実装が大変でした。

感想

良かったこと

今回、OSSへのコントリビュートを探究してみて、良かったことがいくつかありました。

まず、勉強の題材としてめちゃ良さそうだと思いました。業務で経験してない実装だったり、既存のコードを読むのも勉強になりました。

また修正内容を考える時に良いロジックを考えたり、ASTや正規表現の勉強になったりなどなど色々あります。

あとは、Issueを見ると関係するライブラリにも詳しくなれると感じました。例えば、ECMAScriptの新しい構文やReactの新しいAPIに対応する時は、eslint-plugin-reactのIssueに上がってきていました。

継続することで新しいライブラリの機能にも詳しくなれそうです。

追記(2020/02/02)

今回やってみて一番良かったのは、「OSSへのコントリビュート無理ではないな。」ということが分かったことです。

自分が使ってるOSSへのコントリビュートってハードル高く感じてましたが、なんとなく分かりやすそうなIssueを1つ選んで、じっくりコード読んで1つ1つデバッグしてみたら、なんとかなるのが実感できました。

自分も2つめのPRは1週間くらい考えながら作ったりしたので、時間をかければなんとかなるな。と感じました。

あとは、有名だけどメンテされてないOSSも多いのでは。と思いました。eslint-plugin-reactは有名ですが、bughelp wantedラベルがついたIssueがまだまだあるけど、積極的にコントリビュートしてる人も多くなさそうです。他にもこういったOSSは多そうです。

学び

今回対応したIssueどちらも、最初に想定した場所とは違う部分が原因でした。Issueに書いてある内容に囚われすぎないようにしようと思いました。

また、普段の開発はモブプログラミングなのでレビューというフローをあまり経験しないのですが、OSSだとちょっと実装方針に迷っても気軽に相談できなかったので、これでいいのかなーと不安なままPRを作成するような進め方になりました。

普段の開発だと、チームメンバーに実装する前にさくっと相談できるので違うポイントだなと思いました。

後日談

#2542のPRがマージされた後に、そのPRが原因のbugが報告されていて焦りました。

「壊してしまってごめんなさい。。。もっと注意します。。。」という気持ちでしたが、失敗を恐れずに、注意はしつつ、これからも頑張っていこうと思います。