Skip to content

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

Closed
guybedford wants to merge 1 commit into
nodejs:mainfrom
guybedford:toplevel-wasm
Closed

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

Conversation

@guybedford

@guybedfordguybedford 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

nodejs-github-bot commented Mar 25, 2025

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/loaders

@nodejs-github-botnodejs-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
ljharb approved these changes Mar 25, 2025
View reviewed changes

@ljharbljharb 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.

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

@codecov

codecovBot 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 linesCoverage Δ
lib/internal/modules/run_main.js97.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

GeoffreyBooth commented Mar 25, 2025

Copy link
Copy Markdown
Member

Is there any documentation that needs updating?

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

github-actionsBot commented Mar 25, 2025

Copy link
Copy Markdown
Contributor

The notable-changePRs 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.

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

guybedford commented Mar 25, 2025

Copy link
Copy Markdown
ContributorAuthor

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
ExE-Boss reviewed Mar 25, 2025
View reviewed changes
Comment threadtest/fixtures/es-modules/top-level-wasm.wasm Outdated
mcollina
mcollina approved these changes Mar 25, 2025
View reviewed changes

@mcollinamcollina 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.

lgtm

@richardlau

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

guybedford commented Mar 25, 2025

Copy link
Copy Markdown
ContributorAuthor

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
ljharb reviewed Mar 25, 2025
View reviewed changes
Comment threaddoc/api/cli.md Outdated
@nodejs-github-bot

This comment was marked as outdated.

Sign in to view
@nodejs-github-bot

This comment was marked as outdated.

Sign in to view
@nodejs-github-bot

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
@guybedford
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

guybedford commented Apr 4, 2025

Copy link
Copy Markdown
ContributorAuthor

Landed in 870dec2.

RafaelGSS pushed a commit that referenced this pull request May 1, 2025
@guybedford@RafaelGSS
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>
@RafaelGSSRafaelGSS mentioned this pull request May 1, 2025
Merged
RafaelGSS pushed a commit that referenced this pull request May 2, 2025
@guybedford@RafaelGSS
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>
@aduh95aduh95 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
@guybedford@aduh95
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
@guybedford@RafaelGSS
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
@guybedford@aduh95
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-actionsgithub-actionsBot mentioned this pull request May 19, 2025
Merged
aduh95 pushed a commit that referenced this pull request May 19, 2025
@guybedford@aduh95
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
@aduh95
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
@aduh95
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
@ghostghost mentioned this pull request Jun 8, 2025
Closed
@guybedfordguybedford mentioned this pull request Jul 19, 2025
Closed
@AlCalzoneAlCalzone mentioned this pull request May 13, 2026
Closed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ljharbljharbljharb approved these changes
@marco-ippolitomarco-ippolitomarco-ippolito left review comments
@mcollinamcollinamcollina approved these changes
@GeoffreyBoothGeoffreyBoothGeoffreyBooth approved these changes
@aduh95aduh95aduh95 approved these changes
+1 more reviewer
@ExE-BossExE-BossExE-Boss left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

moduleIssues and PRs related to the module subsystem.needs-ciPRs that need a full CI run.notable-changePRs with changes that should be highlighted in changelogs.semver-minorPRs 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

@guybedford@nodejs-github-bot@GeoffreyBooth@richardlau@ljharb@mcollina@ExE-Boss@aduh95@marco-ippolito