🍽️

typescript-eslint v6のスタイル系ルールは主張が強い

2023/07/23に公開

少し前にtypescript-eslintのメジャーアップデートがリリースされました。

https://typescript-eslint.io/blog/announcing-typescript-eslint-v6/

公式で提供されるルールセットに新しくスタイル系のstylisticstylistic-type-checkedが追加されています。

上記記事で「とりあえずこうしとけ」的におすすめされているのでstylistic-type-checkedを有効にしてみましたが、標準的なルールセットとするには主張が強すぎるきらいがあります。

既存プロジェクトに適用した時に気になったものをいくつか取り上げてみます。

他の問題を引き起こす場合があるもの

prefer-optional-chain

https://typescript-eslint.io/rules/prefer-optional-chain/

optional chainが使えそうな場面で使用を強制してきますが、場合によっては既存のアプリケーションを壊すことがあります。

例えば、以下のようなReact Componentを定義する場合

interface Task {
    deadline: Dayjs | null;
}

export const Deadline = (maybeTask: Task | null) => {
    if (maybeTask === null || maybeTask.deadline === null) {
        return null;
    }
    return <p>{maybeTask.deadline.format('YYYY-MM-DD HH:mm')}</p>;
};

if文のところで「optional chain使おうぜ!」って怒られます。

サジェストされるコードをQuick Fixで適用すると、

if (maybeTask?.deadline === null) {

となり、最後の実際に値を取ってくるところが壊れます。

maybeTaskがnullを取り得るので、それはそう。どうもこのルールはnullとundefinedを明確に区別するコードにはマッチしなさそうです。

この挙動が望ましいとは考えにくいので公式にissueが上がってるかなとも思ったのですが、パッとは見付けられませんでした。と言うか、このルールにまつわるissueは結構たくさんあって、既知なのかを調べるのが少し手がかかりそう。

consistent-type-definitions

https://typescript-eslint.io/rules/consistent-type-definitions/

型定義をinterfaceかtypeのどちらかを使うかを統一するためのルールです。デフォルトだとinterfaceに寄せます。

コンセプトは「どちらでもいいものを統一しよう」だと思うんですが、interfaceとtypeは等価ではないので、このルールを徹底できない場合があります。

type Foo = Record<string, string>;
interface Bar {
  email: string;
}

const f = (foo: Foo) => foo;
export const ff = (bar: Bar) => f(bar);

具体例としては、ドメインモデルをログ出力とか通知系の関数で汎用的な型として受けたい場面などを想定したサンプルです。

これ怒られます。

Index signature for type 'string' is missing in type 'Bar'.ts(2345)

interfaceにはIndex signatureが使えないので、Barもtypeで宣言したいところですが「interfaceにしろっつってんだろ!」と言われます。

コーディングスタイルに関するもの

既存のコードを壊すことはなかったものの、個人的にはそのまま使わないかなと感じたものにも少し触れておきます。

prefer-nullish-coalescing

https://typescript-eslint.io/rules/prefer-nullish-coalescing/

??演算子を使える場面では利用させるルールです。

const f1 = (v: string | null): string => {
  return v === null ? "" : v;
};

みたいな場面ではreturn v ?? ""をサジェストしてきます。

vstring | null | undefinedな場合は警告してこないので既存の挙動を壊すことはなさそうですが、nullとundefinedは明確に区別しておきたい。

||を使っている部分も可能であれば??を優先的に使うように言ってきます。falsyな値が入ってきた場合に事故を防ぐためのようですが、ちょっと意味合いが変わってくるのでコードの意図を伝える上では必ずしも有効ではないように思います。

このルールは色々と複雑なオプションを受け付けますが、全てを理解して適切な設定をするのは相当な労力が必要になりそうです。

array-type

https://typescript-eslint.io/rules/array-type/

配列の宣言をT[]にするかArray<T>にするかを統一するためのルールです。デフォルトだとT[]に寄せる形になります。

個人的には(Foo | Bar)[]よりもArray<Foo | Bar>の方が読みやすいと考えています。

順に読んでいった場合に()が付いている意味が分かるのは最後の[]に来た時なので、先に配列であることを宣言してくれた方が私は理解しやすいです。

特に型名が長い場合などは途中に改行が入ることがあり、その場合はなおさらです。

オプションでarray-simpleを設定すれば

  • 単純な型が入る場合はT[]
  • 複雑な場合はArray<T>

を正解としてくれるようになります。この動作は好みには合っていますが、そこまで規定したいかどうかでいうと別にええんちゃうかな… という思いが頭をもたげてきます。

「とりあえず」で有効にするルールセットなのか

ここに挙げた以外にも「分かるっちゃ分かるが、それはどっちでええやろ」と思う場面がわりと多めです。

既存のプロジェクトに導入する場合は、ルールセットを有効にした上で引っかかるところを修正するなり、設定を調整する必要が少なからず出てくるでしょう。

新規プロジェクトだとどうなんでしょうね。デフォルトの設定だと、これまでの自分のコードの書き方を変更することを強いられる人も少なくないでしょう。そこを細かく調整して、チーム内で統一したルールを見出すだけのコストが妥当かは判断が分かれそうです。

いずれにせよ、これまでのrecommended系のように「とりあえず有効にしておく」には過度にopinionatedに感じられます。

個人的には個別の調整コストを上回るメリットを得るのは難しいと感じるので、実務で組み込むことには今のところ消極的です。

Discussion