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

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

Open
ashleygwilliams opened this issue Sep 19, 2018 · 10 comments · May be fixed by #705
Open

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

ashleygwilliams opened this issue Sep 19, 2018 · 10 comments · May be fixed by #705
Assignees
Labels
enhancement New feature or request next release to-do stuff that needs to happen, so plz do it k thx
Milestone

Comments

@ashleygwilliams
Copy link
Member

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

@stristr
Copy link

stristr commented Nov 5, 2018

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

@zrzka
Copy link

zrzka commented Nov 23, 2018

@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
Copy link
Member

drager commented Apr 27, 2019

I should be able to start working on this.

@ashleygwilliams
Copy link
Member Author

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
Copy link

torch2424 commented Jul 20, 2019

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 <script> tag (I want to be able to test this directly in browser console). This would be an IIFE. Similar to the --target web.

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 😄 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! 👍

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

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. 😄

@stephanemagnenat
Copy link

stephanemagnenat commented Aug 16, 2019

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
Copy link

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
Copy link
Contributor

lynn commented Mar 8, 2023

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
Copy link

@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
Copy link

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request next release to-do stuff that needs to happen, so plz do it k thx
Projects
None yet
9 participants