Skip to content

Align deno/module WASM import namespace with other targets#4871

Merged
guybedford merged 1 commit into
wasm-bindgen:mainfrom
magic-akari:refactor/deno-sharing-wasm
Dec 20, 2025
Merged

Align deno/module WASM import namespace with other targets#4871
guybedford merged 1 commit into
wasm-bindgen:mainfrom
magic-akari:refactor/deno-sharing-wasm

Conversation

@magic-akari

@magic-akari magic-akari commented Dec 18, 2025

Copy link
Copy Markdown
Member

Description

This PR updates the WASM import namespace for the Deno target to align with other targets, replacing the placeholder namespace with a relative module path.

Changes

WASM Import Namespace Change

;; Before
(import "__wbindgen_placeholder__" "__wbindgen_init_externref_table" (func ...))

;; After
(import "./reference_test.js" "__wbindgen_init_externref_table" (func ...))

This change also applies to the source-phase target, which reuses the Deno implementation for consistency.

Checklist

  • Verified changelog requirement

@magic-akari magic-akari force-pushed the refactor/deno-sharing-wasm branch 3 times, most recently from 83b89b7 to ab9d461 Compare December 18, 2025 15:35
@magic-akari magic-akari marked this pull request as ready for review December 18, 2025 15:41
Copilot AI review requested due to automatic review settings December 18, 2025 15:41

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR migrates the Deno target to use a dual-file ES module structure, aligning it with the bundler and experimental-nodejs-module targets for consistency.

Key Changes:

  • Introduced dual-file structure: main entry file (.js) and bindings file (_bg.js)
  • Changed WASM import namespace from __wbindgen_placeholder__ to ./{name}_bg.js in generated WAT files
  • Unified code generation path for Deno with other ESM integration targets

Reviewed changes

Copilot reviewed 16 out of 16 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
crates/cli-support/src/lib.rs Added Deno to ESM integration mode check and dual-file output logic
crates/cli-support/src/js/mod.rs Migrated Deno target to unified ESM integration code path with dedicated handling for Deno wasm loading
crates/cli/tests/wasm-bindgen/reference.rs Updated test to check _bg.js output for Deno target
crates/cli/tests/reference/targets-target-deno.wat Updated WASM imports from __wbindgen_placeholder__ to ./reference_test_bg.js namespace
crates/cli/tests/reference/targets-target-deno.js Main entry file now imports from _bg.js and re-exports bindings
crates/cli/tests/reference/targets-target-deno.bg.js New bindings file containing __wbg_set_wasm, exported functions, and glue code
crates/cli/tests/reference/targets-target-deno-mvp.* MVP variant updated with same structural changes
crates/cli/tests/reference/targets-target-deno-atomics.* Atomics variant updated with same structural changes
crates/cli/tests/reference/import-target-deno.* Import test files updated to reflect new dual-file structure and namespace changes
CHANGELOG.md Documented breaking change with description of structural and namespace changes

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread crates/cli-support/src/js/mod.rs Outdated
Comment thread crates/cli/tests/reference/import-target-deno.bg.js Outdated

@guybedford guybedford left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thinking about this some more, how does this contrast with the source phase imports form? Is there alignment with that or not here?

@magic-akari

Copy link
Copy Markdown
Member Author

The source-phase alignment will be implemented in #4850.

As we bring all targets into alignment, the differences between their glue code will diminish.

I will open a dedicated discussion to explore how to unify the glue code across targets — this will reduce duplication and improve maintainability.

@guybedford

Copy link
Copy Markdown
Contributor

I will open a dedicated discussion to explore how to unify the glue code across targets — this will reduce duplication and improve maintainability.

Thanks, my point here is that we have two kinds of designs in play - the old "wasm sandwich" model with the bg file, which is good for instance outputs for bundlers that support tree-shaking of Wasm, and then the more modern instantiation models using source phase that are single file but don't require separate JS files and can have one JS file, and then dynamic instnatiation models which can align with this source phase model.

That is, it is only when using import { foo } from './foo.wasm' style instance integration that we need the "bg" file as separate. In all other cases it should be possibly to unify programmatic instantiation on the single JS file approach, where the source phase is the canonical example of that, and all other dynamic instantiations are effectively "polyfills of the source phase".

So my preference would be unifying on the source phase output model, but then retaining the "bg" model in the direct Wasm instance phase cases only.

Let me know if that makes sense?

@magic-akari

Copy link
Copy Markdown
Member Author

Thanks for the clarification — I should have explained my original intention more clearly. I was actually planning to change the source phase output to use a separate _bg.js file as well. My main reason was maintainability: since the _bg.js logic is identical across output types, separating it out makes the code easier to reuse and refactor in the future. The differences between outputs would then only be in how the WebAssembly module is loaded.

That said, I completely understand and agree with your point—having multiple files adds extra network requests or file reads, so prioritizing performance over maintainability makes sense.

Could you confirm whether this is the design direction the wasm-bindgen team has chosen? If so, I’ll be happy to adjust this PR back to the single-file model.

@guybedford

Copy link
Copy Markdown
Contributor

Yes, exactly, thats the way we need to do it rather to only have bg for the direct Wasm instance phase output cases, and not otherwise, as we must prioritise output simplicity first and foremost.

@magic-akari magic-akari marked this pull request as draft December 19, 2025 13:43
@magic-akari magic-akari force-pushed the refactor/deno-sharing-wasm branch from 726dd8e to 68d3425 Compare December 19, 2025 14:41
@magic-akari magic-akari changed the title Align deno WASM import namespace with other targets Align deno/module WASM import namespace with other targets Dec 19, 2025
@magic-akari magic-akari force-pushed the refactor/deno-sharing-wasm branch 2 times, most recently from d8caa93 to 2f37aaa Compare December 19, 2025 15:15
@magic-akari magic-akari marked this pull request as ready for review December 19, 2025 15:29
@guybedford

Copy link
Copy Markdown
Contributor

This looks good to land to me, we just need to update the PR description and add a changelog entry.

Update WASM import namespace from __wbindgen_placeholder__ to ./{name}_bg.js
@magic-akari magic-akari force-pushed the refactor/deno-sharing-wasm branch from 2f37aaa to 09fa99f Compare December 20, 2025 18:06
@magic-akari

Copy link
Copy Markdown
Member Author

Done.

@guybedford guybedford merged commit 48c5eca into wasm-bindgen:main Dec 20, 2025
58 checks passed
@magic-akari magic-akari deleted the refactor/deno-sharing-wasm branch December 21, 2025 14:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants