Closed16

Prettier や Babel に関する作業ログ(575)

ピン留めされたアイテム
Sosuke SuzukiSosuke Suzuki

このスクラップについて

このスクラップは、私が Prettier や Babel で行った作業についてのログです。

私は Prettier のメンテナーで、Babel のチームメンバーです。Prettier での作業がメインになると思います。

基本的には自分で後から見返して振り返るために記録しています。

また JavaScript ツールチェイン系 OSS の開発に関心がある人が見て楽しむ or 参考にすることもできるようにするため、なるべくハイコンテキストな書き方は避けるようにしています。

自分のみの作業ログにしたいため、他のユーザーの投稿は許可していません。

Sosuke SuzukiSosuke Suzuki

typescript-eslint と Babel は両方とも「ESTree 互換の TypeScript の AST」を扱います。

それぞれのチームはできるだけ同じ AST の形になるように努めています。例えば、typescript-eslint には Babel の AST との互換性を検証するための AST Alignment Tests があります(https://github.com/typescript-eslint/typescript-eslint/tree/master/packages/typescript-estree/tests/ast-alignment)。

しかし、実際はぞれぞれの AST には若干差異があります。これは意図されたものではなく、実装上のミスで偶然そうなってしまっていることがほとんどです。(typescript-eslint の AST Alignment Tests ではそれらの差異を確認しつつテストを通すために明示的な AST transformation が実装されています)

Prettier のような Babel と typescript-eslint の AST を両方統一して扱いたいようなアプリケーションでは、差異があるのは不便です。なので、自分は気づいたときに Babel 側で修正しています。

ついこないだ、TSPredicateTypeassert プロパティが typescript-estree だと boolean 型だけど Babel だと boolean | undefined になっていたので、修正しました。

https://github.com/babel/babel/pull/12352

その修正を含む Babel 7.12.7 がリリースされたので、typescript-eslint のテストを修正する PR を作成しました。

https://github.com/typescript-eslint/typescript-eslint/pull/2792

こんなのは非常に些細な差で、あんまりうれしみもないですが、ツール間で AST が整っていると気持ち良いのでこれからも気づいたらやります。

Sosuke SuzukiSosuke Suzuki

先日 Prettier のバージョン 2.2 をリリースしました。

バージョン 2.2 に含まれている修正として、クラス式を長いメンバー式に代入したときによしなにカッコ付きで改行してくれるというものがあります(リリースブログ)。

つまり、こういう入力があったときに:

aaaaaaaa.bbbbbbbb.cccccccc.dddddddd.eeeeeeee.ffffffff1 = class extends aaaaaaaa.bbbbbbbb.cccccccc.dddddddd.eeeeeeee.ffffffff2 {
  method () {
    console.log("foo")
  }

変なところで折り返すのではなく、カッコをつけて適切に改行するようになりました:

aaaaaaaa.bbbbbbbb.cccccccc.dddddddd.eeeeeeee.ffffffff1 = class extends (
  aaaaaaaa.bbbbbbbb.cccccccc.dddddddd.eeeeeeee.ffffffff2
) {
  method () {
    console.log("foo");
  }
};

Pull Request でいうと https://github.com/prettier/prettier/pull/9341 になります。

最近の JavaScript を書いているとこういったコードを書くことはまずないと思いますが、例えば Google Closure Library の Namespaces を使っているときなどに書くことがあります。

この変更自体は良かったのですが、想定よりも多くのパターンに対して働いてしまうというバグ報告がありました。

https://github.com/prettier/prettier/issues/9740

次のような CommonJS のコードがあったときに:

module.exports = class A extends B {
  method () {
    console.log("foo");
  }
};

Prettier 2.2 はスーパークラスをカッコで囲んで改行していしまいます:

module.exports = class A extends (
  B
) {
  method() {
    console.log("foo");
  }
};

この挙動はバグなので、修正する必要があります。

https://github.com/prettier/prettier/pull/9741

この Pull Request では、AST の Node の形からカッコをつけ改行するかどうかを判断するのではなく、ifBreak という doc builder を使って文字列に変換されるタイミングで動的に判断するように修正しています。

まだマージ/リリースできていませんが、近いうちにパッチバージョンでリリースしようと思います。

Sosuke SuzukiSosuke Suzuki

Prettier 2.2.1 をリリースしました。

含まれる変更は、このスクラップ内で紹介した https://github.com/prettier/prettier/pull/9741 のみです。

チェンジログは https://github.com/prettier/prettier/blob/master/CHANGELOG.md#221 にあります。

パッチバージョンをあげるときは、マイナーバージョンをあげるときとは若干違う手順でリリースを行います。

マイナーバージョンをあげるときは、master ブランチに含まれている変更をすべて(破壊的変更を除く)リリースに含めますが、パッチバージョンをあげるときは、master に含まれる変更のうち前回のマイナーバージョンのリリースで発生したリグレッションに対する修正のみをリリースします。

なので、以下のような手順でリリースをします。

  1. master ブランチから patch-release ブランチを新しく生やす
  2. patch-release ブランチを、前回のリリース直後まで戻す
  3. 必要な変更のみを master ブランチから patch-releasegit cherry-pick する
  4. patch-release ブランチでリリーススクリプトを実行する
  5. リリース完了後、master ブランチに patch-release ブランチをマージする

手動で行う作業が多いので、マイナーバージョンをリリースするときより緊張します。

実はパッチリリースの手順はドキュメント化されていなくて、自分もどこかの Issue で前任者から聞いただけでした。なので自分が消える前にドキュメントに残さなければいけないんですが、めんどくさくてまだやれていません...。

Sosuke SuzukiSosuke Suzuki

これはマジでしょうもない Prettier の修正です。

foo("a", , "b");

こういう JavaScript のコードってインバリッドなので、多くの JavaScript パーサーはシンタックスエラーをスローします。

ただ、Babel はリカバリ可能なエラーとしてスローするので、Prettier のようなエラーリカバリを有効にしている環境ではパースができてしまいます。

しかし、今まではそれに気づいていなかったので、上のコードを Prettier でフォーマットすると予期せぬところでエラーが起きてしまうというバグがありました。

https://github.com/prettier/prettier/issues/9735

(TypeError: Cannot read property 'type' of null が起こる)

これは、関数呼び出しの引数リストにはnullが含まれることはないだろうという前提のもとで実装されていたゆえに起こってしまっていたバグです。

なので、null チェックをして(インバリッドではあるものの一応)フォーマットが成功するように修正する PR を出しました。

https://github.com/prettier/prettier/pull/9787

しばらくしたらマージします。

Sosuke SuzukiSosuke Suzuki

先日、@rollup/plugin-node-resolve の version 11 がリリースされました。

Prettier はこのプラグインに依存しているんですが、dependabot の更新 PR の CI がコケてたので、調査をしてみました。

結論から言うと、どうやら package.json"exports" フィールドのプロパティの値が配列になっていると、正しくバンドルできないようです。

つまり、次のような package.json があるときに "pkg/util" を import すると Unresolved dependencies になってしまいます。

{
  "name": "pkg",
  "exports": {
    ".": "./main.js",
    "./util": [
      {
        "import": "./util.mjs"
      },
      "./util.js"
    ]
  }
}

rollup/plugins リポジトリにスムーズに報告するために、再現リポジトリを作成しました。

https://github.com/sosukesuzuki-sandbox/rollup-plugin-node-resolve-11-bug-repro

README に記載しているように、バンドルされてほしいのに、バンドルされずに吐き出されてしまっています。

これをもとに、Issue を作成しました。

https://github.com/rollup/plugins/issues/670

これが直らないと、Prettier では新しいバージョンの @rollup/plugin-node-resolve を使うことができません。

最悪自分で PR を投げてもいいかなーとも思いますが、@rollup/plugin-node-resolve のソースコードは読んだことがないので、しばらく待ってみようと思います。

Sosuke SuzukiSosuke Suzuki

どうやら、Node.js にはこの "exports" フィールドの書き方についてのドキュメントが存在しないらしく、Rollup ではそれをサポートできていなかったようです。

webpack のドキュメントにはそれについての記述があり、Node.js もサポートしているので Rollup もサポートするべき、という結論になった模様です。

参考:
https://twitter.com/lukastaegert/status/1333999736005808128

Sosuke SuzukiSosuke Suzuki

Prettier の CLI を高速化する(個人的な)目論見があり、その下準備として CLI 周りの簡単なリファクタリングを行いました。

Prettier の CLI 機能は src/cli 下に置かれています。そしてそのコア機能のほとんどが src/cli/util.js に記述されていました。これは非常に良くなくて、最近でこそなれてきたので読めるようになってきましたが、最初の頃は非常に困惑したことを覚えています。

また、個人的には、実行の高速化のために Worker threads などを使うことをなんとなく考えているので、重い処理がファイルに分かれている方が都合が良いです。

なので、その二つの願いを叶えるために二つのことを行う PR を作成しました。

  • src/cli/util.jssrc/cli/core.js にリネームする。
  • src/cli/util.js に含まれる関数群を機能ごとに src/cli 下の他のファイルに分割する。

https://github.com/prettier/prettier/pull/9826

これがマージできたら、高速化に着手できるかなーどうかなーという感じです。どちらにしても、このリファクタリングはコードの可読性を高めるので良かったと思います。

Sosuke SuzukiSosuke Suzuki

しょうもない Prettier のバグ修正です。

次のような JavaScript のコードを考えます:

for (const p of ['fullName', 'organ', 'position', 'rank'])
  // @ts-expect-error
  form.setValue(`${prefix}.data.${p}`, response[p])

このコードを今の Prettier でフォーマットすると、次のように変形されます:

// @ts-expect-error
for (const p of ['fullName', 'organ', 'position', 'rank'])
  form.setValue(`${prefix}.data.${p}`, response[p])

@ts-expect-error という意味のあるコメントが for (...) の上に移動してしまっています。これでは、フォーマット前とフォーマット後で TypeScript コンパイラに与える意味が変わってしまいます。

このようなバグが https://github.com/prettier/prettier/issues/9812 にて報告されました。

これは明らかにバグなので修正する必要があります。

ということで修正のための Pull Request を作成しました。

https://github.com/prettier/prettier/pull/9829

for 文のボディが式文であった場合(ブロックステートメントではない場合)のコメントの処理方法を修正しています。

また、for 文かどうかを判定するユーティリティ関数 isForStatement を追加しています。ESTree の仕様では for(;;)for(x in y)for(x of y) はそれぞれ別のタイプのノードになっているのですが、それらを統一して判定するために追加しました。

Sosuke SuzukiSosuke Suzuki

(ここ数日はコードレビューとか、既存の PR の修正とかをやっていたのであんまり書くことがありませんでした。)

Prettier はブラウザ向けのビルドも出荷しているのですが、そのバンドルサイズを簡単に確認することができるとチューニングする際に便利です。

個人的にPrettier のブラウザ向けの部分のソースから lodash を剥がしたいなーと思っていて、その意義を説明するためにバンドルサイズを計測する必要が出てきましたが、良い方法が今の所ありませんでした(また、そもそも意義があるのかを知る必要がある。)

なので、ビルドスクリプト実行時にオプションでバンドルサイズを出力するように修正します。

https://github.com/prettier/prettier/pull/9900

Prettier のビルドはちょっと特殊です。基本的にすべてのファイルを Rollup でバンドルしていますが、一部のバンドルのみ webpack で行っています。また、Rollup も一度の Rollup の実行ですべてのバンドルを吐き出すわけではなくて、出荷するバンドル1つに対してそれぞれ1回ずつ Rollup を実行します(つまり、16 回くらい Rollup をぶん回している)。

ここに改良の余地があるのはそれはそうとして、今問題なのはサイズを普通に出力してくれるような Rollup/webpack プラグインを入れるだけでは想定どおりに出力されないということです。

なので、バンドラを実行するごとに fs.stat でサイズを取得し、pretty-bytes で整形して空気を読みつつターミナルのログに出す、みたいなことをします。

Prettier のブラウザ向けビルドは基本的には UMD としてバンドルしていますが、2.2 からは
ESM バンドルも出荷するようになったので、ESM バンドルが存在する場合はそのサイズも出力します。

Sosuke SuzukiSosuke Suzuki

Babel のバグ修正です。

Babel の AST には OptionalCallExpression というノードがあります。

foo?.()

こういうやつです。

OptionalCallExpressionoptional という boolean のプロパティが存在するべきです。

ですが、次のようなコードをパースすると、optional が存在しない OptionalCallExpression を作ることができてしまいます。

foo?.foo<T>()

そう、型引数をつけると OptionalCallExpressionoptional が消えます。

これはバグで、本当は optionalfalse がセットされているべきです。

なので、そういう風に修正します。

https://github.com/babel/babel/pull/12562

この PR がマージ・リリースされれば、typescript-eslint の AST との差がまた一つ消えることになります。

Sosuke SuzukiSosuke Suzuki

ビルドスクリプトのリファクタリングです。

fs モジュールの関数を使うときに util モジュールの promisify を使って Promise ベースに変換していましたが、Prettier では Node 10.13 以降でビルドできればいいので、require("fs").promises を使うようにしました。

https://github.com/prettier/prettier/pull/10025

Sosuke SuzukiSosuke Suzuki

Prettier のバグ修正?機能追加?です。

特に関数型プログラミングの文脈でよく使われるテクニックに、関数のカリー化というものがあります。

JavaScript でも以下のようにしてカリー化された関数を書くことができます。

function curried(foo, bar) {
  return function (foo, bar) {
    return function (foo, bar) {
      return function (foo, bar) {
        return function (foo, bar) {
          return function (foo, bar) {
            return foo;
          };
        };
      };
    };
  };
}

これをアロー関数で書き直してみると、以下のようになると思います。

const curried =
  (foo, bar) =>
  (foo, bar) =>
  (foo, bar) =>
  (foo, bar) =>
  (foo, bar) =>
  (foo, bar) =>
  foo;

function 宣言で書いていたときよりも、スッキリした印象になりました。

このカリー化されたアロー関数 curried を Prettier 2.2 でフォーマットすると、次のようになります。

const curried = (foo, bar) => (foo, bar) => (foo, bar) => (foo, bar) => (
  foo,
  bar
) => (foo, bar) => foo;

フォーマットの幅の上限(print-width)がいっぱいになったところで無理やり折り返されてしまっていて、見た目がきたなくなってしまいました。

先日実装を始めた PR では、この挙動を改善しています。

https://github.com/prettier/prettier/pull/9992

このようなprint-width を上回るような長さのカリー化された関数は滅多に書くものではないと思いますが、バグ報告も上がってきており、多少の需要があるようです。

https://github.com/prettier/prettier/issues/9985

もしこの挙動に悩んでいる方がいたら、マージ・リリースをお待ち下さい。

Sosuke SuzukiSosuke Suzuki

普通に Prettier や Babel のメンテはしてるんですが、スクラップを更新するのは飽きてしまったので閉じます

このスクラップは2021/04/29にクローズされました