Skip to content

Conversation

@walkingeyerobot
Copy link

Adds --target emscripten option, which will output JS that Emscripten can consume and integrate into its own JS.

This is still a work-in-progress, but it's getting kind of beefy, so I thought it would be best to start review on this soon. There's still feature work to be done (i.e. I don't think futures work). My preference would be to submit this with only the current set of supporting features, mark --target emscripten as experimental, and then send smaller PRs to add more support.

Also adds a basic test. This test builds some rust code and links it with Emscripten, but the test runner invokes wasm-bindgen. It then tests to make sure the output looks reasonable. Normally, the wasm-bindgen cli tool is invoked by Emscripten at link time. Because cargo does not support binary dependencies, and because wasm-bindgen is effectively being used as a dependency of Emscripten, it's very difficult to do end-to-end testing in the wasm-bindgen repository. I'd like to add more comprehensive testing, but I think it's more appropriate to either Emscripten or to a new repository separate from wasm-bindgen and Emscripten that can depend on both of those repositories at HEAD.

It turns out wasm-bindgen also does not support Emscripten exceptions or PIC, so we've disabled both of those compiler options. This is mostly due to lack of features in the interpreter. Exception support is something I'd like to eventually add. PIC appears to be off by default for wasm32-unknown-unknown and on by default for wasm32-unknown-emscripten. PIC can be supported through changes to wasm-bindgen's interpreter, but that seemed a bit out of scope for this PR.

I've also made a corresponding PR to Emscripten: emscripten-core/emscripten#23493

I've also made a PR to wasm-pack: drager/wasm-pack#1469. However, there has been no activity in wasm-pack by maintainers for ~4 months, and I suspect it's unlikely to get attention in the near future. The purpose of the wasm-pack PR is to provide a more straightforward workflow for Rust users already familiar with targeting Wasm.

walkingeyerobot and others added 12 commits February 12, 2025 12:13
Merge the two generated .js into one library_bindgen.js.
Notably, to work with emscripten, the emitted js has explicit
dependencies listed for the imports.
Expand emscripten support to supporting JS imported closures.
Because the generated .js file when targeting emscripten in wasm-bindgen is meant to be
consumed by emscripten rather than a standalone executable, we need some
custom testing logic for emscripten.
Add the necessary emscripten test mode.
@google-yfyang google-yfyang force-pushed the main branch 2 times, most recently from e02edbe to 79c884a Compare March 4, 2025 20:39
walkingeyerobot and others added 6 commits March 14, 2025 18:08
Reference test goldens are updated to match the change with
isLikeNone() as well as some minor updates with an extra new line at the
beginning of some functions.
@guybedford
Copy link
Contributor

@walkingeyerobot this is a really interesting initiative, thanks for your patience. I would love to review it further soon.

@walkingeyerobot
Copy link
Author

Thanks very much! I appreciate your time and I'm happy to answer any questions and address any concerns you might have.

@@ -281,12 +281,12 @@ impl ToTokens for ast::Struct {
let ptr = #wasm_bindgen::convert::IntoWasmAbi::into_abi(value);

#[link(wasm_import_module = "__wbindgen_placeholder__")]
#[cfg(all(target_arch = "wasm32", any(target_os = "unknown", target_os = "none")))]
#[cfg(all(target_arch = "wasm32", any(target_os = "unknown", target_os = "none", target_os = "emscripten")))]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd say we should change all these checks to just target_arch = "wasm32", no target_os.

Excluding Emscripten used to be the primary reason those were added, and we want to experiment with WASI too, so it doesn't make sense to have an allowlist that lists every possible Wasm OS at this point.

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

Successfully merging this pull request may close these issues.

5 participants