[WIP] Introduce an All target for outputting all supported module types by wasm-pack

[torch2424]

Make sure these boxes are checked! 📦✅

• You have the latest version of rustfmt installed

$ rustup component add rustfmt

• You ran cargo fmt on the code base before submitting
• You reference which issue is being closed in the PR text

✨✨ 😄 Thanks so much for contributing to wasm-pack! 😄 ✨✨

Description

relates to #697
closes #313

This introduces a --target all flag that will run wasm-bindgen for all of our supported outputs, prefixing them with a _[MODULE TYPE HERE]. And then adding all of the files to the resulting package.json.

Things still to do

• Tests (Wanted to get some approval before writing them)
• Comments made in give option target=all which generates pkg supporting commonjs and esm #313 regarding .mjs 😄 cc @stephanemagnenat

cc@alexcrichton @ashleygwilliams

P.S I wrote the last half of this at 2AM. So the code may be terrible, but I figured we could get through this together 😂

Output

[Pauan]

I haven't reviewed this deeply, but I wonder if --target all is the right approach. I can't think of any use case where you would actually want to output all of the targets.

Instead, I think supporting multiple --target would be better, so you could do wasm-pack build --target bundler --target nodejs for example. You could still output all targets, you'd just have to list all of them. This gives a lot more flexibility.

[ashleygwilliams]

@Pauan multiple people have requested an all or "isomorphic" target for publishing cross compatible packages to npm. i do like your suggestion that perhaps the targets are configurable- strikes me as something we could have folks do in a Cargo.toml!

[ashleygwilliams]

given @Pauan's comment and the likelihood that folks will want to customize the types of targets they want... i could imagine that instead of an All type, you have a new type that is a Targets: Vec<Target>- and for just Node, the Targets would by Vec<Nodejs>, and all would be Vec<Web, Nodejs, NoModules>

[Pauan]

@ashleygwilliams I know, but my point is that --target all is the same as --target nodejs --target bundler, so if wasm-pack supports multiple targets then there's no need for --target all anymore, since they're the same thing, except multiple targets is more flexible.

[ashleygwilliams]

@Pauan i think you and i are agreeing with each other! the one difference is that i think aliasing --target nodejs --target bundler to --target all is probably OK (honestly i think configuring target in Cargo.toml is my preferred)

this task is so common that i dont think it's reasonable to make it burdensome with an overly verbose command, and target feels like the property of a project so putting it in the project config makes a lot of sense i think

[torch2424]

Sorry for the late response, was having a fun time at RustConf! 😄

---

@Pauan Thanks for the feedback 😄

---

@ashleygwilliams @Pauan

So what's the plan here? I see 3 things here that I think would be worth getting an agreement on 😄

1. Do we want an all target? (Either as an alias, or a target itself).
2. Do we want this configurable in the Cargo.toml?
3. How do we specify which targets to build in either in the Cargo.toml, or as CLI flags?

In my opinion, I think my answer to each question depends on each of the answers to the above. But reading what we currently have, I think maybe it's worth proceeding like this:

As CLI Flags

1. Restart this PR.
2. I personally think aliasing is confusing, so I would not want an all target unless it had specific behavior (like this PR shows).
3. I'd prefer target to become a comma seperated of space seperated list that you pass in. E.g --target web nodejs bundler
4. We do the Vec<> implementation that @ashleygwilliams mentioned, and migrate relevant code from the PR to a new PR.

As Cargo.toml

1. Restart this PR.
2. Create a new field like [wasm-pack targets]?
3. Read the field when building, and a --target flag overrides the targets in the Cargo.toml.
4. We do the Vec<> implementation that @ashleygwilliams mentioned, and migrate relevant code from the PR to a new PR.

Thanks everyone! 😄 👍 Let me know what you think!

[rivertam]

User waiting on this feature weighing in:

• I currently build using CLI arguments that I like to change without having to change the Cargo.toml. The main reason for this is that I need to output multiple targets. However, --target all feels very natural to me in addition to the fact that it's already part of my workflow. I think maybe a Cargo.toml default which can be overridden by CLI flags? I'd start with just CLI flags because I don't think that should hold up this PR because there's bikeshedding that could be done when the feature could just be completed later
• I think --target all makes perfect sense for aliasing. --target web,nodejs,bundler is fine too.

Basically, in my ideal world, I don't think any of these ideas are at odds. I'm also pretty impatient waiting for this to land as I've been struggling with this kind of issue all week, so take everything here with a grain of salt.

[torch2424]

Thanks for the feedback @rivertam ! 👍

Going to bump this to @ashleygwilliams and @Pauan if they don't mind 😄

As I want to get a plan going before I write more code 😂

[Pauan]

@torch2424 I can't speak for Ashley, but this is my personal opinion:

• CLI flags are mandatory, so we will want multiple --target support no matter what.

• I don't like the all target, because it implies that it builds all the targets, but it actually only builds 2.

  And there's some complex questions like, "what targets should all build? what if we want to change the all targets later?"

  So I think the all target should be debated/handled in a new PR.

• I think configuration with Cargo.toml is okay, but that can be handled in a separate PR, I think this PR should be narrowly scoped to just multiple --target.

• --target cannot be space separated. Comma separated is okay, though passing multiple --target I think is a bit "cleaner".

  I think it might be reasonable to support both multiple --target and comma separated as well, so you can do either --target foo --target bar or --target foo,bar

[cormacrelf]

• --target cannot be space separated. Comma separated is okay, though passing multiple --target I think is a bit "cleaner".

cargo uses space-separators for features. They just need to be surrounded by quotes. Is this anomalous or the standard for cargo?

cargo build --features "one two three"

[Pauan]

@cormacrelf Being surrounded by quotes is okay too, but @torch2424 mentioned space separated without any quotes.

The reason spaces don't work is because then it can't tell the difference between a --target argument and a regular argument. So it needs to have something to distinguish it (whether it be quotes, commas, etc.)

[torch2424]

@cormacrelf @Pauan

Totally okay with not being space separated. If cargo is currently using quotes, than I think using quotes and being space separated within the quotes makes sense 😄

That being said, to just push things along for users like @rivertam , let's proceed as follows:

CLI Flags

1. Restart this PR.
2. Multiple targets are passed in as quotes. E.g --target "web nodejs bundler"
3. We do the Vec<> implementation that @ashleygwilliams mentioned, and migrate relevant code from the PR to a new PR.

I'll wait for a final thumbs up from @ashleygwilliams (I'll ping them on discord as well) and then start on the new PR 😄 🎉

[rivertam]

Thanks @torch2424. I appreciate you.

It's worth noting I used your branch to create a similar as your branch makes:

package.json:

{
  "name": "my-package",
  "collaborators": ["Ben Berman <ben@standardbots.com>"],
  "version": "0.0.0",
  "files": [
    "my-package.bundle/my-package_bg.wasm",
    "my-package.bundle/my-package.js",
    "my-package.bundle/my-package.d.ts",
    "my-package.node/my-package_bg.wasm",
    "my-package.node/my-package.js",
    "my-package.node/my-package_bg.js",
    "my-package.node/my-package.d.ts"
  ],
  "main": "my-package.node/my-package.js",
  "module": "my-package.bundler/my-package.js",
  "types": "my-package.bundler/my-package.d.ts",
  "sideEffects": "false"
}

then I bundle the package with

wasm-pack build --target bundler --out-dir my-package/my-package.bundler 
wasm-pack build --target nodejs --out-dir my-package/my-package.node

and I think this results in basically the same output as I want, so for me and presumably for anyone who reads this comment, the issue isn't too urgent anymore. This workflow is actually totally fine for me, and I'm no longer awaiting this PR because of this. That said, obviously this is a useful feature.

[tlaukkan]

This would be really valuable improvement for npm packages targeting node and webpack.

Managed to get single npm package to work for both node and webpack using manually created package with separate glue js and wasm files for different platforms:

https://github.com/tlaukkan/rust-ray-intersect-js/tree/master/pkg

The node glue js and wasm files have not been modified.
The bundler file has been combined from web and node with some added custom code. It is using wasm file from the web target. Webpack loads the wasm file as base64 which is decode to bytearray:

const bytes = _base64ToArrayBuffer(wasmBase64Bytes);

export async function init() {
    const wasmInstanceSource = await WebAssembly.instantiate(bytes , imports);
    const wasmInstance = wasmInstanceSource.instance;
    wasm = wasmInstance.exports;
}

vue-cli-service webpack config is:

        config.module
            .rule('wasm')
            .type('javascript/auto')
            .test(/\.wasm$/)
            .use('wasm')
            .loader('base64-loader')
            .end();

Glue js required async init() call to instantiate WebAssembly module asynchronously as Chrome requires this for large modules.

In the end it might be necessary to have separate target for creation this kind of isomorphic packages.

[jordy25519]

+1 for this feature. Any help needed to progress the PR?

[aminya]

Looking forward to this. 👍 It simplifies many things.

[torch2424]

Yoooo! What's up @aminya haha! 😄

---

So, this PR got a bit stale, as I was waiting for some confirmation on the right way to move forward on the PR. And now, I don't really have the time to fix up this PR. 😢

Though, I would be more than happy if someone were to take over the PR and things 😄

[curiousdannii]

Is there any way to manually accomplish this at present? I would like to be able to make an isomorphic package which uses a wasm-pack sub-package, ideally without needing a bundling step (for Node, something will still be needed for browser). The idea would be for my isomorphic package to be able to import the sub-package, with the sub-package loading the appropriate version, either Node or browser.

Problem is that I don't think there's any current target that will make it output for Node using ES modules.

Any ideas? What are people currently doing to make isomorphic packages?

[rivertam]

@curiousdannii Yeah, check out my comment earlier. That works for me.

[jordy25519]

In our project we generate a nodejs and browser compatible package with wasm-pack and have a top-level package.json which is published like so: https://github.com/cennznet/doughnut-rs/blob/2af67a162f650e993779642c6879a110fad7a498/js/package.json#L5-L6

Poking around the project may give you some ideas: https://github.com/cennznet/doughnut-rs/tree/master/js

The main issue we're seeing with this approach is it generates 2 wasm blobs which blows out the package size

[curiousdannii]

@rivertam The bundler output is not useable in Node: it tries to import the wasm file directly. (I also thought that using require would be a problem, but it doesn't seem to complain?)

@Holygits It looks like you're outputting CommonJS for Node and ESM for browsers, which is not an isomorphic package like I'm hoping can be achieved, as dependent packages would need to consume it in different ways, so they could not themselves be isomorphic.

I'm thinking that to get an ESM Node module I'll have to run the node output through Rollup or the like. That's okay - I already need to have a build step into order to invoke wasm-pack, and the main thing I wanted to avoid was requiring my package's users to have a build step.

[rivertam]

@curiousdannii I'm not sure what you mean. The method I describe above includes a package for both bundler and node. I'm not sure whether the bundler works for Rollup. I have the following setup:

• Package W written in Rust/wasm
• Package A written in TS targeted to both node and browser imports W. Jest runs test using node (and they're successful).
• A React application that imports package A

This topology works fine for me with the method I describe above, bundling both bundler and nodejs targets into the same package and using one package.json to point at both of them

[curiousdannii]

@rivertam I want package A to be able to import package W, loading either the Node or the Browser version as appropriate. This means for Node it would load the module property of package W. But if you generate that with the bundler you can't load it in Node because it tries to import the .wasm directly.

[rivertam]

@curiousdannii it's hard for me to understand what the problem is because that sounds exactly like what I'm describing to me. A is sometimes used in node and sometimes with webpack, and they both work.

[curiousdannii]

@rivertam Could it be that you're not running into issues because either Jest or TS is rewriting your import statements into CommonJS require calls, so that you're getting the main version instead of the module version?

If not, well I don't know then. Hard to say what's different in our setups.

[rivertam]

@curiousdannii Err... I'm pretty sure Jest/TS are rewriting to require calls, which I think is what it's supposed to do in a node context. It's only once you add a bundler (which I'm not doing except in the React application) that you should want it to use the "module" version. That's the point of the setup I describe -- it has a different "main" and "module" key so different environments load them differently.

[curiousdannii]

@rivertam Node has supported ES Modules natively for years (behind flags originally, though they're not required anymore), so you're not supposed to rewrite them to CommonJS. And even if you use TS, it can output ESM too. It's perfectly reasonable to ask for a tool such as wasm-pack to output ES Modules for Node, and if you want to make an isomorphic package without further build steps, it's necessary.

[rivertam]

Ah, I see. I wasn't aware that node natively supported modules at this point (and it appears it's still experimental as of node 15) but I see the problem now. Does it work in node if you export with target web? Just curious -- I understand that doesn't solve your problem.

[curiousdannii]

web doesn't work in Node either, though I can't remember exactly why.

Until something like this PR is merged, I'm expecting I'll just have to run the node output through Rollup to make an ESM version. Then at least consumers of my package won't need to use a transpiler/build step unless they already were.