runtime: implement WebAssembly/ESM integration for .wasm imports - #35587
runtime: implement WebAssembly/ESM integration for .wasm imports#35587robobun wants to merge 9 commits into
Conversation
import('./x.wasm') and `import * as x from './x.wasm'` previously fell
through to the file loader and resolved to { default: '<path>' }. The
wasm loader now reads the bytes, validates the magic header and hands
them to JSC as a WebAssemblySourceProvider so the module namespace is
the instance's exports, matching Node with --experimental-wasm-modules.
Legacy path-string behaviour is kept for ?query specifiers (#16476),
`with { type: 'file' }`, and require('./x.wasm').
Fixes #12434
Fixes #30369
|
Warning Review limit reached
Next review available in: 13 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (7)
Comment |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
…est, bundler fixture updates
- Factor 'auto_watch' block into auto_watch_asset() shared by the wasm
and file arms; gate the fd close on REQUIRES_FILE_DESCRIPTORS so
FreeBSD doesn't leak when add_file fails.
- Use create_external_globally_allocated_latin1 for disk-read wasm to
avoid one memcpy of the file.
- Factor sourceCodeForWasm() in ModuleLoader.cpp and call it from both
fetchESMSourceCode and handleVirtualModuleResult so a plugin whose
specifier ends in .wasm no longer hands raw bytes to a JS parser.
- Use bun_core::fmt::quote for path quoting in the two error messages.
- Add test for a wasm module importing from a JS module (exercises the
.wasm-keyed referrer resolve→fetch→link chain).
- Move test to test/js/bun/wasm/esm-integration.test.ts (feature, not
regression).
- Update the two bundler fixtures that used import-default-from-wasm as
an asset path to spell the intent with { type: 'file' }, so the same
source works both bundled and under bun run.
- Trim multi-line comments per comment-cop.
…loads after a bad header
There was a problem hiding this comment.
Thanks for addressing the earlier feedback — the watcher is now registered before the read/magic check, handleVirtualModuleResult handles the Wasm tag, error messages use bun_core::fmt::quote, the wasm→JS import chain is tested via simple.wasm, and the test file lives in test/js/bun/wasm/. No new issues found in this pass.
This is a user-facing behavior change to what import './x.wasm' returns, threaded through the module loader on both the Rust and C++ sides, so it's worth a human look before landing. The one remaining open nit (the ?query demotion overriding an explicit with { type: 'wasm' }) is minor and non-blocking.
What was reviewed
auto_watch_assetextraction: verified behavior matches the inlined block it replaces; fd-close now keys onREQUIRES_FILE_DESCRIPTORS(covers FreeBSD alongside macOS, as the PR description notes).sourceCodeForWasmis called from bothfetchESMSourceCodeandhandleVirtualModuleResult; theis_commonjs_requiredemotion intranspile_filekeeps CJS on the path-string route sofetchCommonJSModuleNonBuiltinnever sees aWasmtag.ResolvedSourceTag::Wasm = 3matches the pre-existingSyntheticModuleType::Wasmslot in the generated header.
Extended reasoning...
Overview
This PR implements WebAssembly/ESM integration: import './x.wasm' now compiles and instantiates the module and exposes its exports as named ESM bindings, matching Node's --experimental-wasm-modules and the WebAssembly/ESM integration proposal. Changes span src/runtime/jsc_hooks.rs (the L::Wasm arm of transpile_source_code_inner, a new auto_watch_asset helper factored out of the .file arm, and a CJS-require demotion in transpile_file), src/jsc/bindings/ModuleLoader.cpp (new sourceCodeForWasm helper called from fetchESMSourceCode and handleVirtualModuleResult), src/jsc/lib.rs (ResolvedSourceTag::Wasm = 3), a new 8-test spawn-based suite at test/js/bun/wasm/esm-integration.test.ts, and updates to three existing test files whose fixtures relied on the old path-string behavior.
Security risks
None identified. Wasm bytes are read via bun_sys::File::read_from and validated with a 4-byte magic-header check before being handed to JSC's WebAssemblySourceProvider; JSC owns the actual parse/validate/instantiate. No user-controlled data reaches shell/SQL/path-traversal surfaces. The Latin-1 byte packing in source_code round-trips through span8() on the C++ side without interpretation as text.
Level of scrutiny
High. This changes what a bare import './x.wasm' evaluates to — a documented, intentional break with escape hatches (?query, with { type: 'file' }, require()), but still a runtime-semantics change in the module loader that every .wasm import will hit. The implementation crosses the Rust↔C++ FFI boundary with a new ResolvedSource tag and touches watcher registration and the plugin virtual-module path. That's squarely in "a human should sign off" territory even though the code looks correct.
Other factors
The author has iterated through three rounds of review feedback on this PR and addressed each: fmt::quote for path quoting, the missing Wasm-tag branch in handleVirtualModuleResult, a wasm→JS-import test exercising the .wasm-keyed referrer resolve chain, test-file placement per CLAUDE.md, and watcher registration ordering so --watch reloads after a bad-magic error. Test coverage is solid (8 concurrent spawn tests covering dynamic/static/named imports, wasm-imports-JS, the three legacy escape hatches, and the bad-magic error path; USE_SYSTEM_BUN=1 divergence documented). One prior nit remains open (the ?query demotion firing even when type: 'wasm' was explicit) but was flagged as non-blocking. The pre-existing specifier/source_url refcount leak in the non-Zig::SourceProvider branches is unchanged in magnitude by this PR and was noted as out-of-scope. The auto_watch_asset refactor is a straight extraction with one intentional widening (fd-close on REQUIRES_FILE_DESCRIPTORS rather than macOS-only), which the PR description calls out.
4554dfe to
9659bec
Compare
There was a problem hiding this comment.
Additional findings (outside current diff — PR may have been updated during review):
-
🔴
src/js/internal/inspector/cdp.ts:1-9— This PR includes two unrelated new files —src/js/internal/inspector/cdp.ts(702 lines) andsrc/jsc/bindings/BunDebugger.h— added in commit 4554dfe ("ci: retrigger"). Neither is referenced anywhere else insrc/, neither is mentioned in the PR description, and both carry 20 unresolved comment-cop findings; they appear to have been committed accidentally and should be removed from this PR and landed separately with tests.Extended reasoning...
What
Commit
4554dfe7on this branch is titled "ci: retrigger" — conventionally an empty commit to re-run CI — butgit show --stat 4554dfe7reveals it actually adds two brand-new files totalling 717 lines:src/js/internal/inspector/cdp.ts(702 lines) — a Chrome DevTools Protocol ↔ JSC inspector protocol adapter, exportingInspectorCDPAdapterfornode:inspectorsrc/jsc/bindings/BunDebugger.h(15 lines) —JSC_DECLARE_HOST_FUNCTIONdeclarations forjsFunction_openNodeInspector/waitForNodeInspectorConnection/postNodeInspectorControl/closeNodeInspector
Neither has anything to do with WebAssembly/ESM integration, which is the sole subject of this PR's title, description, and every other changed file.
Why this is dead code
Grep across
src/forInspectorCDPAdapter,jsFunction_openNodeInspector,jsFunction_waitForNodeInspectorConnection,jsFunction_postNodeInspectorControl,jsFunction_closeNodeInspector,BunDebugger.h, andinspector/cdpreturns only the two new files themselves. Concretely:- Nothing
#includesBunDebugger.h. The four host functions it declares have noJSC_DEFINE_HOST_FUNCTIONcounterpart in any.cppfile (BunDebugger.cppdoesn't define them). - Nothing imports
InspectorCDPAdapterfromcdp.ts. The internal-module registry has no entry for it. - The PR description's "Fix" section enumerates every file it touches (
jsc_hooks.rs,ModuleLoader.cpp,lib.rs) — neither of these is mentioned.
Step-by-step proof
git log --onelineon this branch shows the wasm work landed across commits 48260d5 ("address review"), 78d67b7 (autofix), 4f686e4 (comment-cop cleanup), 9659bec (watcher fix) — all wasm-related.git show --stat 4554dfe7→ the "ci: retrigger" commit adds exactlycdp.ts(+702) andBunDebugger.h(+15), nothing else.rg 'InspectorCDPAdapter|jsFunction_.*NodeInspector|BunDebugger\.h|inspector/cdp' src/→ matches only the two new files.- The PR timeline shows 19 comment-cop inline findings on
cdp.ts(lines 9, 19, 31, 42, 70, 171, 189, 202, 209, 215, 222, 235, 247, 265, 277, 322, 370, 416, 485) and 1 onBunDebugger.h:9, all posted at 12:50 (one minute after the 12:49 commit) and none resolved — contrast with the earlier comment-cop findings onModuleLoader.cpp/jsc_hooks.rs, which the author addressed in 4f686e4 and which are marked resolved.
Why existing structure doesn't excuse it
REVIEW.md is explicit on both counts:
- "Every behavioral change ships an automated test in the same PR" —
cdp.tsis 702 lines of protocol-translation logic (message parsing, id correlation, scope-type mapping, console-message translation) with zero test coverage in this diff. - "Delete dead code in the same PR that makes it dead" — this is the inverse: adding 717 lines of code with no consumers. If it's WIP for a future
node:inspectorPR, it belongs on that branch. - PR-process guidance in
.claude/docs/landing-prs.md— unrelated features go in separate PRs so they can be reviewed with appropriate context.
Impact
Merging as-is would (a) ship 717 lines of unreviewed, untested dead code under a wasm-titled PR (making it undiscoverable in
git logfor whoever later works onnode:inspector), (b) leave 20 comment-cop findings unaddressed, and (c) landJSC_DECLARE_HOST_FUNCTIONdeclarations with no matching definitions. Since a "ci: retrigger" commit adding 717 lines is almost certainly accidental (bad rebase, wrong-branch commit, or leftover WIP), the author will want to remove these before merge regardless.Fix
git rm src/js/internal/inspector/cdp.ts src/jsc/bindings/BunDebugger.h git commit --amend # or a fresh commit dropping themLand the
node:inspectorCDP adapter in its own PR with tests, where the comment-cop findings can be properly addressed and the four declared host functions can ship alongside their definitions and JS-side callers.
|
The |
8e32f01 to
b0da7af
Compare
…atin-1→UTF-8 transcoding Bun.plugin build.module() whose specifier picks the wasm loader hands an untagged ZigString over ArrayBufferView bytes; to_slice() expands bytes ≥ 0x80 to their two-byte UTF-8 encoding, corrupting the module past the (all-ASCII) magic header. byte_slice() returns the raw span. Adds a plugin virtual-module test (add.wasm contains 0x80/0x88).
StatusDiff is ready at 1263c02. The gate's 4-file set passes 65/65 under the debug+ASAN build; CI build #80998 shows only known flakes on unrelated lanes ( |
…s ESM (#37525) Builtins implemented in `src/js` are handed to ES module importers by `generateInternalModuleSourceCode` (`src/jsc/bindings/ModuleLoader.cpp`), which snapshots the builtin's CommonJS exports object into a synthetic module record. It did that with `object->get()` on every own enumerable property, so every accessor on the exports object ran at import time. Those accessors are the builtins' lazy-loading mechanism, which means they only ever helped `require()` callers: - `node:fs`: `ReadStream`, `WriteStream`, `FileReadStream`, `FileWriteStream`, `Utf8Stream` each `require("internal/fs/streams")`, which loads the whole `node:stream` stack. - `node:tls`: `rootCertificates` parses the bundled CA store, `DEFAULT_CIPHERS` queries BoringSSL. - `node:http`: `globalAgent` instantiates the agent. `node:timers` / `node:stream`: `promises` load `timers/promises` and `stream/promises`. `node:assert`: `AssertionError` loads `internal/assert/assertion_error`. `node:repl`, `node:events`, `node:buffer`, `node:os` have a few more. #35541 is about to turn more of these into accessors (`fs.promises` among them), which this path would immediately defeat for the most common import style there is. ## Repro ```js // probe.mjs, run as `bun --expose-internals probe.mjs esm` vs `... require` const mode = process.argv[2]; const { createRequire } = await import("node:module"); let t = performance.now(); if (mode === "esm") await import("node:fs"); else createRequire(import.meta.url)("node:fs"); const fsMs = performance.now() - t; t = performance.now(); createRequire(import.meta.url)("internal/fs/streams"); console.log(mode, "node:fs", fsMs.toFixed(1), "ms; internal/fs/streams afterwards", (performance.now() - t).toFixed(1), "ms"); ``` bun 1.4.0 (release): `esm node:fs 13.3 ms; internal/fs/streams afterwards 0.2 ms` (already loaded) vs `require node:fs 7.8 ms; internal/fs/streams afterwards 5.0 ms`. On a debug build the difference is ~370ms per process, and `test/harness.ts` does `import fs from "node:fs"`, so every test file paid it. The directly observable form, which the tests use: the `node:fs` getters replace themselves with data properties when they run, so after `import fs from "node:fs"`, `Object.getOwnPropertyDescriptor(fs, "ReadStream")` had a `value` instead of a `get`. ## Fix An ES module binding cannot be made lazy from the embedder side: once the record exists, named imports read the exporting environment's slot directly (`ModuleVar`) and namespace reads go through `getValue()` in `JSModuleNamespaceObject`, so the slot has to hold the value before anything binds to it. The engine half is oven-sh/WebKit#408, merged as 7b763944f0ec. `WEBKIT_VERSION` moves from 09e477744721 (what main pins) to that sha; it is the only commit between the two: - `SyntheticModuleRecord::tryCreateWithExportNamesAndValues()` gets an overload taking a source object; an export whose value is the empty `JSValue` is declared but left in TDZ. - `materializeLazyExport()` fills such a slot from `source[name]` the first time something binds to it: `CyclicModuleRecord::initializeEnvironment` when an importer links a named import that resolves to it (directly or through `export *` / `export { x } from` chains), `JSModuleNamespaceObject::getOwnPropertySlotCommon` when a namespace read finds the slot empty (it then re-reads and installs the value the same way as before, so the namespace IC still applies), and `WebAssemblyModuleRecord::initializeImports`, which snapshots imported bindings directly (unreachable in bun today, reachable once #35587 lands). It is a no-op for a slot that already has a value, so `overrideExportValue` (what `mock.module` and `spyOn` use) keeps winning, `*namespace*` is skipped, a getter that re-enters keeps the first value that landed, and an exception from the getter propagates and leaves the slot for the next attempt. The write uses the same `symbolTablePutTouchWatchpointSet` the eager path used for its one write, so IC / DFG behaviour after materialization is identical to an eagerly built record; the baseline namespace IC already bails to the slow path on an empty slot and the DFG only folds loads that went through an installed IC. - `SyntheticSourceProvider::createWithLazyExports()` takes a generator that returns the source object, and `makeModule` passes it through. Records built any other way (JSON modules, `vm.SyntheticModule`, every existing `create()` caller) have no source object and take a pointer test on the new paths. The bun half is `generateInternalModuleSourceCode`: it now looks each property up with `getOwnPropertySlot`, snapshots data properties exactly as before, declares everything else (accessors) as a lazy export, and returns the exports object as the source. A builtin without accessors (most of them) produces a record identical to the old one. `require()` of a builtin never went through this function (`fetchCommonJSModule` returns the registry object), so it is unaffected. The engine change is inert without this opt-in: building the new WebKit with the old `ModuleLoader.cpp` still fails the new tests the same way the release does. ### Why this is the right behaviour Node builds the ESM facade of a builtin by reading the getters too, so the old behaviour was not wrong, just expensive, and there is no compat reason to keep it. The only observable difference is *when* an accessor export is sampled: at import before, at the point something first binds to it now. Data properties still snapshot at import. A namespace export or named import of an accessor is a snapshot either way (that is what `module.syncBuiltinESMExports()` exists for), and the later sample is never staler than the old one. Everything an importer can do with the export resolves to the same object `require()` would hand out, which is what the tests check for each binding path. `Object.keys(ns)`, `hasOwnProperty`, spread, and `console.log(ns)` still materialize what they touch, because `[[GetOwnProperty]]` has to produce the value; `in` ([[HasProperty]]) does not. Plugin `loader: "object"` modules and user CommonJS imported from ESM are deliberately not touched: those expose user objects whose getter timing is user-visible (#36677 is about that), whereas builtins' accessors are our own lazy-loading idiom. ## Tests `test/js/bun/resolve/builtin-esm-lazy-exports.test.ts`, next to the other module-system behaviour tests in that directory. Each case runs in its own process (a binding can only be materialized once) and checks both that importing left the `node:fs` accessors untouched and that the binding is the real class once used, for: default + named data import, named import of an accessor (exactly that one materializes), `import * as`, `import defer * as`, dynamic `import()` including `Object.keys` and that the export list is unchanged, `export *` and `export { x as y } from` re-exports (named import through the star and a read off the re-exporter's namespace, which exercises materializing on a record other than the namespace's own), accessors on `node:assert` (a function exports object), `node:timers` and `node:stream`, and `spyOn` / `mock.module` on an already imported builtin. 7 of the 8 cases fail on the current eager code (everything reports `value`); the `assert`/`timers`/`stream` identity case is a regression guard. With the fix, the probe above reports `internal/fs/streams` as not loaded after `import("node:fs")` (debug build: 338ms to load it afterwards in both modes, versus 10ms before because the import had already loaded it). Also run on the new build: `test/js/node/fs/fs.test.ts` (covers the existing `export { ReadStream, WriteStream } from "node:fs"` and `export * from "node:fs"` fixtures), the `test/js/bun/resolve` module tests, `test/js/bun/test/mock/mock-module*.test.ts`, `node-module-module.test.js`, `buffer-inspectmaxbytes.test.ts` (which pins that a named import of an accessor is a snapshot), and the events / os / timers / assert / tls-internals / node-http suites, all green apart from two failures that are identical on the released binary in this environment (`os.userInfo`, the http proxy test). <!-- robobun:evidence:begin --> --- **[decide:webkit]** gate passed · iteration 0 · 3 files touched <details><summary>fails on main (without fix)</summary> ```console ASAN without fix: 7 FAILED $ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/mechgate.xml" test/js/bun/resolve/builtin-esm-lazy-exports.test.ts bun test v1.4.0 (435641a) test/js/bun/resolve/builtin-esm-lazy-exports.test.ts: 132 | afterKeys: kinds(), 133 | keysMatch: namespaceKeys.sort().join() === [...exportsKeys, "default"].sort().join(), 134 | allAreTheClasses: STREAMS.every(name => typeof ns[name] === "function" && ns[name] === fs[name]), 135 | }); 136 | `); 137 | expect(result).toEqual({ ^ error: expect(received).toEqual(expected) { "afterImport": { - "FileReadStream": "accessor", - "FileWriteStream": "accessor", - "ReadStream": "accessor", - "Utf8Stream": "accessor", - "WriteStream": "accessor", + "FileReadStream": "value", + "FileWriteStream": "value", + "ReadStream": "value", + "Utf8Stream": "value", + "WriteStream": "value", }, "afterKeys": { "FileReadStream": "value", "FileWriteStream": "value", "ReadStream": "value", "Utf8Stream": "value", "WriteStream": "value", }, "after ... (truncated) release without fix: 7 FAILED bun test v1.4.0-canary.1 (9008ae7) test/js/bun/resolve/builtin-esm-lazy-exports.test.ts: 68 | const result = await runEntry(` 69 | import { ReadStream } from "node:fs"; 70 | import { fs, kinds, print } from "./helper.mjs"; 71 | print({ kinds: kinds(), isTheClass: typeof ReadStream === "function" && ReadStream === fs.ReadStream }); 72 | `); 73 | expect(result).toEqual({ kinds: kinds("ReadStream"), isTheClass: true }); ^ error: expect(received).toEqual(expected) { "isTheClass": true, "kinds": { - "FileReadStream": "accessor", - "FileWriteStream": "accessor", + "FileReadStream": "value", + "FileWriteStream": "value", "ReadStream": "value", - "Utf8Stream": "accessor", - "WriteStream": "accessor", + "Utf8Stream": "value", + "WriteStream": "value", }, } - Expected - 4 + Received + 4 at <anonymous> (/workspace/bun/test/js/bun/resolve/builtin-esm-lazy-exports.test.ts:73:18) 89 | afterRead: kinds(), 90 | isTheClass: typeof WriteStream === "function" && WriteStream === fs.WriteStream, 91 | secondReadIsStable: ns.WriteStream === WriteStream, 92 | }) ... (truncated) ``` </details> <details><summary>passes on PR (with fix)</summary> ```console ASAN with fix: all passed $ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/mechgate.xml" test/js/bun/resolve/builtin-esm-lazy-exports.test.ts bun test v1.4.0 (435641a) test/js/bun/resolve/builtin-esm-lazy-exports.test.ts: (pass) importing the module does not run its accessors [1746.65ms] (pass) a named import materializes exactly the binding it links [2145.14ms] (pass) a namespace export materializes when it is read, not on import or `in` [2175.43ms] (pass) a deferred namespace (import defer) materializes on read as well [2720.80ms] (pass) import() namespace: same export list as before, enumerating it materializes everything [2873.44ms] (pass) re-exports bind through to the builtin's own binding [2232.58ms] (pass) spyOn and mock.module on an imported builtin [2125.50ms] (pass) accessors on other builtins bind to what require() returns [2735.47ms] 8 pass 0 fail 24 expect() calls Ran 8 tests across 1 file. [8.97s] __F:0:S:0 release with fix: all passed $ bun scripts/build.ts --profile=release [configured] bun-profile → bun (stripped) target linux-x64-gnu build type Release build dir ./build/release revision 435641a features baseline 22 deps, 107 codegen, 1176 objects in 1540ms ninja: Entering directory `/workspace/bun/build/release' [1/1238] gen ErrorCode+*.h [2/1238] gen bindgenv2 [3/1238] gen JSBuffer.lut.h Generating /workspace/bun/build/release/codegen/JSBuffer.lut.h from /workspace/bun/src/jsc/bindings/JSBuffer.cpp [4/1238] fetch zlib [zlib] up to date [5/1238] fetch libjpeg-turbo [libjpeg-turbo] up to date [6/1238] fetch tinycc [tinycc] up to date [7/1237] fetch picohttpparser [picohttpparser] up to date [8/1237] gen .bind.ts → GeneratedBindings.cpp [9/1237] gen ProcessBindingBuffer.lut.h Generating /workspace/bun/build/release/codegen/ProcessBindingBuffer.lut.h from /workspace/bun/src/jsc/bindings/ProcessBindingBuffer.cpp [10/1237] gen ProcessBindingConstants.lut.h Generating /workspace/bun/build/release/codegen/ProcessBindingConstants.lut.h from /workspace/bun/src/jsc/bindings/ProcessBindingConstants.cpp [11/1237] gen ProcessBindingHTTPParser.lut.h Generating /workspac ... (truncated) ``` </details> <details><summary>diff hotspot</summary> ``` scripts/build/deps/webkit.ts | 2 +- src/jsc/bindings/ModuleLoader.cpp | 28 ++- .../bun/resolve/builtin-esm-lazy-exports.test.ts | 232 +++++++++++++++++++++ 3 files changed, 254 insertions(+), 8 deletions(-) ``` </details> **gate history** · 1 passed · 0 rejected · iteration 0 <details><summary>evidence per changed file</summary> ``` file reads edits tests scripts/build/deps/webkit.ts 3 3 0 src/jsc/bindings/ModuleLoader.cpp 1 3 0 test/js/bun/resolve/builtin-esm-lazy-exports.test.ts 0 0 0 ``` </details> <!-- robobun:evidence:end -->
1 participant
What
import * as m from './file.wasm'(andawait import('./file.wasm')) now compiles and instantiates the module, exposing its exports as named ES module exports. This matches Node's--experimental-wasm-modulesand the WebAssembly/ES Module Integration proposal.Repro
Cause
The runtime
.wasmloader intranspile_source_code_innerrecursed into.file, which returns{ __esModule: true, default: pathString }via theExportDefaultObjecttag. JSC'sJSModuleLoaderalready dispatchesSourceProviderSourceType::WebAssemblytoJSWebAssembly::instantiate; we just never handed it aWebAssemblySourceProvider.Fix
src/runtime/jsc_hooks.rs: in theL::Wasmarm, read the wasm bytes (fromvirtual_sourceor disk viabun_sys::File::read_from), validate the magic header, register with the watcher, and returnResolvedSource { tag: Wasm, source_code }. The disk-read path wraps theVec<u8>as an external string to avoid a copy. Factored the open-fd-and-watch block intoauto_watch_asset()shared with the.filearm, and keyed its fd-close onREQUIRES_FILE_DESCRIPTORSso FreeBSD matches macOS.src/jsc/bindings/ModuleLoader.cpp: newsourceCodeForWasm()reads the 8-bit span back out and buildsJSC::WebAssemblySourceProvider::create(bytes, origin, key). Called fromfetchESMSourceCode(filesystem imports) andhandleVirtualModuleResult(Bun.pluginvirtual modules whose specifier picks the wasm loader).src/jsc/lib.rs: addResolvedSourceTag::Wasm = 3(matching the existingSyntheticModuleType::Wasmin the generated header).Legacy behaviour preserved where an asset path is the intent:
?query-suffixed specifiers ('./x.wasm?1', see Regression: release 1.1.45 broke?urlimports #16476) keep the old path-string demotion.with { type: 'file' }already overrides the loader before reaching the.wasmarm.require('./x.wasm')keeps the path-string behaviour via anis_commonjs_requiredemotion intranspile_file(Node rejects CJS wasm entirely; Bun has always returned the path here and users rely on it).Bundler note
bun buildstill treats.wasmas a copied asset (unchanged by this PR), soimport x from './a.wasm'means two different things at runtime vs in a bundle. The two existing bundler tests that relied onimport wasmPath from './x.wasm'returning a path now spell that intent explicitly withwith { type: 'file' }, which works the same under both. Teaching the bundler to emit ESM-integration semantics fortarget=bunis tracked by #22026 and out of scope here;import source(#31677) is the spec-blessed way to get a compiled-but-not-instantiated module in bundled output.Why this is the right shape
The ESM integration proposal is the standards-track answer to "what does
import './x.wasm'mean", Node ships it behind a flag (unflagged for source-phase in ≥ 24), and JSC already has the whole instantiation path wired toSourceProviderSourceType::WebAssembly. Returning a path string was a placeholder from before any of that existed. The compat escape hatches (?query,type: "file",require()) cover every case where someone was intentionally treating it as an asset.Previously attempted as #30371 against the Zig tree; that PR was closed when the source files moved under the Rust rewrite.
Verification
test/js/bun/wasm/esm-integration.test.ts(8 spawn-based tests) covers: dynamic import, staticimport * as, named imports, wasm importing from a JS module (exercises the.wasm-keyed referrer resolve→fetch→link chain),?querypath-string preservation,with { type: 'file' }path-string preservation,require()path-string preservation, and bad-magic-header rejection.Also passing under the debug build:
test/regression/issue/16476(?querywasm imports),test/js/bun/resolve/import-empty.test.js(updated:type: "wasm"on an empty file now throws the magic-header error, matching Node),test/js/bun/wasm/wasi.test.js(CLIbun ./foo.wasmpath unchanged),test/js/web/fetch/wasm-streaming.test.ts,test/bundler/bundler_loader.test.ts -t wasm-is-copied-to-outdir,test/bundler/bun-build-compile-wasm.test.ts,test/bundler/bundler_plugin.test.ts -t FileLoaderMultipleAssets.Fixes #12434
Fixes #30369
[review] gate passed · iteration 2 · 8 files touched
fails on main (without fix)
passes on PR (with fix)
diff hotspot
gate history · 1 passed · 2 rejected · iteration 2
evidence per changed file