-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
swのesbuildの更新とビルドスクリプトの更新 #10549
swのesbuildの更新とビルドスクリプトの更新 #10549
Conversation
`define`では文字列を渡さなければならないので、`JSON.stringify`をするようにした。
…る問題を修正 コメントの文言を調整
Codecov Report
@@ Coverage Diff @@
## develop #10549 +/- ##
===========================================
+ Coverage 74.16% 75.11% +0.95%
===========================================
Files 726 886 +160
Lines 67675 87097 +19422
Branches 5634 5907 +273
===========================================
+ Hits 50188 65420 +15232
- Misses 17487 21677 +4190 see 161 files with indirect coverage changes Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
}); | ||
}; | ||
|
||
(async () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IIFEである必要あるかしら?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
top-level await使えない?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Node.jsに直接実行させるファイルなので使えません。一応、package.json
で"type": "module"
した上で、require()
をやめたり__dirname
を手動で求めたりすることで、top-level awaitな書き方ができるようにはなります。(試しました。必要であればコミットします。)
ただ個人的には、/scripts/dev.js
というasyncなIIFEを使っている前例があることなどを考えると、そこまでしてこのファイルでだけtop-level awaitができるようにする旨味がないようにも感じます。
🙏🏻 🙏🏻 |
* cleanup(sw/build.js) * fix(sw/build.js): `define`に真偽値を渡していた問題を修正 `define`では文字列を渡さなければならないので、`JSON.stringify`をするようにした。 * fix(sw/build.js): `string`が期待される`define`において`undefined`になる場合がある問題を修正 * update(sw): esbuild 0.17.15 * fixup! update(sw): esbuild 0.17.15 * fixup! fix(sw/build.js): `string`が期待される`define`において`undefined`になる場合がある問題を修正 コメントの文言を調整
* cleanup(sw/build.js) * fix(sw/build.js): `define`に真偽値を渡していた問題を修正 `define`では文字列を渡さなければならないので、`JSON.stringify`をするようにした。 * fix(sw/build.js): `string`が期待される`define`において`undefined`になる場合がある問題を修正 * update(sw): esbuild 0.17.15 * fixup! update(sw): esbuild 0.17.15 * fixup! fix(sw/build.js): `string`が期待される`define`において`undefined`になる場合がある問題を修正 コメントの文言を調整
What
このPRは #10525 を解決することを目的としたものです。swパッケージで使われているesbuildと、それを使うビルドスクリプト(
build.js
)を更新することで、satisfies
のような新しい構文を含むソースコードをビルドできるようにしました。このとき、ビルド用スクリプト(
build.js
)の整理もしました。差分がいろいろと出てはいますが、やったことはプロパティを並べ替えた程度ですのでご確認ください。明示的に指定する必要がなく省略できそうなプロパティもいくつかあったのですが、明示的に指定しているからといって何かよくないことが起きているわけでもないため触れないでおきました。Why
現在swパッケージで使用しているesbuildが古いため
satisfies
のような一部の構文が認識されず、ソースコードにそれが含まれていたときビルドに失敗する状況になってしまっています。TypeScript自体は新しいため、型チェックなどではその問題に気付くことができません。Additional info (optional)
#10525 ではesbuildからswcへ移行してもよいのではないかという話もありましたが、swcのBundling機能は「still under construction」とのことでしたので、とりあえずesbuildの更新による解決を試してみました。結果としてうまく更新できたと思われるため、こうしてPRを作成しました。この機会にswcへ移行するのであれば、このPRはマージせず閉じてしまってください。
Checklist