FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

esm: top-level Wasm without package type by guybedford · Pull Request #57610 · nodejs/node · GitHub

/ node Public

esm: top-level Wasm without package type - #57610

Closed
guybedford wants to merge 1 commit into
nodejs:mainfrom
guybedford:toplevel-wasm
ConversationCommits (1)ChecksFiles changed
Closed

esm: top-level Wasm without package type#57610
guybedford wants to merge 1 commit into
nodejs:mainfrom
guybedford:toplevel-wasm

Conversation

guybedford commented Mar 25, 2025

Copy link
Copy Markdown
Contributor

We currently support node x.wasm through the ESM integration where .wasm files are located inside packages with "type": "module", but not without the module type on the package. This fixes the top-level execution case to always use the ESM loader.

This fix also adds a comprehensive test case for top-level Wasm execution that in turn can be extended to various end-to-end Wasm execution scenarios, in this example creating a JS object, setting some properties on it, and logging it back.

nodejs-github-bot commented Mar 25, 2025

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/loaders

nodejs-github-bot added module Issues and PRs related to the module subsystem. needs-ci PRs that need a full CI run. labels Mar 25, 2025
ljharb approved these changes Mar 25, 2025
View reviewed changes

ljharb left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

wasm should definitely work regardless of the presence or value of the type setting 🚀

codecov Bot commented Mar 25, 2025 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.24%. Comparing base (1c2d98d) to head (98d440a).
Report is 299 commits behind head on main.

Additional details and impacted files 
@@            Coverage Diff             @@
##             main   #57610      +/-   ##
==========================================
+ Coverage   90.22%   90.24%   +0.01%     
==========================================
  Files         630      630              
  Lines      185073   185074       +1     
  Branches    36222    36226       +4     
==========================================
+ Hits       166990   167017      +27     
+ Misses      11044    11031      -13     
+ Partials     7039     7026      -13
Files with missing lines Coverage Δ
lib/internal/modules/run_main.js 97.61% <100.00%> (+0.01%) ⬆️

... and 25 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

GeoffreyBooth commented Mar 25, 2025

Copy link
Copy Markdown
Member

Is there any documentation that needs updating?

GeoffreyBooth added the notable-change PRs with changes that should be highlighted in changelogs. label Mar 25, 2025

github-actions Bot commented Mar 25, 2025

Copy link
Copy Markdown
Contributor

The notable-change PRs with changes that should be highlighted in changelogs. label has been added by @GeoffreyBooth.

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.

ljharb added the semver-minor PRs that contain new features and should be released in the next minor version. label Mar 25, 2025

guybedford commented Mar 25, 2025

Copy link
Copy Markdown
Contributor Author

According to the existing ESM_FILE_FORMAT spec, the package.json type shouldn't affect the .wasm Wasm interpretation, so I don't think we need to make any changes to the docs as this is what's already described.

ExE-Boss reviewed Mar 25, 2025
View reviewed changes
Comment thread test/fixtures/es-modules/top-level-wasm.wasm Outdated
mcollina approved these changes Mar 25, 2025
View reviewed changes

mcollina left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

lgtm

richardlau commented Mar 25, 2025

Copy link
Copy Markdown
Member

Is there any documentation that needs updating?

Maybe https://nodejs.org/docs/latest/api/cli.html#program-entry-point?

guybedford commented Mar 25, 2025

Copy link
Copy Markdown
Contributor Author

Maybe https://nodejs.org/docs/latest/api/cli.html#program-entry-point?

Good point, I've updated that section to match the current implementation.

ljharb reviewed Mar 25, 2025
View reviewed changes
Comment thread doc/api/cli.md Outdated

This comment was marked as outdated.

Sign in to view

This comment was marked as outdated.

Sign in to view

nodejs-github-bot commented Apr 3, 2025

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/65992/

guybedford added a commit that referenced this pull request Apr 4, 2025
esm: support top-level Wasm without package type
870dec2 
PR-URL: #57610
Reviewed-By: Jordan Harband <ljharb@gmail.com>
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>

guybedford commented Apr 4, 2025

Copy link
Copy Markdown
Contributor Author

Landed in 870dec2.

guybedford closed this Apr 4, 2025
RafaelGSS pushed a commit that referenced this pull request May 1, 2025
esm: support top-level Wasm without package type
cbf9a64 
PR-URL: #57610
Reviewed-By: Jordan Harband <ljharb@gmail.com>
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
RafaelGSS mentioned this pull request May 1, 2025
Merged
RafaelGSS pushed a commit that referenced this pull request May 2, 2025
esm: support top-level Wasm without package type
21f3c96 
PR-URL: #57610
Reviewed-By: Jordan Harband <ljharb@gmail.com>
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
aduh95 added baking-for-lts PRs that need to wait before landing in a LTS release. and removed baking-for-lts PRs that need to wait before landing in a LTS release. labels May 6, 2025
aduh95 pushed a commit that referenced this pull request May 6, 2025
esm: support top-level Wasm without package type
c54bc00 
PR-URL: #57610
Reviewed-By: Jordan Harband <ljharb@gmail.com>
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
RafaelGSS pushed a commit that referenced this pull request May 14, 2025
esm: support top-level Wasm without package type
d72e510 
PR-URL: #57610
Reviewed-By: Jordan Harband <ljharb@gmail.com>
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
aduh95 pushed a commit that referenced this pull request May 17, 2025
esm: support top-level Wasm without package type
295093b 
PR-URL: #57610
Reviewed-By: Jordan Harband <ljharb@gmail.com>
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
github-actions Bot mentioned this pull request May 19, 2025
Merged
aduh95 pushed a commit that referenced this pull request May 19, 2025
esm: support top-level Wasm without package type
832640c 
PR-URL: #57610
Reviewed-By: Jordan Harband <ljharb@gmail.com>
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
aduh95 added a commit that referenced this pull request May 19, 2025
2025-05-21, Version 22.16.0 'Jod' (LTS)
9774069 
Notable changes:

deps:
  * update timezone to 2025b (Node.js GitHub Bot) #57857
doc:
  * add dario-piotrowicz to collaborators (Dario Piotrowicz) #58102
  * (SEMVER-MINOR) graduate multiple experimental apis (James M Snell) #57765
esm:
  * (SEMVER-MINOR) graduate import.meta properties (James M Snell) #58011
  * (SEMVER-MINOR) support top-level Wasm without package type (Guy Bedford) #57610
sqlite:
  * (SEMVER-MINOR) add StatementSync.prototype.columns() (Colin Ihrig) #57490
src:
  * (SEMVER-MINOR) set default config as `node.config.json` (Marco Ippolito) #57171
  * (SEMVER-MINOR) create `THROW_ERR_OPTIONS_BEFORE_BOOTSTRAPPING` (Marco Ippolito) #57016
  * (SEMVER-MINOR) add config file support (Marco Ippolito) #57016
  * (SEMVER-MINOR) add ExecutionAsyncId getter for any Context (Attila Szegedi) #57820
stream:
  * (SEMVER-MINOR) preserve AsyncLocalStorage context in finished() (Gürgün Dayıoğlu) #57865
util:
  * (SEMVER-MINOR) add `types.isFloat16Array()` (Livia Medeiros) #57879
worker:
  * (SEMVER-MINOR) add worker.getHeapStatistics() (Matteo Collina) #57888

PR-URL: #58388
aduh95 added a commit that referenced this pull request May 21, 2025
2025-05-21, Version 22.16.0 'Jod' (LTS)
0bd9680 
Notable changes:

deps:
  * update timezone to 2025b (Node.js GitHub Bot) #57857
doc:
  * add dario-piotrowicz to collaborators (Dario Piotrowicz) #58102
  * (SEMVER-MINOR) graduate multiple experimental apis (James M Snell) #57765
esm:
  * (SEMVER-MINOR) graduate import.meta properties (James M Snell) #58011
  * (SEMVER-MINOR) support top-level Wasm without package type (Guy Bedford) #57610
sqlite:
  * (SEMVER-MINOR) add StatementSync.prototype.columns() (Colin Ihrig) #57490
src:
  * (SEMVER-MINOR) set default config as `node.config.json` (Marco Ippolito) #57171
  * (SEMVER-MINOR) create `THROW_ERR_OPTIONS_BEFORE_BOOTSTRAPPING` (Marco Ippolito) #57016
  * (SEMVER-MINOR) add config file support (Marco Ippolito) #57016
  * (SEMVER-MINOR) add ExecutionAsyncId getter for any Context (Attila Szegedi) #57820
stream:
  * (SEMVER-MINOR) preserve AsyncLocalStorage context in finished() (Gürgün Dayıoğlu) #57865
util:
  * (SEMVER-MINOR) add `types.isFloat16Array()` (Livia Medeiros) #57879
worker:
  * (SEMVER-MINOR) add worker.getHeapStatistics() (Matteo Collina) #57888

PR-URL: #58388
ghost mentioned this pull request Jun 8, 2025
Closed
guybedford mentioned this pull request Jul 19, 2025
Closed
AlCalzone mentioned this pull request May 13, 2026
Closed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

ljharb ljharb approved these changes
marco-ippolito marco-ippolito left review comments
mcollina mcollina approved these changes
GeoffreyBooth GeoffreyBooth approved these changes
aduh95 aduh95 approved these changes
+1 more reviewer
ExE-Boss ExE-Boss left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

module Issues and PRs related to the module subsystem. needs-ci PRs that need a full CI run. notable-change PRs with changes that should be highlighted in changelogs. semver-minor PRs that contain new features and should be released in the next minor version.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

9 participants

Back | FazBrowse Home | New Git URL