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

JS bindings don't work when there are dependencies on wasm bindgen #700

Open
jcrist1 opened this issue Sep 29, 2024 · 4 comments
Open

JS bindings don't work when there are dependencies on wasm bindgen #700

jcrist1 opened this issue Sep 29, 2024 · 4 comments

Comments

@jcrist1
Copy link
Contributor

jcrist1 commented Sep 29, 2024

Problem

I ran into an issue while using diplomat to create js bindings for a library I'm working on. The library has dependencies which themselves seem to depend on wasm bindgen. The generated wasm expects interfaces provided by wasm_bindgen in the form of

[TypeError: WebAssembly.instantiate(): Import #0 module="__wbindgen_placeholder__" error: module is not an object or function]

I created a minimal reproducible example here: https://github.com/jcrist1/diplomat-dbg/tree/6649bacd8e004f82814510774acde572625be891/wasm_bindgen

Workaround

I found a relatively easy workaround, but did have to spend a lot of time debugging to figure it out. Just needed to run wasm-bindgen on the generated wasm, use that for the module, and add a line to imports in diplomat-wasm.mjs:

import cfg from "../diplomat.config.mjs";
import { readString8 } from "./diplomat-runtime.mjs";
import * as dbg_bg from "./dbg_bg.js";  // <- this line

let wasm;
const imports = {
  "./dbg_bg.js": dbg_bg, //  <-- and this line
  env: {
    diplomat_console_debug_js(ptr, len) {
      console.debug(readString8(wasm, ptr, len));
    },
    diplomat_console_error_js(ptr, len) {
      console.error(readString8(wasm, ptr, len));
    },
    diplomat_console_info_js(ptr, len) {
      console.info(readString8(wasm, ptr, len));
    },
    diplomat_console_log_js(ptr, len) {
      console.log(readString8(wasm, ptr, len));
    },
    diplomat_console_warn_js(ptr, len) {
      console.warn(readString8(wasm, ptr, len));
    },
    diplomat_throw_error_js(ptr, len) {
      throw new Error(readString8(wasm, ptr, len));
    },
  },
};

Proposals:

wasm-bindgen seems pretty embedded in a lot of crates that support wasm, and is a big part of the rust wasm ecosystem, so some kind of support would be nice, but it feels like directly integrating with wasm-bindgen doesn't seem like it aligns with diplomat's goals. I came up with

  1. Add documentation on how to use wasm bindgen and manually modify the generated files when libraries depend on it
  2. Require any extra imports be specified in diplomat.config.mjs, and include their use in the generated diplomat-wasm.mjs, but don't automatically generate bindgen (i.e. generating {lib}_bg.js) (and document using wasm-bindgen)
  3. Directly integrate wasm-bindgen, as part of some kind of diplomat tool

I hope 1. would not be too much, but I would prefer 2. I don't think 3. makes sense as dipomat doesn't seem to do really do anything to the compile step.

@Manishearth
Copy link
Contributor

and include their use in the generated diplomat-wasm.mjs, but don't automatically generate bindgen (i.e. generating {lib}_bg.js) (and document using wasm-bindgen)

Can you expand on this more? What do you mean by "don't automatically generate bindgen", would that mean not doing anything we are doing now?

I'm generally okay with a way to config that "hey there's wasm-bindgen here, do the thing to make wasm-bindgen work" if it's just a matter of configuring an additional import. I don't want to lose functionality we have right now.

I'm not in favor of calling wasm-bindgen, but you aren't proposing that.

@jcrist1
Copy link
Contributor Author

jcrist1 commented Oct 7, 2024

Sorry was unable to sit down at a computer for a bit. With

automatically generate bindgen

I meant that it might be possible to somehow integrate wasm-bindgen directly into diplomat to identify what binding functions are needed for the library to work. I can't, however, see how we could do it without using the compiled wasm in the first place, as we would somehow have to introspect which dependencies expect some bindgen generated binding functions. And it doesn't feel very um ... diplomatic (for lack of a better word) to process the compiled library for generating binding code.

I think what's bothering me more is that it feels like wasm-bindgen and diplomat are doing the same work twice sometimes, like this bindgen generated code for decoding a string:

function getStringFromWasm0(ptr, len) {
    ptr = ptr >>> 0;
    return cachedTextDecoder.decode(getUint8ArrayMemory0().subarray(ptr, ptr + len));
}

vs diplomat's

    getValue() {
        switch (this.bufferType) {
            case Uint8Array:
                return this.#decoder.decode(super.getValue());
            case Uint16Array:
                return String.fromCharCode.apply(null, super.getValue());
            default:
                return null;
        }
    }

In a perfect world it would be nice to figure out how to avoid that, but I think this would require way too much coordination with wasm-bindgen.

@jcrist1
Copy link
Contributor Author

jcrist1 commented Oct 7, 2024

In any case, that was a big aside to say that some small changes to the config and some additions to the documentation will get us working with wasm-bindgen dependent libraries.

@Manishearth
Copy link
Contributor

I think what's bothering me more is that it feels like wasm-bindgen and diplomat are doing the same work twice sometimes, like this bindgen generated code for decoding a string:

This seems like very minor duplication IMO.

I meant that it might be possible to somehow integrate wasm-bindgen directly into diplomat to identify what binding functions are needed for the library to work.

Yeah, that seems like it would be much deeper integration than we are looking for. Ways for the init layer to mesh together seem sufficient.

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