-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Example of Default Output that is compatible for Node and Browsers #233
Comments
Would love to receive a PR that clarified the README!
I don't know how node plans on supporting es6 modules, but maybe @ashleygwilliams knows more.
If you aren't supporting the Web, and are only supporing node, then the |
Well node supports es6 modules if you do this:
However the problem is in the top of the generated bindgen file you have something like:
Here are the files that it generated (default settings):
and running:
Results in:
It'd be great if the default output of wasm_bindgen accomodated for both commonjs and es6 module with webpack. Since commonjs isn't going away anytime soon. :-\ |
I spent a good amount of time on this problem ~ a week ago. Looking over what google has to say about writing js for both Node and the browser is pretty tricky. The biggest issue right now is that node isn't currently resolving There isn't a good test that I can find to account for this if (require) {
//node
} else {
//browser
} The above doesn't work since webpack would attempt to bundle any of the code you included in either side of the block, which would again attempt to resolve I think there might be a way to get this to work using const nodeWasm = `...`;
module.exports = function() {
if (typeof self !== 'object') {
return eval(nodeWasm);
}
return require('./module_bg.wasm');
}() but I haven't been able to test that as of yet |
You hit all the painpoints we did. Eventually I had to create a post build script to remove node related files. It feels super janky but it was the only way that webpack wouldn't try to read files we didn't want it to. :-/ |
After some additional work I was able to get the eval solution to actually work for both environments. It is actually pretty weird to me that webpack can resolve The full solution can be found here; In the pkg folder you can see that I have taken the At this point I think that instead of updating the default bindings to use the method that a new flag get added to direct the cli tool to use this method. Using eval feels kinda hacky so I am not sure we want to go that route. Ideally I would like to see the target flags be --nodejs, --dual-env (or similar) with the default the browser version since webpack is doing the heavy lifting for us there. I might be able to dig in on this next week. |
If we use the experimental esm support in Node, it would be interesting to demonstrate https://github.com/wasm-tool/node-loader too? |
The intention of wasm-bindgen is that the contents of the JS file, by default, are compatible with both Node and browsers. The main caveat is that neither (unfortunately) supports ESM today by default (although browsers may soon). In that sense I think this is primarily related to how JS imports are defined, right? If so @xtuc's suggestion may be a great way to go! |
That make sense @alexcrichton. I'm part of the wg for ESM in Node and indirectly for browser as well, interop between CJS and ESM is a concern there. I think that using ESM by default is the way to go since they are usable from CJS and will be default in the JS ecosystem at some point. @mbalex99 Webpack supports now ESM by default, I don't see an issue there. One issue is see is for Node without the ESM support, in that case it's necessary to transpile ESM down to CJS (using Babel?). I would be happy to help with that. |
It seems like there are two issues going on:
Doing both of these would hopefully simplify a few things and allow the library to be more independent of any specific bundler while still being fully compatible with them. It should also make documentation/getting started simpler as webpack wouldn't be required to get started but would work fine if/when the user decides to use it in their pipeline. |
@brendankenny thanks for taking the time to clarify that and I agree. Similar discussions are going on in a few other places, let's try to keep them in one place. It seems that a Webpack loader for wasm-bindgen is the way to go 😇 It might be worth going through the RFC process https://github.com/rustwasm/rfcs to have a clear idea of what to do here.
While I agree, I don't think we should drop it. That's the goal we're aiming with https://github.com/WebAssembly/esm-integration/. One thing to keep in mind is that we don't want to add a JS dependency in |
@brendankenny I totally agree with both points. Having the one unified solution without worrying about the target environment would simplify my workflow and help with adoption. I also have been thinking about how to switch on the correct wasm loading mechanism. In my experience, I have found conditional require statements to be a pain point for bundlers like webpack. What I have seen work well is producing two bundles (one for node and one for browser) and then using the target fields in package.json to help the bundler. Most shims and the wasm would be shared, only the loading mechanisms would be duplicated. Is that what you were thinking, or did you have conditional require in mind? |
I had another idea too: inline the wasm with the shims. I think the idea is not aligned with the long term goals of the project, but it might be valuable to some projects who don't care about esm modules and just want to add wasm to their project. I can imagine an "inlined" target for wasm-bindgen. Most cross-platform wasm loading concerns would be eliminated (no need to fetch or readFile) and bundlers would require zero special logic for bundling the wasm. The downside is you don't get the lazy loading, and the misalignment with project goals I wrote out above. |
@xmclark Do you have an example of using the target fields in a package.json working for this use? By inline do you mean to include the bytes as a variable? The wasm2es6js binary project will actually do this for you. The overall issue with that method is that by using a text encoding instead of a binary encoding you increase the total module size by about 33%. That might not be much of an issue if you are using it to compile the wasm for node and not the browser. |
@FreeMasen I don't know of one on hand. I could easily make one. Both webpack and rollup via plugin document it. As far as I know, parcel only targets browser. wasm2es6js is awesome. Thank you for showing me this. That does exactly what I was thinking. The size increase is a valid point. |
Ok this issue has been quite for quite some time now. I'm gonna go ahead and close this as the general answer for "output compatible everywhere" is "the default output of es modules plus a bundler/compiler step to work with other npm deps". |
Inlining the .wasm file may support both Browser and Node.js. |
How do folks address this problem today? Webpack v5 with webassembly doesn't work with packaging for under anymore:
Are folks solving this problem of publishing a universal package another way? |
In the main README.md, it states:
But it seems like the ES module can't be consumed by node directly. Like I can't do this:
However the
--nodejs
flag for the compiler allows this to work seamlessly. Do we need to use webpack for both browser setup and node setup with the default es6 modules?Ideally, it would be fine if the default usage in the browser would need webpack, but what is the ideal build setup for using it in nodejs without webpack?
The text was updated successfully, but these errors were encountered: