Zenn
マナリンク Tech BlogPublicationへの投稿
🦔

【ESLint】Reactで”&&”で分岐した際、うっかり「0」を表示しないためのルール3選

meijin
2024/05/13に公開
React
TypeScript
ESLint
tech

Reactでコンポーネントの表示を分岐する際、

{someVariable && <Component />}

のように実装することが多いと思います。

このような実装の罠として、変数がbool型ならよいのですが、たとえばnumber型かつ0が格納されている際、式の評価としては左辺の0が返されてしまい、ブラウザ上に「0」とだけ表示されたり、React Nativeでは最悪の場合クラッシュを引き起こします。

対策としてはシンプルで、かならず!!を先頭に付与すると良いです。

{!!someVariable && <Component />}

これで値が実行時にbool型になるため問題ありませんね。
また、NULLが式評価の結果になっても構わないので、以下のように実装してもいいです。

{someVariable ? <Component /> : null}

続いて考えるべきことは、これを仕組みに乗せて、誰もが同じミスをしないようにすることです。フロントエンドにおいてはESLintを使うことが適切でしょう。
本記事では該当の懸念を解消するための、ESLintルールを二つ紹介し、それぞれのメリットデメリット適用するべき場面を解説していきます。

react/jsx-no-leaked-renderを試してみる

react/jsx-no-leaked-render は皆さん御用達eslint-plugin-reactのルールの1つです。これを使って良ければ話は早いですね。

https://github.com/jsx-eslint/eslint-plugin-react/blob/master/docs/rules/jsx-no-leaked-render.md

このルールはauto fixにも対応しているため、さっそく個人開発しているReact製Webサービスにて、react/jsx-no-leaked-render を設定してfixしたところ、以下のようなDiffになりました。

           </div>
-          {passwordErrorMessage && <span className="text-red-500 mt-2 text-xs">{passwordErrorMessage}</span>}
+          {!!passwordErrorMessage && <span className="text-red-500 mt-2 text-xs">{passwordErrorMessage}</span>}
         </div>

passwordErrorMessageはString型なので本来0やNaNが入るリスクは小さいですが、このように確実にランタイムでbool型に変換する!!が挿入されます。

余談ですが空文字も一定のリスク（React18以前のReact Native、およびWebでもXPathの変更のリスク。詳しくは【余談】の章で解説）があるため、String型としか言い切れない以上は、Bool型にするのが望ましいです。

一方で、若干作りが甘い部分もあるように感じました。以下のように、Propsの内側に通常の値を表示するための三項演算子があった際、Invalid扱いになることがあります。本当はElementの表示自体を分岐しているところだけ怒ってほしいので、オーバーキルですね（※validStrategiesにcoerceのみを指定した場合）。

// ESLint: Potential leaked value that might cause unintentionally rendered values or rendering crashes(react/jsx-no-leaked-render)
<ToolbarToggleGroup type="single" value={pressed ? 'single' : undefined}>

本プラグインの実装を見る限り、AST Nodeによる指定でConditionalExpressionが指定されているので、このようにElement内の関係ない式が検知されてしまっても仕方がないように見えました。

どっちかというと厳し目にエラーレポートする思想で実装されているようです。

次に紹介するプラグインでは、TSの型情報を使うため、誤検知がないことが期待されます。

eslint-plugin-jsx-expressionsを試してみる

eslint-plugin-jsx-expressionsは独立したESLintルールのライブラリです。

https://github.com/hluisson/eslint-plugin-jsx-expressions

READMEを読むと分かる通り、「0が入りうるようなNumber型」のときだけ「!!」を必須化するなど、なんでもかんでもBoolへのキャストを必須化するのではなく、本件の問題となりうるときのみESLintのエラーとしてレポートします。

テストコードを見てみると以下のように、初期値が100でも、let で宣言されていればInvalidとするようにテストケースが実装されています。

    {
      code: ["let num = 100;", "<App>{num && <Foo/>}</App>"].join("\n"),
      errors: [{ messageId: "conditionErrorFalseyNumber" }],
      parserOptions: {
        ecmaFeatures: {
          jsx: true,
        },
      },
      output: ["let num = 100;", "<App>{!!num && <Foo/>}</App>"].join("\n"),
      filename: "react.tsx",
    },

実装されているソースコードを見ても、TypeScriptの型判定用の関数を使ってNodeに対して型を取得して、Numberのリテラルではないか、0の固定値で宣言されていることをチェックしています。

      const hasPotentiallyFalseyNumber = types.some(
        (type) =>
          tsutils.isTypeFlagSet(
            type,
            ts.TypeFlags.NumberLike | ts.TypeFlags.BigIntLike
          ) &&
          (!type.isNumberLiteral() || type.value === 0)
      );

実際に弊社のコードで試してみると、.length > 0ではなく.lengthのみになっているときだけ警告してくれて、当初の目的を達成しています。

// ESLint: Potentially falsey number in logical AND expression. Please use boolean.(jsx-expressions/strict-logical-expressions)
            <div>
              {course.tags.length && (
                <div>

もちろん欠点としては、TSの型情報を信用していることから、ネットワーク越しにやってきたデータの型をランタイムで検査せず信用している場合などは意図せず実行時エラーを引き起こす可能性があり、本ルールのみでリスクを完全にケアできているとはいい難いでしょう。

一方で、zodのparseを使うなどしてランタイムで型安全をある程度保証しているプロジェクトであれば、本ルールを導入することで、最小限の手間でより安全に分岐が実装できそうですし、ランタイムの型安全にこだわっていなくても実際問題信用できるよね、で他のいろいろな判断が通っているようなProject（弊社も現在はこれです）であれば、本ルールのほうが余計なBoolキャストを入れなくていいし、前節で説明したような誤検知も少ないと思われます。

@typescript-eslint/strict-boolean-expressions

そもそもJSXに限らず、Boolean型でない変数を安易にA && Bのように書くべきではない、という見解もあると思います。

@typescript-eslint/strict-boolean-expressionsはtypescript-eslintに属するルールの1つであり、たとえば以下のようなコードをエラー扱いにします。

// nullable strings are considered unsafe by default
let str: string | null = null;
if (!str) {
  console.log('str is empty');
}

上記のように、!strだけでは、空文字の場合なのか、nullの場合なのか、どっちもなのか意図が読み取れないので、安全ではない可能性が高いとしてエラーにしてくれます。
以下のように修正することでエラーが出なくなります。

let str: string | null = null;
if (str != null && !str) {
  console.log('str is empty');
}

本記事の主題は、JSXに0を表示させないことですが、デフォルトで本プラグインはallowNumberがTrueになっており、単なるNumber型はそのまま分岐に使っていいことになっています。したがって、以下のようにallowNumberをFalseにしましょう。

    '@typescript-eslint/strict-boolean-expressions': ['error', { allowNumber: false }],

上記設定により、以下のような分岐がエラーになります。

            // ESLint: Unexpected number value in conditional. An explicit zero/NaN check is required.(@typescript-eslint/strict-boolean-expressions)
            choiceItems.length ? (
              <ChoiceItems choiceItems={choiceItems} correct={correct} onChange={handleInput} />
            ) : (
              <Input onChange={handleInput} />
            )

また、array.lengthに対してはautofixが機能するので、自動で> 0をつけてくれます。
ですので、本来の目的である0表示のリスクを防ぐことができそうです。

ただし、本ルールの欠点というか特性として、JSXに限らずTS全般に効いてくるルールなので、以下のように普通のTSコードでもエラーがレポートされます。

  const hash = url?.hash;
  // ESLint: Unexpected nullable string value in conditional. Please handle the nullish/empty cases explicitly.(@typescript-eslint/strict-boolean-expressions)
  if (!query && !hash) return '';

個人的にはかなり型安全性が高まるルールだし、ある程度autofix/suggestも効くことから、プロジェクト立ち上げ時には入れておくといいし、途中からでもあまり対応がいらずautofixで済みそうなら加えても良いのではないかと思いました。
ですが、一方で、本記事の主題であるJSXの懸念を解消する観点からは、too muchすぎるという見方もできます。なので、今このルールを入れていなくて、取り急ぎJSXの懸念だけでも解消したいときは、前節までで紹介したルールのほうが適切だと思います。

具体的にtoo muchな例として、@typescript-eslint/strict-boolean-expressionsは条件式への利用をとにかく制限するため、冒頭で解決した3項演算子による解決策をやっていてもエラー扱いになるのです。

// (オプションによるが)someVariableが純粋なboolean型ではない場合、エラーになる
{someVariable ? <Component /> : null}

本ルールは若干論点がずれているんですよね。JSX上で安全に条件分岐を実装したい、という課題に向かっているのが前節までのルールで、そもそも分岐自体にバグの可能性を潜ませないようにしよう、が本ルールです。
目的が違うことによる動作差異の例として、細かいのですがeslint-plugin-jsx-expressionsはNumber型でもリテラル型かつ0以外だったらエラーレポートの対象にしないという特長があります。@typescript-eslint/strict-boolean-expressionsはNumber型だったらとにかくエラーなので、以下のようなコードでもエラーになります。

const x = 100;
// ESLint: Unexpected number value in conditional. An explicit zero/NaN check is required.(@typescript-eslint/strict-boolean-expressions)
x && (
  <ChoiceItems choiceItems={choiceItems} correct={getCorrectFromElement(element)} onChange={handleInput} />
)

（これ、@typescript-eslint/strict-boolean-expressions側からしても意図しない挙動なのでは...と思って実装まで追ったのですが、どうもallowNumber = true時の挙動として意図的っぽいようにも見えて、このまま沼りそうなので一旦置きます）

以上から、個人的な結論は「@typescript-eslint/strict-boolean-expressionsは安全性を高める素晴らしいルールだが、JSXの懸念を解消するにはallowNumberをfalseにする必要がある。JSX以外のソースコードにも影響する点や、const値への間違ったエラー報告の懸念もあるので、実際はallowNumberをTrueにしたままJSX専用ルールと併用するのがベストかも」です。

もしこの記事を読んだ方に似たような悩みに直面したことがある方、どうやっているかコメントで教えて下さい🙏

まとめ

以下のように判断し、適切なルールを使うと良いと考えます。

  • TS環境以外で実装しているならreact/jsx-no-leaked-render
  • TSの型を信頼できる・信頼していい環境ならeslint-plugin-jsx-expressions
  • 別軸で@typescript-eslint/strict-boolean-expressionsは一般的におすすめできるルールだけど、エッジケースへの対応を考えると上記ルールの併用も一考の余地あり

また、次節を見ていただけるとわかりますが、実際はno-restricted-syntaxを使うなどして自分でASTを定義して戦うような案もありますので、自チームのフロントエンド力みたいなところとも兼ね合いで決めると良いかなと思います。

【余談】本家eslint pluginで議論されたissueを追う

本記事で紹介した2つのルールは、以下のIssueの議論の末に誕生したようです。

https://github.com/jsx-eslint/eslint-plugin-react/issues/2073

以下、誰が読んで嬉しいコンテンツかわかりませんが、Issueのやり取りを簡単な和訳で追っていきます。ちゃんと英文を読みたい方は上記URLを開いて読んでください。けっこう面白いです。

（2018年末）

  • 以下のガイドに則って真偽値による分岐を実装するけど、よく不等号を忘れて0がレンダリングされるんだ。可能なら解決できない？
  • それは厄介だね、なぜなら数値以外のあらゆるFalsyな値は正常に動作するから
  • number型の変数に.lengthがついているときだけError扱いにするルールがこちらのPRで実装されたが、有用性に欠けるのでボツにした
    • 筆者注）正確にはASTしか見ていなかったので任意の変数に.lengthがついているときReportするという簡易的な実装になっていました
  • そもそも.lengthだけじゃなくてSet.prototype.sizeも対象じゃない？
  • いやいや、ていうか.lengthが数値型であるって保証はどこにもないでしょ。適当に自分でクラスを作って.lengthプロパティが文字列型になるように実装だってできるわけだし
    • 筆者注）きな臭くなってきた。この時点でもうTypeScriptの型見ようよっていう結論が透けて見える
  • こんなESLintルールが見つかったよ。これはlengthの使い方をチェックしてくれるようだ

（2019年末）

  • 私はここ数ヶ月で0を表示するミスを何度も見た！これはESLintルールに加えるべき案件だ

（2020年末）

  • そうだそうだ、ReactのESLintルールに加えよう
    • 筆者注）OSSのIssueあるある。カジュアルにめっちゃ時間経過する
  • こんにちは。私はReact Nativeの開発者です。React Nativeにおいてはこのエラーはより深刻です。なぜなら<Text>タグ以外の場所でテキストをレンダリングするとアプリがクラッシュするからです
  • React Nativeに限って言うならさ、普通React NativeプロジェクトってTypeScriptプロジェクトなんだから、TypeScript Compilerから情報を取得したらいいんじゃない？
    • 筆者注）きたきた
  • @typescript-eslintにルールがあるよ！
  • ところで、普段から3項演算子を使ったらいいし、そもそもデータがなくてコンポーネントをトルツメするわけじゃなくて、「No data to show」みたいに表示しない？まあどうしても必要なら上のようなルールを使えばいいけど
    • 筆者注）ひぇっ。あなたのUIUX論お呼びじゃないよパターンのコメントだ・・・
  • 素敵なUXかどうかなんて状況次第でしょ
    • 筆者注）ひぇ。そのとおり

（2021年初頭）

  • ところで僕の最近の学びをシェアするよ。0だけじゃなくて空文字のレンダリングも防いだほうがよさそうだ。空文字の表示はユーザーには差異がないように見えるけど、Xpathが壊れる。なのでブラウザテストに影響することがあるんだ
    • コメントはこちら
    • 筆者注）僕このコメント大好き。確かにすぎる
  • XPathに依存したテストは脆いよ
    • 筆者注）はい、そうですね（まあこのコメントを読んだ人がXpathをテストに使うことを正当化すると良くないっちゃ良くないから良いコメントではあるか）
  • no-restricted-syntaxっていうルールがあるよ。これでJSX内で3項演算子を使うことを強制できる
    • ESLint公式にあるルールで、AST上のセレクタを直接指定できる
    • 恥ずかしながら知らなかったけど、すでにZennで記事化されている程度には一般化した概念なんですね。いやー、メンテナンスできる人どれくらいいるねん問題があるけど、個人的には好き
    • 筆者注）3項演算子のことをternaryって言うんですね

（2021年夏）

  • 3項演算子を使う話で盛り上がっているけど、もともとの話、&&を使っていて0などをレンダリングする恐れのあるコードを検知できるできないの話じゃなかった？
  • その通り。.lengthに限らず、0がレンダリングされる可能性のあるコードを見つけることだよね。型情報が必要だ
  • まあ、型情報を使った実装はTypeScriptもReactも必要で問題が多いね。no-restricted-syntaxを使った完全な例をここに貼っておくよ
  • ここまでの話を統合すると、ReactとReact Nativeで困っていることも若干違うし、モードによって変えられるようなルールを誰か作ってくれないかなぁ

（2021年10月）

（この後）

  • なんか細かいところで、TS使うべきじゃない派？と思われる人のコメントがしばらく続き、結果として、React ESLint側にもルールが追加されたし、TSの型情報を参照するルールも追加されました。

また、jsx-no-leaked-renderルールを追加するPull Requestが以下URLにあり、ここに並んでいるコメントも勉強になります。React18以降で空文字に対してReact Nativeアプリがクラッシュしなくなったのでは、という仮説に対する討論もされています。
https://github.com/jsx-eslint/eslint-plugin-react/pull/3203

参考文献

マナリンク Tech Blog
マナリンク Tech BlogPublication

オンライン家庭教師マナリンクを運営するスタートアップNoSchoolのテックブログです。 manalink.jp/ 創業以来年次200％前後で売上成長しつつ、技術面・組織面での課題に日々向き合っています。 カジュアル面談はこちら！ forms.gle/fGAk3vDqKv4Dg2MN7

Discussion