Skip to content
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

add bundler target to wasm-bindgen-test-runner to support ESM #3242

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

robertkiel
Copy link

@robertkiel robertkiel commented Jan 17, 2023

tldr

Allows wasm-bindgen-test-runner to create and execute native ESM code.

Context

More and more JS / TS libraries are transforming from historic CommonJS to ESM, once this transformation is done, Node.js refuses to load them via const myModule = require('my-module'). Instead, modules need to loaded as import myModule from 'my-module' - or dynamically using const myModule = await import('my-module').

Within Node.js, ES modules are distinguished from CommonJS modules by their file ending .mjs vs .cjs or by the closest package.json file's type section. The file ending has precedence over the package.json file, allowing ES modules in a CommonJS package and vice-verse.

Changes in detail

Exposes the compile target bundler via the environment variable WASM_BINDGEN_USE_BUNDLER to wasm-bindgen-test-runner.

Adds a new bundler execute section, written in ESM syntax and explicit .mjs file ending such that Node.js loads it in ESM-mode.

@robertkiel robertkiel marked this pull request as ready for review January 17, 2023 13:33
@robertkiel robertkiel changed the title add bundler target to wasm-bindgen-test-runner add bundler target to wasm-bindgen-test-runner to support ESM Jan 17, 2023
@Liamolucko
Copy link
Collaborator

This won't work. The bundler target relies on the WebAssembly esm-integration proposal to import WebAssembly files, which Node (and all other runtimes that I'm aware of) doesn't support yet, unless you pass the --experimental-wasm-modules flag.

@robertkiel
Copy link
Author

This won't work. The bundler target relies on the WebAssembly esm-integration proposal to import WebAssembly files, which Node (and all other runtimes that I'm aware of) doesn't support yet, unless you pass the --experimental-wasm-modules flag.

So why don't run node with the flag? Within the client that we're creating this works quite well.

@daxpedda daxpedda added the needs discussion Requires further discussion label May 10, 2023
@daxpedda
Copy link
Collaborator

So why don't run node with the flag?

I guess we don't really want to support all the experimental stuff out there. I know little about Node.js, WDYT @Liamolucko.

@robertkiel it seems to me that, currently at least, it isn't really required to get things going, is that correct?

@daxpedda daxpedda added needs review Needs a review by a maintainer. and removed needs discussion Requires further discussion labels May 11, 2023
@Liamolucko
Copy link
Collaborator

So why don't run node with the flag?

I guess we don't really want to support all the experimental stuff out there. I know little about Node.js, WDYT @Liamolucko.

I mean, we could, but I think it’d be better to avoid relying on experimental features, particularly when it’s being run inside wasm-bindgen-test-runner as opposed to being explicitly specified by the user.

@robertkiel, is there a particular reason why this needs to use esm-integration to load the wasm file rather than instantiating it the normal way? If not, this could avoid the experimental flag by instead using the web target, which is still ESM, and manually loading the wasm from the file system and passing it to init to sidestep the web target's usual fetch-based loading. Obviously the web target isn’t designed to be used with Node, but neither is the bundler target.

There’s also the issue of telling Node to interpret the shim as an ES module. This PR already names the wasm-bindgen-test runtime script run.mjs to make that get interpreted as an ES module, but it doesn't do the same for the shim, which means that Node interprets it as CommonJS if ”type”: “module” isn’t set. It shouldn’t be too hard to fix that, by renaming the files from .js to .mjs after wasm-bindgen-cli-support writes them.

@daxpedda daxpedda added waiting for author Waiting for author to respond and removed needs review Needs a review by a maintainer. labels May 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
waiting for author Waiting for author to respond
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants