give option target=all which generates pkg supporting commonjs and esm

[ashleygwilliams]

trafficstars

groundwork in #312 once merged, i'll start on it. in the meantime- if we want to bikeshed the target argument, let's

[stristr]

Hey @ashleygwilliams, is this up for grabs, or underway? Happy to help out.

[zrzka]

@ashleygwilliams seems that #312 is merged. Do you have any plans to fix this?

This would be really awesome. Then I'll be able to trash the following script, which packs for both targets and then merges these two packages into one.

#!/usr/bin/env bash

set -e

# Check if jq is installed
if ! [ -x "$(command -v jq)" ]; then
    echo "jq is not installed" >& 2
    exit 1
fi

# Clean previous packages
if [ -d "pkg" ]; then
    rm -rf pkg
fi

if [ -d "pkg-node" ]; then
    rm -rf pkg-node
fi

# Build for both targets
wasm-pack build -t nodejs -d pkg-node
wasm-pack build -t browser -d pkg

# Get the package name
PKG_NAME=$(jq -r .name pkg/package.json | sed 's/\-/_/g')

# Merge nodejs & browser packages
cp "pkg-node/${PKG_NAME}.js" "pkg/${PKG_NAME}_main.js"
sed "s/require[\(]'\.\/${PKG_NAME}/require\('\.\/${PKG_NAME}_main/" "pkg-node/${PKG_NAME}_bg.js" > "pkg/${PKG_NAME}_bg.js"
jq ".files += [\"${PKG_NAME}_bg.js\"]" pkg/package.json \
    | jq ".main = \"${PKG_NAME}_main.js\"" > pkg/temp.json
mv pkg/temp.json pkg/package.json
rm -rf pkg-node

[drager]

I should be able to start working on this.

[ashleygwilliams]

from #469, @KSXGitHub:

When I write an NPM package, I rarely think of bundlers as they can comprehend require() syntax, so as long as I only use require() to load module, I don't worry about bundlers. This is not the case with Node.js WASM modules generated by wasm-bindgen as it uses fs.readFileSync (which cannot be polyfilled by bundlers) to load .wasm file.

wasm-pack build currently supports 2 targets:

• --target browser support bundlers (today's browsers cannot load the output script/module directly) but not Node.js.
• --target nodejs support Node.js runtime but not bundlers/browsers.

This is inconvenient, if I want to support both targets, I have to write my own wrapper that calls wasm-bindgen twice.

Suggestion

• Without options, generate 4 files for 4 targets:

  • index.bundler.js for JavaScript bundlers, set "browser" field in package.json to point to this file.
  • index.js for Node.js runtime, set "main" field in package.json to point to this file.
  • index.mjs to work with node --experimental-module, "module" field of package.json may or may not point to this file.
  • index.script.js to work with <script> tag (I want to be able to test this directly in browser console).

• Deprecate the use of --target <target> in favor of --nodejs index.js, --module index.mjs, --browser index.bundler.js, and --script index.script.js.

Additional Suggestion

Please don't use package name in Cargo.toml for JavaScript entry files, use index.* like most NPM packages.

[torch2424]

I totally agree with the suggestion to generate multiple outputs as mentioned by @KSXGitHub and @ashleygwilliams ! And think it is definitely a good way forward for a --target all.

However, I think it'd be good to look at "prior art" here, and take a look at how Rollup handles this same issue (specifying multiple JS targets for a library). Especially since rollup is "the bundler for libraries" (NOTE: The article is a bit outdated, but it drives the point).

I think maybe we should follow a file bundling strategy similar to rollup-starter-lib. In which the --target all generates the following:

• index.cjs.js CommonJS Module for Node.js runtime, set "main" field in package.json to point to this file. Similar to the --target nodejs, and explicitly use node wasm APIs (fs.readFile...).
• index.esm.js ES Module meant for use in the browser. set "module" field in package.json. Similar to the --target web.
• index.iife.js to work with

Regarding blunders, I think it would be up to the bundler to choose which format to choose when bundling (or up to the project author to choose the correct format for their bundler). Or, we can simply run the bundler option, and do an index.bundler.js as already suggested, to preserve immediate functiontiality?

This is very similar to what @ashleygwilliams is suggesting above, but I guess I'm just more trying to nail down the convention here :smile: Please let me know if there is gaps in this at all I am missing.

Also, it seems that a lot of other people have offered to pick this up, but I'd like to also offer as well. Let me know, thank you! :+1:

P.S Glad I found this issue after the rust/wasm WG meeting haha! :joy:

EDIT: P.S P.S I have started working on this, on my fork/branch. I've also been giving updates on this at the rust/wasm WG meetings. I'm about 1/2 way done. :smile:

[stephanemagnenat]

Just a note from a user here. We encountered the same issue as mentioned by @ashleygwilliams: We are using Typescript sources along with a Rust-based wasm helper. These are built by Webpack, and we would like to use Jest (or another node-based tool) for unit testing.

Even by running node with --experimental-modules --experimental-wasm-modules, it seems due to the way ts-jest compiles Typescript file on its side, we did not manage to tell node to load the ES6 module that wasm-pack generated (it seems unclear where to place the file package.json containing { "type": "module" } so that it is found by ts-jest).

Hence, it would work nicely if wasm-pack would generate index.mjs as it is an easy (or even the only reliable one?) way to tell node to load the file as ES6 module on import. @torch2424 please consider that loading challenge from node side when choosing the file extensions.

[chriamue]

Just a note from a user here. We encountered the same issue as mentioned by @ashleygwilliams: We are using Typescript sources along with a Rust-based wasm helper. These are built by Webpack, and we would like to use Jest (or another node-based tool) for unit testing.

Even by running node with --experimental-modules --experimental-wasm-modules, it seems due to the way ts-jest compiles Typescript file on its side, we did not manage to tell node to load the ES6 module that wasm-pack generated (it seems unclear where to place the file package.json containing { "type": "module" } so that it is found by ts-jest).

Hence, it would work nicely if wasm-pack would generate index.mjs as it is an easy (or even the only reliable one?) way to tell node to load the file as ES6 module on import. @torch2424 please consider that loading challenge from node side when choosing the file extensions.

Same here, did you find any solution or workaround @stephanemagnenat ?

[lynn]

I ran into this issue in the same way as the Jest users above.

@chriamue, my workaround to simply run two wasm-pack commands

wasm-pack build --target bundler --out-dir pkg  # (the defaults)
wasm-pack build --target nodejs --out-dir pkg_node

My web app's package.json has a dependency like

    "foo_wasm": "file:../wasm/foo/pkg",

Then my jest.config.ts contains

  moduleNameMapper: {
    foo_wasm: "<rootDir>/../wasm/foo/pkg_node",
  },

[stephanemagnenat]

@chriamue sorry I missed your ping, we use a similar work-around as @lynn, we create two packages and remap; here is an extract from our jest.config.ts

moduleNameMapper: {
    "app-core-web/pkg": "app-core-web/pkg-node"
}

[segevfiner]

Having just one package generated that can work with either a bundler, browser, or node will be awesome.

Sadly, there is a whole lot of mess in the JS ecosystem about stuff like this, that is: workers, wasm, static files in packages, meant for use in both the browser, node, multiple types of bundlers, etc. So you might to actively try the output in different targets to make sure they all take the correct build or otherwise write instructions on how to make them do so.

Worse are the inconsistencies in handling all the top level main, and de-facto standard module, browser fields and the newer exports field between different bundlers, test runtimes and so on, which might further trip this up.