wasm plugin shouldn’t use base64 unless a file is marked as sync

[surma]

• Rollup Plugin Name: @rollup/plugin-wasm
• Rollup Plugin Version: 3.0.0
• Rollup Version: 2.0
• Operating System (or Browser): Chrome on MacOS
• Node Version: v12.8

How Do We Reproduce?

https://repl.it/@surma/SaneCarelessSphere

Expected Behavior

The .wasm file should be emitted as an asset and loaded using fetch() when the .wasm file is not marked as `sync.

Actual Behavior

The .wasm file is inlined using base64, causing unnecessary file bloat.

I am opening an issue first to see if y’all would accept an PR to change the behavior of this plugin. Either me or @RReverser would be happy to whip one up :D

[shellscape]

The .wasm file should be emitted as an asset and loaded using fetch() when the .wasm file is not marked as `sync.

That sounds like a browser environment. When working on a patch for this, it'll be important to consider non-browser environments.

[surma]

Sure, I wouldn’t break node support. Although it might be preferrable to make the target environment an option of the plugin to avoid loading unnecessary code on the web.

[RReverser]

Note that plugin runtime code already performs different actions for Node.js vs browser, so it would be quite easy to perform fetch + instantiateStreaming only in browser:

plugins/packages/wasm/src/index.js

Line 30 in e9651fb

if (isNode) {

[manzt]

I'm curious if folks here have thoughts around best practices with shipping wasm in a library. I've found base64 encoding to be very useful for making a wasm-based module npm-installable and easy to handle with bundlers. However, I'm not happy with using base64 since it adds bloat and feels like a win for DX and not users ultimately. Apologies if not totally related, but a real "benefit" of base64 encoding means that the bundles produces are easy to npm install and use in other applications.

[RReverser]

I don't really see why separate Wasm files are any harder to npm install or use in other apps?

[manzt]

Perhaps my nativity is showing here, but I'm not sure if something like @rollup/plugin-node-resolve can will end up adding the wasm from an npm package as an asset in the final bundle.

library code using rollup-plugin-wasm

// myLib.js
import wasm from './sample.wasm';

async function init(){
  const { instance } = await wasm({ ...imports });
  return instance;
}

export init;

module on npm

// myBundledLib.js
// rollup-plugin-wasm generated code (drastically simplified)
async function init() {
  const { instance }  = await WebAssembly.instantiateStreaming(
    fetch('sample.wasm'), 
    importObject
  );
  return instance 
}

export init;

module usage in another project

// my-lib.js
import { init } from ''myBundledLib";
init(); // how does the binary asset end up in this bundle?

In my experience, the wasm asset doesn't end up in the final bundle when using @rollup/plugin-node-resolve. Am I missing something? (likely).

EDIT: I updated the example (with "sample" library code) to illustrate my point.

[RReverser]

I'm a bit confused too, to be honest. What is the sample source here?

This issue is about the Rollup Wasm plugin that handles things like import wasm from './sample.wasm';, but I don't see that in either of samples above?

The idea is that Rollup will use emitFile like it does for other binary assets, which takes care of emitting them into a preconfigured folder (either alongside JS or elsewhere), then resolving URLs to the emitted assets, and using either fetch in the browser or fs APIs in Node to load it asynchronously.

Not sure if it makes sense? 😀

[manzt]

Makes total sense! Thanks for the detailed description. I was trying to (unsuccessfully) make the point that as a consumer of a library bundled with @rollup/plugin-wasm, it is convenient to have the binary base64 encoded as I'm not sure how something like @rollup/plugin-node-resolve handles including binary assests from an npm dependency in the comsumer's bundle.

[RReverser]

Ah I see, thanks for clarification. I think it should work, but I have no idea how Rollup resolves references to emitFile-produced binaries in Node.js.

[stale]

Hey folks. This issue hasn't received any traction for 60 days, so we're going to close this for housekeeping. If this is still an ongoing issue, please do consider contributing a Pull Request to resolve it. Further discussion is always welcome even with the issue closed. If anything actionable is posted in the comments, we'll consider reopening it. ⓘ

[bminixhofer]

Hi, is there still interest to implement this @RReverser @surma ?

I would need it for bminixhofer/tractjs#12. I see a 36% size increase when inlining the WASM. I think anyone who develops for the browser using WASM and Rollup needs this option. A > 30% size increase is rarely acceptable.

I could give this a go if you are not planning to implement it anymore, but I currently don't know the internals of Rollup so I would be grateful if you could do it 😃

[RReverser]

This is absolutely still an important issue that shouldn't have been closed as stale. If you can implement it, give it a go!

[shellscape]

I'm willing to reopen this if someone opens a PR to address it. It previously went over two months without anyone giving it attention, which is why it was closed automatically by the bot. We simply don't have the manpower to address all issues, so we welcome community contribution. If anyone would like to step up as lead for the wasm plugin we'd also welcome that.

[bminixhofer]

Understandable. I'll try implementing this and open a PR if I get it to work.

[RReverser]

We simply don't have the manpower to address all issues, so we welcome community contribution.

All the more reason to keep issues open, so that community could easily see what work is needed or which bugs are known, and help to address them.

[shellscape]

All the more reason to keep issues open, so that community could easily see what work is needed or which bugs are known, and help to address them

We disagree there. That methodology was tried and failed in both the previously existing individual plugin repos and the main rollup repo. What we ended up with was a 4 year backlog of hundreds of issues that no one had touched in eons, some of which were out of sync with current versions. The optics of a project with pages upon pages of open issues is discouraging to potential contributors and users alike. You're of course welcome to disagree, but we've been at this a long time, and that's how we've chosen to manage the issues.

[shellscape]

Big thanks to @bminixhofer for stepping up with a PR

[stale]

Hey folks. This issue hasn't received any traction for 60 days, so we're going to close this for housekeeping. If this is still an ongoing issue, please do consider contributing a Pull Request to resolve it. Further discussion is always welcome even with the issue closed. If anything actionable is posted in the comments, we'll consider reopening it. ⓘ