🦁

大型開発にそなえるための割れ窓改修（AdaInspector Cloudを例にして）

2022/11/28に公開

TypeScript

こんにちは。アダコテックWebフロントエンジニアの谷口です。

普段は、アダコテックが提供しているAutoMLクラウドサービス、AdaInspector Cloudの開発を担当しております。

今回は、プロダクトリリースから一定期間経ち、新規開発案件が落ち着いたタイミングで、サービス開発担当のエンジニアとしてどのようなことに取り組んできたのかを記事にまとめました。

前提

AdaInspector Cloudは2021年の7月に満を持してリリースされて以降、大手の製造業のクライアントさんから利用いただきながら、細かい機能改善施策を繰り返してきました。

https://adacotech.co.jp/news/3273

機能改善を行う中で、経営サイドから一旦、AdaInspector Cloudの利用ケースから得られた仮説をベースに再度、市場調査および課題抽出を行うという発表がありました。これに伴い、プロダクト開発としては、新規開発Issueを消化しきって2022年3月ごろに開発が落ち着きます。

このように、新規開発Issueがないと、ソフトウェアエンジニアとしては「少し退屈だな〜」と感じるかもしれません。

ただ、こんなタイミングだからこそやらなくてはいけないことがあります。

そう、割れている窓の改修作業です

私はちょうどAdaInspector Cloudがリリースされる直前の2021年の6月から開発に携わったのですが、以下のような割れ窓理論さながら、

運用されていないE2Eのテストコード
CIで大量に出力されるlintエラー
よくよく追ってみるとどこからも呼ばれていない関数や変数
OpenAPIの定義の誤り

と言った、直接機能には影響がないものの、新機能開発する上では弊害となりうるような事象が多々発生していました。

このような事態は対応優先順位は低いため、プロダクト開発においては放置されがちですが、プロダクトの規模が大きくなればなるほど、新規メンバーが参画する際、不必要に認知コストが上がってしまい、結果として開発速度を落としてしまう要因となってしまいます。

子供の部屋を片付けをするような感覚で、一つずつ割れ窓を改修したので、今日はそれぞれの課題と解決策を紹介していきたいと思います。

割れ窓改修施策一覧

TypeScriptのany型の撲滅

【発生していた課題】
anyを使用するということは、pureなJavaScriptで実装していることと同義で、TypeScriptを導入している意味がなくなってしまいます。

例えば以下の通り。stringの値を代入した変数でhogeというメソッドを呼び足してもtypeエラーにはなりません。

const anyVariable: any = "fuga"
anyVariable.hoge()
// Uncaught TypeError: anyVariable.hoge is not a function

any型で定義された変数で上記のようなTypeErrorに遭遇してしまうと、そもそもどのような型定義を想定して実装されたのかが分からないため、不具合の原因特定にも時間がかかってしまいます。

【解決策】
eslintの'@typescript-eslint/no-explicit-any’を採用して、anyを使用している箇所を検知して、不要なanyを削除します。
https://github.com/typescript-eslint/typescript-eslint/blob/main/packages/eslint-plugin/docs/rules/no-explicit-any.md

TypeScriptは型推論をしてくれるため、よっぽど変な実装をしていない限りは、逐一型定義をしなくても、変数の型が一意に決まります。

const anyVariable = "fuga"
// プロパティ 'hoge' は型 string に存在しません。
anyVariable.hoge()

どうしても明示的な型定義が難しい場合には、unknownを使用することで、想定外の型定義エラーを防ぎましょう。

const unknownVariable: unknown = "fuga"
// オブジェクト型は 'unknown' です。
unknownVariable.hoge()

OpenAPIの定義を実態に合わせる

【発生していた課題】
AdaInspectorCloudでは、API側の仕様ドキュメントとしてOpenAPIを採用しており、openapi-generatorを使用することで、OpenAPIの定義からそのままTypeScriptの型定義を生成しています。しかしこのOpenAPIの定義が間違っていると、Cannot read properties of undefinedのエラーに遭遇します

フロントエンドのエンジニアとしては、OpenAPIの定義に誤りがあることなど、知る由もないため、

わざわざAPIから返ってくるレスポンスを確認
→レスポンスに該当のプロパティがない
→サーバーサイドのエンジニアにヒアリング
→「あ、ごめん、OpenAPIの定義が間違ってました」
という無駄なやり取りおよび工数が発生していました。

【解決策】
地道に実装とopenAPIの定義を比較して、openAPI側が間違っている場合には修正作業を行います。
サーバーサイドはmarshmallowというpythonのJSONSerialzierを採用していたため、Schemaの定義とOpenAPIの定義のプロパティを確認するだけで、比較的容易に定義漏れに気づけました。
https://marshmallow.readthedocs.io/en/stable/

上記の対応で漏れていた箇所については、都度フロントエンドエンジニア側からOpenAPIの修正PRを出してサーバーサイドエンジニアにレビューする体制を作ることで、おおよその定義と実態の食い違いを潰し切ることができました。

lintエラーの撲滅

【発生していた課題】
フロントエンド側にはeslintが設定されており、CIでもlintが確認できるようになっていたものの、100件以上のlintエラーがPR作成のたびに流れ、誰もメンテナンスしない状況でした。
エラーが流れるたび「あ、これは直さなくていいエラーだから」というやり取りが発生していましたが、これこそまさに割れ窓理論で、むしろ直したほうがいいlintエラーが隠れてしまいます。

【解決策】
.eslintrc.jsのRule Severitiesをwarnからerrorへ変更しました。
https://eslint.org/docs/latest/user-guide/configuring/rules

warnはexit codeがトリガーされないため、CIに組み込んでもworkflowが落ちることはありません。
中途半端にwarnを設定して無駄に出力を増やすくらいなら、offにしたほうがいいです。チーム内で必ず守ったほうがいいと思うRuleのみを設定するようにしましょう。

例外として、ruleとして設定しておきたいが、この行だけはescapeしておきたいというコードについては、eslint-disable-next-lineの追加して、無駄な出力を防止します。

// eslint-disable-next-line no-console
console.log(message)

不要な変数や型定義の削除

【発生していた課題】
既存のAdaInspector Cloudのコードには不要な変数や関数が散在しており、新機能開発や不具合修正の際に無駄な影響範囲調査のコストがかかっていました。
このような不要なコードはすでに昔プロジェクトに参画していたメンバーが書いたコードであることが多く影響範囲も読みきれないので、わざわざ削除しようというインセンティブが働きづらいです。
結果として、開発すればするほど、負債として溜まっていくことになります。

【解決策】
ts-unused-exportsというライブラリを使い、どこからもimportされていない変数や型定義をCIで検知して、そのようなコードが有る場合にはPRのマージができないように設定しました。
https://github.com/pzavolinsky/ts-unused-exports

不要なコードの削除という楽しくない作業は、CIで開発メンバーに対して強制力を持たせるのが一番効果的です。

E2Eテストの導入

【発生していた課題】
E2Eテストについては、Cypressを導入していましたが、仕様変更に弱い設計となっており、もはやローカルでも動くかどうかもわからない、、、という状況でした。

【解決策】
コードベースのテストはフロントエンドエンジニアしかメンテナンスできないため、AutifyというノーコードによるE2Eテストへの舵切りをしました。
https://autify.com/ja

Autifyでは、Seleniumブラウザ上での操作を保存して、テスト時に再実行してくれるため、フロントエンドエンジニア以外のメンバー(サーバーサイドエンジニアやプロダクトマネージャー)などもテストシナリオを作成することが可能です。
※画像のように、登録した操作は削除したりコピーして追加したりすることができます。

要素の取得はAIによる自動判定のため、CSSのclass名の変更などに強く、実装を変更した際に、テストシナリオも変更する、、、という手間は比較的少ないです。

ただ、要素によっては、具体的にselectorを指定しなくてはいけないケースもあるため、トライアルで運用に耐えうるかどうか事前に検証することをおすすめします！

Visual Regression Testの導入

【発生していた課題】
全体影響のあるCSSのリファクタリングなどをした際に見た目のデグレを検証する必要がありますが、人の目でリグレッションテストを行っても、どうしてもデグレを見逃してしまうことがあります。
CSSの設計の見直しはしたいが、リグレッションテストの工数を考えると割に合わず、古い設計のまま実装せざるをえない、という判断になりがちでした。

【解決策】
ChromaticというVisual Regression Testという見た目の自動テストの導入を行いました。ChromaticではstorybookのStory単位でComponentの差分検知を自動で行ってくれます。

例えば添付の画像はリストコンポーネントの高さを変えたときの差分検知です。

これは意図通りのデザイン変更でしたが、PRをマージする前に想定外のレイアウト修正が入っていないか確認できます。

Visual Regression Testでは画像単位で差分を取ってくれるので、リファクタリングでCSSを変更しても見た目が変わらない場合には、テストが通る仕組みになっています。

もちろんCIとの連携も可能で、PRを更新するたび自動でベースブランチとのUIの差分検出をしてくれます。

最後に

いかがでしたでしょうか？
このような割れ窓改修作業は地味だし、面倒だし、つまらないし、、、ではありますが、いずれくる新機能開発を快適にするために必要な作業です。
新規プロダクトをリリースすると、ついなおざりになってしまいがちな「割れ窓」ですが、今回の記事が快適な開発環境を作る一助になれば幸いです。

宣伝

アダコテックではデータエンジニアの採用を強化しています。製造業界のお客様に対して、機械学習の知識がないユーザーでもPDCAの回せる機械学習基盤を構築するミッションを担う非常にチャレンジングなポジションになります！

興味のある方はぜひ以下の代表河邑のnoteをご覧ください！

求人の詳細は以下の通りとなります。

ぜひ皆様の応募お待ちしております！