はじめに
あけましておめでとうございます！ OHEYAGOの開発の田渕です。
OHEYAGOではTypeScriptを導入し、少しでもバグが発生しにくい状態を心がけています。
しかし、リリース直後(昨年9月末)にはTypeScriptの良さを活かしきれていない状態でした。具体的には、
- 型を明示していない箇所が多かった
- any型が横行していた
という状態でした。
そこで、リリース後に追加開発をこなしながら、コツコツTypeScriptの型システムの恩恵を受けやすいようリファクタリングを進めたので、その記録を書き留めておきます。
実際にTypeScriptを使っているけれどあまり秩序が保てていない、という方や、これからTypeScriptのプロジェクトを始めるけれど、eslintの設定をどうしようと思っている方などの参考になれば嬉しいです。
方針
typescript-eslintを活用して、機械的に最低基準を作り、それを徐々に引き上げていく、という方針で進めました。
eslintの設定ファイルのcommit logを見れば大体やったことがわかるので、適宜.eslintrc.jsonの状態を貼っていきます。
リリース直後の状態
およそ250個の.tsまたは.tsxファイルが存在し、行数換算で12000行程度でした。 .eslintrc.jsonは以下の通りです。(一部抜粋)
"extends": [ "plugin:@typescript-eslint/recommended" ], "rules": { "@typescript-eslint/interface-name-prefix": 0, "@typescript-eslint/prefer-for-of": 0, "@typescript-eslint/no-namespace": 0, "@typescript-eslint/camelcase": 0, "@typescript-eslint/no-explicit-any": 0, "@typescript-eslint/explicit-function-return-type": 0, "@typescript-eslint/no-unused-vars": 0, "@typescript-eslint/no-non-null-assertion": 0, "@typescript-eslint/explicit-member-accessibility": 0, "@typescript-eslint/no-use-before-define": 0, }
かなりのルールが無効化されていますね……。
まずはルールを分類
10個以上のルールを前にしてもどうすればよいのかわからないので、これらのルールを「すぐに直すもの」「徐々に直していくもの」「そのままで問題ないもの」に分類しました。
実際に順番を変えて、コメントも付けるようにしています。
分類する際には、それぞれのルールだけを有効化した状態でeslintを回し、件数や、すぐ直せそうかどうか、影響範囲はどのくらいか、などを精査します。
"rules": { "@typescript-eslint/interface-name-prefix": 0, // コーディングスタイルと合わない上に影響範囲が大きすぎるのでこのままで良い // すぐに直すもの "@typescript-eslint/camelcase": 0, // 一部の変数の命名を変えるだけ "@typescript-eslint/prefer-for-of": 0, // ごく一部のfor文を書き換えるだけ "@typescript-eslint/no-namespace": 0, // ごく一部のモジュールを書き換えるだけ "@typescript-eslint/no-unused-vars": 0, // ごく一部の変数を変えるだけ "@typescript-eslint/explicit-member-accessibility": 0, // 範囲は大きいけれど、間違えたらコンパイルが落ちてくれるはずなので安心 "@typescript-eslint/no-use-before-define": 0, // 範囲は大きいけれど、関数の順番を変えるだけなので安全にできるはず // 徐々に直すもの "@typescript-eslint/no-non-null-assertion": 0, "@typescript-eslint/no-explicit-any": 0, "@typescript-eslint/explicit-function-return-type": 0, }
すぐに直すものについては、1日くらい時間を取って、まとめてささっとやりました。
「いまやらないと、コードの変更でどんどん修正箇所が増えていくので、どんな時も気づいた瞬間にやるのが一番楽なはず」という信念を持って頑張ります。
結局、残ったルールは、
{ "@typescript-eslint/no-non-null-assertion": 0, "@typescript-eslint/no-explicit-any": 0, "@typescript-eslint/explicit-function-return-type": 0, }
この3つでした。
documentを読んで、デフォルトで無効だけれど、有用なものを漁る
このタイミングでドキュメントを一通り読んで、他にも入れたいルールを探します。@typescript-eslint/typedefを導入するべきと感じたので、それもルールに追加します。
{ "@typescript-eslint/no-non-null-assertion": 0, "@typescript-eslint/no-explicit-any": 0, "@typescript-eslint/explicit-function-return-type": 0, "@typescript-eslint/typedef": 0 }
直すべきルールは以上です。あとはやるだけです。
一個ずつルールを潰していく
開発も同時に進めないといけないので、手戻りが発生しないように、速やかにeslint上でerrorが出せるところまで持っていき、マージしてしまうのが大事です。
まずは、explicit-function-return-typeに目をつけました。200箇所ほどありましたが、簡単な型であればすぐつけることができます。わからない場合には一旦anyをつけてでも、とりあえずexplicit-function-return-typeを違反したらerrorが出る状態まで持っていきます。
型アノテーションを付けるだけなので、コンパイルが通ってさえいれば影響は出ていない(はず)という気持ちでサクサク進めます。 悩んだらanyをつけてしまえば良いので、200件の修正でも、意外と数時間もあれば終わります。
同様に、typedefについても200箇所程度、数時間で進めてしまいました。
no-explicit-anyの修正
こちらについては、頭を使わないと修正できないものが150箇所くらいありました。
今まで散らかしてきた分のお型付けの時間です。
基本的にはやるしかないのですが、VSCodeのhintを使うとやりやすいです。
バックエンドからデータを受け取る際のJSON.parseの返り値がanyなので、そこは自力で型assertionしてしまいます。
また、redux周辺の型が難しかったのですが、思い切ってすべて書き換えて、reduxをやめてしまいました！
(とはいっても、3ページ程度でしか使われていなかったので、そこまで大変ではなかったです)
ここまで進めれば、undefinedとnull以外での型に起因するエラーは起こらなくなるはずです。
non-null-assertionの修正
一つずつ場合分けするだけですが、至るところでnon null assertionをしているのは、そもそも設計がおかしいことが多いと思います。
internal APIでもviewからのデータ受け渡しでも、TypeScriptにとってのIO部分で型アノテーションとnullチェックをしてしまえば、それより子のコンポーネントではnon null assertionはほぼ不要になります。
子コンポーネントでHTMLからデータを取得したりしている部分は、そもそもentrypointで取得するように書き換えました。
また、entrypointでのnon null assertionは許容しました。
最終的なeslintの設定
現在はeslint.jsになっていますが、その設定をそのまま貼り付けます。
module.exports = { env: { browser: true, es6: true }, extends: [ 'airbnb', 'airbnb/hooks', 'plugin:@typescript-eslint/recommended', 'prettier/react', 'prettier/@typescript-eslint', 'plugin:prettier/recommended' ], parser: '@typescript-eslint/parser', parserOptions: { sourceType: 'module', ecmaFeatures: { jsx: true } }, globals: { Atomics: 'readonly', SharedArrayBuffer: 'readonly' }, plugins: ['prettier', '@typescript-eslint'], rules: { 'prettier/prettier': [2, { singleQuote: true, parser: 'typescript' }], 'no-unused-expressions': [2, { allowShortCircuit: true }], 'react/jsx-filename-extension': [2, { extensions: ['.tsx'] }], "import/extensions": [2, { extensions: [".ts", ".tsx"] }], '@typescript-eslint/no-unused-vars': 2, '@typescript-eslint/explicit-function-return-type': 2, '@typescript-eslint/no-explicit-any': 2, '@typescript-eslint/typedef': 2, '@typescript-eslint/no-non-null-assertion': 2, '@typescript-eslint/no-inferrable-types': 0, // typedefと相性が悪いので切る '@typescript-eslint/interface-name-prefix': 0, // 現状のコーディングスタイルとは合わない // TODO: 直し方がわからない部分があるので、一旦切っている 'jsx-a11y/label-has-associated-control': 0, 'jsx-a11y/click-events-have-key-events': 0, 'jsx-a11y/no-static-element-interactions': 0, 'jsx-a11y/anchor-is-valid': 0 }, overrides: [ { files: [ '**/entrypoints/**/*.{ts,tsx}', ], rules: { '@typescript-eslint/no-non-null-assertion': 0 } } ], settings: { react: { version: 'detect' }, "import/resolver": { node: { extensions: ['.js', '.jsx', '.ts', '.tsx'] } } } };
この設定であれば、かなり秩序を持った開発が進められると思います。
おわりに
全体にかかった時間としては、僕一人がコミットして丸5日くらいでした。その最中に、そもそも型がおかしい部分(潜在的なバグ)などもいくつか発見できました。
プロダクトオーナーに対しては多少ゴリ押ししましたが、5日かける価値は十二分にあったと思います。
大きなメリットとして、
- 開発中にデバッグに割く時間が減る
- リリースバージョンにバグを埋め込む可能性が減る
- 「anyは避けよう」などのレビューをする必要がなくなる
などが挙げられます。コンパイラが人間の代わりに仕事をしてくれるので、かなり楽できます。
仮にチーム全体で1日30分(0.5[h/日])の作業時間が削減できると仮定します。(エンジニア3名+デザイナー2名なので、「一人1日6分だけ楽できる」という控えめな見積もりです)
作業時間は8[h/日]×5[日]=40[h]なので、40[h]/0.5[h/日]=80日の営業日が経過すればペイするはずです。
実際、eslintがこの設定になってから1ヶ月開発した肌感としては、4ヶ月も経てば5日の作業時間は余裕でペイすると確信しています。皆様もぜひ！
