2022/11/11
非同期システムでESLint の for..of 警告に従いたくなかった話
この記事では、現在Node.js で行っている機能のTypeScriptプログラムにESLintで静的解析をかけたところ、for..of 構文に関する警告が発生し、その対処について記載します。
目次
この記事で伝えたいこと
これまで主にPHPエンジニアとして開発してきた筆者が、Node.js でTypeScriptによる機能実装を行う事になったとき、非同期実行がベースであることに気づきました。
また、開発手法に継続的インテグレーションやテスト駆動開発を採用している中、ESLint の警告もクリアすべく努力してきましたが、今回解決をしなかったこと警告があった点を伝えたいです。
手法
Node.js におけるTypeScript の for..of の処理の記述
Node.js上でTypescriptを使ってGoogle CloudのNoSQL DBのCloud Firestore(以下Firestore)のデータにアクセスするプログラムで、単体テストに使用するテストデータをFirestoreに登録後、すぐにそのデータを参照する処理を書きました。該当部分のコードを抜粋すると以下のようになります。
1 2 3 4 5 6 7 8 9 10 11 12 | const firestore = new Firestore(); const testData = Array(100).fill({ key: "value"}); (async () => { // 1. ここでDBに100件データを追加 for (const value of testData) { await firestore.collection("test").add(value); } // 2. 直後にDBのデータを参照(ここでは件数取得) console.log((await firestore.collection("test").get()).docs.length); })(); |
for…of 記法のESLintによる指摘
上記のコードを ESLint で静的解析を行ったところ、
1 2 3 | for (const value of testData) { await firestore.collection("test").add(value); } |
の箇所で、以下の2つのエラーが検出されました。
no-await-in-loop
Performing an operation on each element of an iterable is a common task. However, performing an
await
as part of each operation is an indication that the program is not taking full advantage of the parallelization benefits ofasync
/await
.Usually, the code should be refactored to create all the promises at once, then get access to the results using
Promise.all()
. Otherwise, each successive operation will not start until the previous one has completed.
要約 loop内で各要素毎にawaitを使わず、すべてのプロミスを作成後、Promise.all()を使って結果を取得してください。
no-restricted-syntax ( ForOfStatement )
iterators/generators require regenerator-runtime, which is too heavyweight for this guide to allow them. Separately, loops should be avoided in favor of array iterations.
要約 for…ofはイテレータを使っており重くなるので使用しないでください。map()関数などを使えば配列の反復処理が可能なのでそちらを使ってください。
これらのルールはESLintをrecommendedベースで使っている場合には警告にすらならないのですが、スターフィールドで採用しているairbnb-baseベースのルールを適用した状態だとエラーとして扱われます。
そのため、この度、実装の置換を検討しましたが、主に no-restected-syntax エラーについて課題が発生しました。
forEach への置き換え検討/実装
そのため、for…of 構文を止め、forEach関数を使うように書き換えを行いました。先程のコードイメージでいうと下記のようになります。
1 2 3 4 5 6 7 8 9 10 11 | const firestore = new Firestore(); const testData = Array(100).fill({ key: "value"}); (async () => { // 1. ここでDBに100件データを追加 testData.forEach(async (value) => { await firestore.collection("test").add(value); }); // 2. 直後にDBのデータを参照(ここでは件数取得) console.log((await firestore.collection("test").get()).docs.length); })(); |
すると今度はテストが失敗するようになりました。書き換え前のコードでは、1件づつ追加完了を待ち合わせて、100件データを追加後に全データを取得して件数を表示しているため毎回100が表示されていたのが、書き換え後から0になったり6のように毎回結果が変わるようになり、その結果テストが失敗する状態となっていました。
結論からいうとこれはforEachの仕様によるもので、forEachが引数で受け取るコールバック関数は同期関数を想定しているため、async〜awaitを使った関数を渡してもプロミスをまたずに実行する仕様であったため、上記コードイメージだと
1 2 3 | testData.forEach(async (value) => { await firestore.collection("test").add(value); }); |
のループで追加処理が完了する前に
1 | console.log((await firestore.collection("test").get()).docs.length); |
が実行されていました。
参照: forEach 仕様
Array.prototype.forEach() – JavaScript | MDN
https://developer.mozilla.org/ja/docs/Web/JavaScript/Reference/Global_Objects/Array/forEach
for…of 記法での実装に戻した
今回のコードはテストコードの先頭で1回だけ実行される処理であり、処理時間をそれほど気にする必要がなかったため、書き換え前のfor..ofを使った処理に戻した上で、ESLintのルールを無視するコメント(// eslint-disable〜)を書くことでエラーを抑止することにしました。
ファイル単位やプロジェクト全体で無視する設定もありますが、指摘された場所によって無視して問題ないかを常に確認すべきと考えて、今回は行単位のコメントによる無視設定を採用しました。
1 2 3 4 5 6 7 8 9 10 11 12 13 14 | const firestore = new Firestore(); const testData = Array(100).fill({ key: "value"}); (async () => { // 1. ここでDBに100件データを追加 // eslint-disable-next-line no-restricted-syntax for (const value of testData) { // eslint-disable-next-line no-await-in-loop await firestore.collection("test").add(value); } // 2. 直後にDBのデータを参照(ここでは件数取得) console.log((await firestore.collection("test").get()).docs.length); })(); |
この課題からの反省と方針
今回対面した、ESLint が for..of 構文を指摘する意図を調べてみたところ、anb のコメント https://github.com/airbnb/javascript/issues/1271#issuecomment-283736133 では、
for..of will not be allowed by the airbnb preset because it requires Symbols to exist, and Symbols can not truly be polyfilled – and regenerator-runtime is too heavyweight. This concern outweighs the minor concern that side-effecty iterations (which are rare) are slightly more statically detectable as for..of than as .forEach. |
と記載されています。
私たちは、Node.js 等による、非同期のプログラムを考えた場合、for..of から、forEach への変更の最適解を見つけることが出来ずにいる状態です。
また今回は、テストスクリプトの処理というお客様に直接触れることがない環境であることや、チームメンバー間でのソースコードの可読性に対する意見もありました。現実解としては、for..of エラーの無視をソースコード上に指定することが最適と考えています。
なお、今回のこの問題を解決するに当たって、クラスメソッド社のテックブログ、「DevelopersIO」 の記事、 [小ネタ]forEachではasync/awaitが使えない を参照しましたが、この記事では、for..ofのループをmap()とPromise.all()を使った処理に書き換えるやり方が紹介されています。しかし、この方法では並列実行されることによりデータの追加順が保証されなくなるため、現在開発している機能のテストへの影響が大きく、現状採用しない選択をしています。
まとめ
今回は、Node.js 等で非同期プログラムの実装を行っていった段階で、ESLint の指摘への対処について記載しました。
Author Profile
ARIKAWA
バックエンドエンジニアです。 自転車が好きです。
SHARE