-
-
Notifications
You must be signed in to change notification settings - Fork 32.3k
esm: unflag --experimental-wasm-modules #57038
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
Conversation
Review requested:
|
//cc @nodejs/loaders |
So I assume its semver major? |
Strictly speaking, this isn't semver major since it has no breaking semantics. But, we only have fully V8 support on 24 unless there is a backport of the V8 update. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #57038 +/- ##
==========================================
+ Coverage 90.03% 90.05% +0.02%
==========================================
Files 648 648
Lines 190967 190978 +11
Branches 37425 37433 +8
==========================================
+ Hits 171931 171987 +56
+ Misses 11665 11607 -58
- Partials 7371 7384 +13
🚀 New features to boost your workflow:
|
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.
lgtm
The
notable-change
Please suggest a text for the release notes if you'd like to include a more detailed summary, then proceed to update the PR description with the text or a link to the notable change suggested text comment. Otherwise, the commit will be placed in the Other Notable Changes section. |
Thanks for all the approvals it is great to see the interest in this. After further discussion, since this is still a Phase 3 Wasm specification, that implies it is still in experimental implementation status until it reaches Phase 4. While strictly speaking we can land experimental features unflagged in Node.js, the ESM Integraiton has already taken long enough that there is no need to rush something out the door here - so I will hold off on landing this for Node.js 24 and instead seek to land it for (hopefully) 25 instead where ideally we would be able to land under Phase 4 later in the year, or to then at least reconsider unflagging under Phase 3 if we feel it is in the interests of the project at that point for our users. |
We discussed at the collab summit and we realized its not semver major so it can land |
Thanks for clarifying the case here, I'll aim to land this then when the current design discussions on the ESM Integration proposal repo have been resolved (theres a few final PRs in progress and under discussion right now - https://github.com/WebAssembly/esm-integration/pulls). Hopefully these should resolve fairly soon, but it may be more than a few weeks yet too. |
There is some relevant unflagging discussion in #57525 (comment). I think the important thing for now is that given this is non-breaking that means we are able to backport the unflagging. But we should not rush unflagging and try to reconcile with Chromium's shipping process here first as much as possible. |
b4b432c
to
8056814
Compare
I've made some final adjustments to this unflagging:
This seems to me to strike the correct balance between communicating the correct experimental status and adoption. |
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.
LGTM with one minor comment.
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.
Nice!
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
Co-authored-by: Chengzhong Wu <legendecas@gmail.com>
PR-URL: #57038 Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Stephen Belanger <admin@stephenbelanger.com> Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com> Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Landed in 0df1518. |
PR-URL: nodejs#57038 Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Stephen Belanger <admin@stephenbelanger.com> Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com> Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
PR-URL: nodejs#57038 Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Stephen Belanger <admin@stephenbelanger.com> Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com> Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
PR-URL: nodejs#57038 Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Stephen Belanger <admin@stephenbelanger.com> Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com> Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
PR-URL: nodejs#57038 Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Stephen Belanger <admin@stephenbelanger.com> Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com> Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
PR-URL: #57038 Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Stephen Belanger <admin@stephenbelanger.com> Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com> Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Notable changes: cli: * (SEMVER-MINOR) support `${pid}` placeholder in --cpu-prof-name (Haram Jeong) #59072 crypto: * (SEMVER-MINOR) add tls.setDefaultCACertificates() (Joyee Cheung) #58822 dns: * (SEMVER-MINOR) support max timeout (theanarkh) #58440 doc: * update the instruction on how to verify releases (Antoine du Hamel) #59113 esm: * (SEMVER-MINOR) unflag --experimental-wasm-modules (Guy Bedford) #57038 http,https: * (SEMVER-MINOR) add built-in proxy support in http/https.request and Agent (Joyee Cheung) #58980 net: * (SEMVER-MINOR) update net.blocklist to allow file save and file management (alphaleadership) #58087 test: * (SEMVER-MINOR) move http proxy tests to test/client-proxy (Joyee Cheung) #58980 worker: * (SEMVER-MINOR) add web locks api (ishabi) #58666 PR-URL: #59257
PR-URL: nodejs#57038 Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Stephen Belanger <admin@stephenbelanger.com> Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com> Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Notable changes: cli: * (SEMVER-MINOR) support `${pid}` placeholder in `--cpu-prof-name` (Haram Jeong) #59072 crypto: * (SEMVER-MINOR) add `tls.setDefaultCACertificates()` (Joyee Cheung) #58822 deps: * upgrade to openssl-3.5.1 (Node.js GitHub Bot) #59234 dns: * (SEMVER-MINOR) support max timeout (theanarkh) #58440 doc: * update the instruction on how to verify releases (Antoine du Hamel) #59113 esm: * (SEMVER-MINOR) unflag `--experimental-wasm-modules` (Guy Bedford) #57038 http,https: * (SEMVER-MINOR) add built-in proxy support in http/https.request and `Agent` (Joyee Cheung) #58980 net: * (SEMVER-MINOR) update net.blocklist to allow file save and file management (alphaleadership) #58087 test: * (SEMVER-MINOR) move http proxy tests to test/client-proxy (Joyee Cheung) #58980 worker: * (SEMVER-MINOR) add web locks api (ishabi) #58666 PR-URL: #59257
Notable changes: cli: * (SEMVER-MINOR) support `${pid}` placeholder in `--cpu-prof-name` (Haram Jeong) #59072 crypto: * (SEMVER-MINOR) add `tls.setDefaultCACertificates()` (Joyee Cheung) #58822 deps: * upgrade to openssl-3.5.1 (Node.js GitHub Bot) #59234 dns: * (SEMVER-MINOR) support max timeout (theanarkh) #58440 doc: * update the instruction on how to verify releases (Antoine du Hamel) #59113 esm: * (SEMVER-MINOR) unflag `--experimental-wasm-modules` (Guy Bedford) #57038 http,https: * (SEMVER-MINOR) add built-in proxy support in http/https.request and `Agent` (Joyee Cheung) #58980 net: * (SEMVER-MINOR) update net.blocklist to allow file save and file management (alphaleadership) #58087 test: * (SEMVER-MINOR) move http proxy tests to test/client-proxy (Joyee Cheung) #58980 worker: * (SEMVER-MINOR) add web locks api (ishabi) #58666 PR-URL: #59257
Notable changes: cli: * (SEMVER-MINOR) support `${pid}` placeholder in `--cpu-prof-name` (Haram Jeong) #59072 crypto: * (SEMVER-MINOR) add `tls.setDefaultCACertificates()` (Joyee Cheung) #58822 deps: * upgrade to openssl-3.5.1 (Node.js GitHub Bot) #59234 dns: * (SEMVER-MINOR) support max timeout (theanarkh) #58440 doc: * update the instruction on how to verify releases (Antoine du Hamel) #59113 esm: * (SEMVER-MINOR) unflag `--experimental-wasm-modules` (Guy Bedford) #57038 http,https: * (SEMVER-MINOR) add built-in proxy support in http/https.request and `Agent` (Joyee Cheung) #58980 net: * (SEMVER-MINOR) update net.blocklist to allow file save and file management (alphaleadership) #58087 test: * (SEMVER-MINOR) move http proxy tests to test/client-proxy (Joyee Cheung) #58980 worker: * (SEMVER-MINOR) add web locks api (ishabi) #58666 PR-URL: #59257
Notable changes: cli: * (SEMVER-MINOR) support `${pid}` placeholder in `--cpu-prof-name` (Haram Jeong) #59072 crypto: * (SEMVER-MINOR) add `tls.setDefaultCACertificates()` (Joyee Cheung) #58822 deps: * upgrade to openssl-3.5.1 (Node.js GitHub Bot) #59234 dns: * (SEMVER-MINOR) support max timeout (theanarkh) #58440 doc: * update the instruction on how to verify releases (Antoine du Hamel) #59113 esm: * (SEMVER-MINOR) unflag `--experimental-wasm-modules` (Guy Bedford) #57038 http,https: * (SEMVER-MINOR) add built-in proxy support in http/https.request and `Agent` (Joyee Cheung) #58980 net: * (SEMVER-MINOR) update net.blocklist to allow file save and file management (alphaleadership) #58087 test: * (SEMVER-MINOR) move http proxy tests to test/client-proxy (Joyee Cheung) #58980 worker: * (SEMVER-MINOR) add web locks api (ishabi) #58666 PR-URL: #59257
Notable changes: cli: * (SEMVER-MINOR) support `${pid}` placeholder in `--cpu-prof-name` (Haram Jeong) #59072 crypto: * (SEMVER-MINOR) add `tls.setDefaultCACertificates()` (Joyee Cheung) #58822 deps: * upgrade to openssl-3.5.1 (Node.js GitHub Bot) #59234 dns: * (SEMVER-MINOR) support max timeout (theanarkh) #58440 doc: * update the instruction on how to verify releases (Antoine du Hamel) #59113 esm: * (SEMVER-MINOR) unflag `--experimental-wasm-modules` (Guy Bedford) #57038 http,https: * (SEMVER-MINOR) add built-in proxy support in http/https.request and `Agent` (Joyee Cheung) #58980 net: * (SEMVER-MINOR) update net.blocklist to allow file save and file management (alphaleadership) #58087 test: * (SEMVER-MINOR) move http proxy tests to test/client-proxy (Joyee Cheung) #58980 worker: * (SEMVER-MINOR) add web locks api (ishabi) #58666 PR-URL: #59257
This unflags
--experimental-wasm-modules
for Node.js 24, while keeping the implementation experimental under 2.1 active development.Source phase imports and string builtins as stable proposals are both promoted to 2.2 release candidate experimental status. The warning for importing Wasm modules is moved to only apply when importing module instances, rather than module sources as well.
With #56919 landed, Node.js supports both source phase imports and instance phase imports to WebAssembly modules and for Wasm imports to JS, in line with the current Phase 3 WebAssembly ESM Integration proposal (https://github.com/webassembly/esm-integration).
Recent additions to the source phase integration here also include:
By unflagging, this will enable build tools to target source phase imports for Node.js as an output format, providing immediate benefit to Wasm consumers in enabling more seamless interoperability between JS and Wasm modules.