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

[FR] Making bundler mode working #7

Closed
aminya opened this issue Jul 29, 2020 · 10 comments
Closed

[FR] Making bundler mode working #7

aminya opened this issue Jul 29, 2020 · 10 comments

Comments

@aminya
Copy link

aminya commented Jul 29, 2020

I added the bundler option in #6. To make budnler mode work, I had to also use "@rollup/plugin-wasm". However, I get an error using the two examples for importing Cargo.toml.

[!] Error: 'default' is not exported by target\wasm-pack\hello_world\hello_world_rs.js, imported by Cargo.toml
https://rollupjs.org/guide/en/#error-name-is-not-exported-by-module
Cargo.toml (2:27)
1:
2:                     import init from "./target/wasm-pack/hello_world/hello_world_rs.js";
                              ^
3:
4:                     init("hello_world.wasm").catch(console.error);
Error: 'default' is not exported by target\wasm-pack\hello_world\hello_world_rs.js, imported by Cargo.toml
    at error (C:\Users\hello_world\node_modules\rollup\dist\shared\rollup.js:5174:30)
    at Module.error (C:\Users\hello_world\node_modules\rollup\dist\shared\rollup.js:9629:16)
    at handleMissingExport (C:\Users\hello_world\node_modules\rollup\dist\shared\rollup.js:9551:28)
    at Module.traceVariable (C:\Users\hello_world\node_modules\rollup\dist\shared\rollup.js:10019:24)
    at ModuleScope.findVariable (C:\Users\hello_world\node_modules\rollup\dist\shared\rollup.js:8573:39)
    at Identifier$1.bind (C:\Users\hello_world\node_modules\rollup\dist\shared\rollup.js:4152:40)
    at CallExpression$1.bind (C:\Users\hello_world\node_modules\rollup\dist\shared\rollup.js:2880:23)
    at CallExpression$1.bind (C:\Users\hello_world\node_modules\rollup\dist\shared\rollup.js:6532:15)
    at MemberExpression.bind (C:\Users\hello_world\node_modules\rollup\dist\shared\rollup.js:2880:23)
    at MemberExpression.bind (C:\Users\hello_world\node_modules\rollup\dist\shared\rollup.js:6357:19)

npm ERR! code ELIFECYCLE
npm ERR! errno 1
npm ERR! @ js: `rollup -c`
npm ERR! Exit status 1
npm ERR!
npm ERR! Failed at the @ js script.
npm ERR! This is probably not a problem with npm. There is likely additional logging output above.

npm ERR! A complete log of this run can be found in:
@Pauan
Copy link
Collaborator

Pauan commented Jul 29, 2020

bundler mode cannot work, since it only works with Webpack. That's why this plugin uses web mode. Why are you trying to use bundler mode?

You do not need to use @rollup/plugin-wasm, this plugin handles everything for you, so you only need this plugin and nothing else.

Also, your import is incorrect, you are supposed to import the Cargo.toml file, like this:

import init from "./path/to/Cargo.toml";

You should never import any of the files inside of the target folder.

@aminya
Copy link
Author

aminya commented Jul 29, 2020

My application runs in Nodejs. The web mode does not work in Node. The bundler or nodejs work. For example, fetch is not defined in Node.

@Pauan
Copy link
Collaborator

Pauan commented Jul 29, 2020

For example, fetch is not defined in Node.

Ah, yes, that's a good point. I should change it so that when the format is cjs it will use readFile instead of fetch.

@aminya
Copy link
Author

aminya commented Jul 29, 2020

#6 is necessary for that so we know which target the user wants. I am working on fixing the imports. After that, we can fully support the bundler mode.

@Pauan
Copy link
Collaborator

Pauan commented Jul 29, 2020

@aminya bundler mode cannot be supported, period. It only works with Webpack, it cannot work with any other bundler. I do know what I'm talking about.

As for Node support, that is unrelated to --target nodejs in wasm-pack (which also will not work with Rollup). The fix for that is to properly support output.format: "cjs" (which I am working on).

In the meantime, you can use inlineWasm: true, which does work in Node.

@Pauan
Copy link
Collaborator

Pauan commented Jul 29, 2020

Unfortunately Rollup doesn't allow for checking output.format, so instead I just added a nodejs: true option.

@xtuc Can I get commit writes again, so I can push the changes?

@aminya
Copy link
Author

aminya commented Jul 29, 2020

@aminya bundler mode cannot be supported, period. It only works with Webpack, it cannot work with any other bundler. I do know what I'm talking about.

Could you put some lights on the reasons and disclose them, so we can fix them? We can contribute to wasm-pack for fixing that. I am interested to fix this issue once for Rollup.

In a simple example, wasm-pack generates the following code in bundler mode. This is just a simple JavaScript wrapper for importing the exports of the wasm file. So Rollup should find a way to instantiate wasm and load the code.

import * as wasm from './index_bg.wasm';

/**
* @param {number} a
* @param {number} b
* @returns {number}
*/
export function add(a, b) {
    var ret = wasm.add(a, b);
    return ret >>> 0;
}

let cachegetFloat64Memory0 = null;
function getFloat64Memory0() {
    if (cachegetFloat64Memory0 === null || cachegetFloat64Memory0.buffer !== wasm.memory.buffer) {
        cachegetFloat64Memory0 = new Float64Array(wasm.memory.buffer);
    }
    return cachegetFloat64Memory0;
}

let WASM_VECTOR_LEN = 0;

function passArrayF64ToWasm0(arg, malloc) {
    const ptr = malloc(arg.length * 8);
    getFloat64Memory0().set(arg, ptr / 8);
    WASM_VECTOR_LEN = arg.length;
    return ptr;
}
/**
* @param {Float64Array} arr
* @returns {number}
*/
export function sum(arr) {
    var ptr0 = passArrayF64ToWasm0(arr, wasm.__wbindgen_malloc);
    var len0 = WASM_VECTOR_LEN;
    var ret = wasm.sum(ptr0, len0);
    return ret;
}

cc: @xtuc

@Pauan
Copy link
Collaborator

Pauan commented Jul 30, 2020

@xtuc Thanks!

@aminya I just published version 1.0.4 which adds in a nodejs: true option, so it now works with Node.

The reason why --target bundler does not work is not because of wasm-pack, it is because of Rollup. In the generated code, notice how it imports the .wasm file:

import * as wasm from './index_bg.wasm';

The ability to import .wasm files is an experimental feature called esm-integration. Since it is not yet standardized, only Webpack supports it (and you have to enable experimental flags in order to use it in Webpack). No other bundler supports it, including Rollup. The @rollup/plugin-wasm plugin does not support esm-integration, it is different (so it does not work with --target bundler).

On the other hand, --target nodejs is designed to be used in Node without a bundler, it is standalone (just like no-modules). If you tried to use it with Rollup, it would not work because it uses require, but Rollup only supports ES6 modules. So you would need to use @rollup/plugin-commonjs, and even then it still wouldn't work, because it uses String.raw. It simply was not designed for usage in bundlers.

Using --target no-modules does work, but then you cannot import JS files using module or inline_js, and it pollutes the global scope.

However, --target web works perfectly. It supports importing JS files, and it only uses standardized features, so it works everywhere (in the browser, in Node, and in all bundlers). Using --target web is the current best way to use wasm-bindgen with bundlers (except for Webpack, which should use --target bundler).

That's why I said that it's pointless to support a target option, because the only target that works with Rollup is web.

The same goes for --out-name and --scope. They are useful when using wasm-pack by itself, but they don't make sense with Rollup.

@Pauan
Copy link
Collaborator

Pauan commented Aug 2, 2020

Thanks for reporting the issue with Node, since it has been fixed, I'm going to close this.

@Pauan Pauan closed this as completed Aug 2, 2020
@aminya
Copy link
Author

aminya commented Aug 2, 2020

This is an issue in Rollup. Once this is solved we will get back to this.
rollup/rollup#2099

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants