wasm-bindgen icon indicating copy to clipboard operation
wasm-bindgen copied to clipboard

Feat: add support for Cloudflare Workers via `--workerd` option

Open jkomyno opened this issue 2 years ago • 14 comments

Currently, the bindings generated by wasm-bindgen cannot be used on Cloudflare Workers out of the box (see "JavaScript Plumbing" section in the docs). This is a pain for developers, as shown in https://github.com/rustwasm/wasm-bindgen/issues/2972.

This PR proposes a new --workerd option for --target bundler that closes https://github.com/rustwasm/wasm-bindgen/issues/2972.

Tests and markdown documentation updates are included.

Example

Consider a wasm32-unknown-unknown crate called cf-worker. The current cf_worker.js binding generated by wasm-bindgen --target bundler is:

import * as wasm from "./cf_worker_bg.wasm";
import { __wbg_set_wasm } from "./cf_worker_bg.js";
__wbg_set_wasm(wasm);
export * from "./cf_worker_bg.js";

This PR, following Cloudflare's own documentation and using wasm-bindgen --target bundler --workerd, creates the following binding instead:

import * as imports from "./cf_worker_bg.js";
import wkmod from "./cf_worker_bg.wasm";
const instance = new WebAssembly.Instance(wkmod, { "./cf_worker_bg.js": imports });
imports.__wbg_set_wasm(instance.exports);
export * from "./cf_worker_bg.js";

which is indeed compatible with Cloudflare Workers.

jkomyno avatar Jul 06 '23 19:07 jkomyno

Unfortunately I know nothing about bundlers, so I will need some help understanding this.

Considering this PR doesn't actually touch the JS shim, only the final bundler file (the JS file without _bg), can't users not add this on their own before using the bundler? It seems straightforward to me, it's basically like users having to add the index.html file when using the web target.

I think I already addressed the original painpoint in https://github.com/rustwasm/wasm-bindgen/issues/2972#issuecomment-1625180501.

daxpedda avatar Jul 07 '23 10:07 daxpedda

I guess one question I have is what would accomplishing the same thing look like from the user's perspective without this PR? Like, what steps would they have to take?

asimpletune avatar Jul 07 '23 10:07 asimpletune

Like, what steps would they have to take?

Following the proposed solution of this PR, the only step the user would have to take is replace the output file with this:

import * as imports from "./cf_worker_bg.js";
import wkmod from "./cf_worker_bg.wasm";
const instance = new WebAssembly.Instance(wkmod, { "./cf_worker_bg.js": imports });
imports.__wbg_set_wasm(instance.exports);
export * from "./cf_worker_bg.js";

daxpedda avatar Jul 07 '23 10:07 daxpedda

Some considerations for @asimpletune and @daxpedda:

this PR doesn't actually touch the JS shim

That's correct. Cloudflare Workers only require changes to the (very small) bundler file.

the only step the user would have to take is replace the output file with this

import * as imports from "./cf_worker_bg.js";
import wkmod from "./cf_worker_bg.wasm";
const instance = new WebAssembly.Instance(wkmod, { "./cf_worker_bg.js": imports });
imports.__wbg_set_wasm(instance.exports);
export * from "./cf_worker_bg.js";

Essentially, yes, keeping in mind that cf_worker is an example name and not a constant string. Let's call the file above bundler-template.js and refer to the variable shim filename (without the postfix _bg.js part) as $WASM_SHIM_NAME. In this example, WASM_SHIM_NAME=cf_worker.

what would accomplishing the same thing look like from the user's perspective without this PR?

  1. run wasm-bingen --target bundler
  2. in the output folder, replace $WASM_SHIM_NAME.js's content with bundler-template.js' content
  3. run a :s/cf_worker/$WASM_SHIM_NAME/g search&replace command on $WASM_SHIM_NAME.js

However, I'd like to point out that:

  • Cloudflare Workers explicit refer to wasm-bindgen in their documentation
  • Cloudflare Workers are one of the major platforms for deploying Wasm modules online in a serverless fashion
  • I think Cloudflare Workers are big enough to justify a first-class support (which, as you can see, doesn't require major changes in wasm-bindgen's source code) without having to resort to third-party utils / Wasm binding patch scripts.

This PR is an effort to make the task of deploying Rust+Wasm modules online more user-friendly :)

jkomyno avatar Jul 07 '23 11:07 jkomyno

  • Cloudflare Workers explicit refer to wasm-bindgen in their documentation

Not sure why this justifies us supporting it. Rather this is an issue on their side.

  • Cloudflare Workers are one of the major platforms for deploying Wasm modules online in a serverless fashion

True.

  • I think Cloudflare Workers are big enough to justify a first-class support (which, as you can see, doesn't require major changes in wasm-bindgen's source code) without having to resort to third-party utils / Wasm binding patch scripts.

I don't disagree, but on the other hand it would be better if they provide support when they are doing non-standard stuff.

Case in point: worker-build. From personal experience I know that Cloudflare itself doesn't maintain it very well, but it seems better to me to let them support it as long as we provide the tools necessary to do so.

I think it's gonna be problematic if the JS shim needs adjustment, then I would lean in favor of providing better support, but if it's only the bundle file, I think some effort from Cloudflare's side is appropriate.

I'm currently against merging this PR:

  • The web target already works as intended.
  • The bundle target can be used by modifying/fixing the bundler file.
  • A dedicated tool for Cloudflare Workers already exists: worker-build. So this should be out of our hands to begin with, aside from making sure we continue to support such tools.

I also want to note here that this problem in general has been a big issue in wasm-bindgen. The Web is pretty diverse and has a lot of very different tools. wasm-bindgen already supports a bunch of tools I personally have no clue about and it's already unmaintainable as-is, widening the scope even further is not really a good idea imo.

Instead we should push further towards making wasm-bindgen as agnostic as possible so other tools can be built on top of it to support all these diverse needs. worker-build is a perfect example on how we should handle this imho.

daxpedda avatar Jul 07 '23 14:07 daxpedda

Thanks for your feedback @daxpedda. I see you're heavily hinting at moving this PR towards the worker-build tool. To shed additional context on this - Cloudflare Workers support WebAssembly in two different scenarios:

  1. When defining workers in JavaScript/TypeScript, you can import Wasm modules (which, referencing this PR, requires wasm-bindgen --target bundler --workerd)
  2. When defining workers purely in Rust via workers-rs, to compile to Wasm them via worker-build and upload them.

Afaik, the worker-build tool is only useful in this latter case. This PR addresses the first case.

I'll ask people from Cloudflare to chime in - whether this PR's logic ships in wasm-bindgen or in Cloudflare's own infrastructure is rather indifferent to me, as long as all Cloudflare Workers devs can avoid an additional "glue code patching" step (as it's currently the case).

jkomyno avatar Jul 07 '23 16:07 jkomyno

  1. When defining workers in JavaScript/TypeScript, you can import Wasm modules (which, referencing this PR, requires wasm-bindgen --target bundler --workerd)

That indeed would make sense to me then.

I'll ask people from Cloudflare to chime in - whether this PR's logic ships in wasm-bindgen or in Cloudflare's own infrastructure is rather indifferent to me, as long as all Cloudflare Workers devs can avoid an additional "glue code patching" step (as it's currently the case).

Great, thank you!

daxpedda avatar Jul 07 '23 16:07 daxpedda

I'm not a fan of this. What about environments other than workerd? Cloudflare isn't the only one providing a worker environment where wasm-bindgen generated code will run. Would we be supporting every environment with its own flag, as needed? I don't think that's a good idea

Fixing #2972 isn't a bad thing. But I don't think this is the way to do this. One solution for #2972 could be to let init function take the WebAssembly.Module

ranile avatar Jul 07 '23 21:07 ranile

Is there any updates on this?

I'm able to run the tests on firefox with wasm_bindgen_test_configure!(run_in_worker); or wasm_bindgen_test_configure!(run_in_browser);

But I wanted to run at least some of them on the workerd, because of the workerd extra stuff, like D1.

Is there a way for the runner to load them on workerd?

I believe that one way or another, this goes through either forking your runner or improving your runner to handle it.

spigaz avatar Nov 30 '23 15:11 spigaz

I think both @hamza1311 and I covered how we want to proceed in previous posts. If anything is unclear let us know.

daxpedda avatar Nov 30 '23 15:11 daxpedda

Sorry @daxpedda, I have read several times your comments, and they only talk about producing final files ready to be deployed to workerd.

And to be honest I understand perfectly your reasoning.

But using wasm-bindgen produced test files to run the tests with your test runner using workerd is a bit of a different use case.

The only thing that I can infer from them, regarding my question, is that your point of view is that CF should also provide their own test runner.

Like cargo wasix or webassembly_test that target wasmtime.

I understand that, that's okay.

I'm willing to pitch in, so its relevant for me to know what makes sense to invest my time in.

Thank you!

spigaz avatar Nov 30 '23 18:11 spigaz

Apologies, you are right your case is different, I didn't pay enough attention.

I think the same philosophy applies to the test runner as well. The preference here it not to have to cover every use case there is in wasm-bindgen. So as you said, CF should provide their own test runner.

That said, if the wasm-bindgen test runner can be adjusted to provide a general way for other runtimes/platforms to inject what they need to get things to work, that would be great as well. That would obviously require some design work, but considering the lack of time from maintainers this has to be done in small steps that can be easily discussed and reviewed.

daxpedda avatar Nov 30 '23 18:11 daxpedda

I'm already in contact with someone from CF to see how this can be done, his comment was that my approach seemed reasonable.

But at the moment it seems they don't export enough yet to run the tests, but this is something that I do require, so I'm going to start experimenting soon anyway.

I'll update you when I have relevant info.

spigaz avatar Dec 01 '23 15:12 spigaz

using wasm-bindgen produced test files to run the tests with your test runner using workerd is a bit of a different use case.

That is a use case I would love to support. There's also a point to be made of running wasm-bindgen output in non-node.js environments like Deno or Bun. I think this PR is out of scope for this discussion. Can you create a new issue for this so we can discuss how to handle this? I think we can take inspiration from the Jasmine testing framework in the JS ecosystem

ranile avatar Dec 01 '23 16:12 ranile

I'm going to close this due to inactivity.

The request is continuing to be tracked in https://github.com/rustwasm/wasm-bindgen/issues/3891.

daxpedda avatar Mar 15 '24 07:03 daxpedda