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

build: Replace wasm-pack #2881

Closed
wants to merge 1 commit into from
Closed

Conversation

max-sixty
Copy link
Member

@max-sixty max-sixty commented Jun 20, 2023

This is the beginning of #1836, replacing wasm-pack with https://crates.io/crates/substrate-wasm-builder. wasm-pack requires additional binaries to be installed outside of the normal cargo build process, has been the source of lots of issues, makes builds slower — in particular small code changes cause a very long iteration cycle in playground builds — and is only lightly maintained now.

Using wasm-builder is a bit harder than I thought:

  • We need to create the paths bundler, node, web paths ourselves, replacing these lines, by calling wasm-bindgen debug/wbuild/prql-js/prql_js.wasm --target=....
  • We probably need some way of getting the .wasm file location, by reading include!(concat!(env!("OUT_DIR"), "/wasm_binary.rs")); for the WASM_BINARY or WASM_BINARY_BLOATY constants. Ideally we'd do this at build time, all in the build.rs script; I'm not sure whether that's possible though.
  • Possibly I'm figuring too much of this out on my own and it's done elsewhere? The precedent for this approach seems to be blockchain rather than JS applications... I'll ask around to see if I'm missing anything.

This is the beginning of PRQL#1836, replacing `wasm-pack` with https://crates.io/crates/substrate-wasm-builder. `wasm-pack` has been the source of lots of issues, makes builds slower -- in particular small code changes cause a very long iteration cycle in playground builds -- and is no longer maintanied.

It's a bit harder than I thought:
- We need to create the paths `bundler`, `node`, `web` paths ourselves, replacing [these lines](https://github.com/PRQL/prql/blob/5db61f45ad31ff5673fdfb7890cc3699d47940f9/bindings/prql-js/package.json#L20-L21), by calling `wasm-bindgen debug/wbuild/prql-js/prql_js.wasm --target=...`.
- We probably need some way of getting the `.wasm` file location, by reading `include!(concat!(env!("OUT_DIR"), "/wasm_binary.rs"));` for the `WASM_BINARY` or `WASM_BINARY_BLOATY` constants. Ideally we'd do this at build time, all in the `build.rs` script; I'm not sure whether that's possible though.
- Possibly I'm figuring too much of this out on my own and it's done elsewhere? The precedent for this approach seems to be blockchain rather than JS applications...
@max-sixty
Copy link
Member Author

Postponing given context at #1836, somewhat replaced by #2907

@max-sixty max-sixty closed this Jun 22, 2023
@max-sixty max-sixty deleted the wasm-builder branch October 12, 2023 04:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant