Remove WASM build workaround#1060
Conversation
|
Great news from wasm-bindgen 0.2.108: It now generates identical WASM binaries across all targets. This allows us to build each target independently while reusing a single shared WASM file, and simply collect the generated JavaScript bindings per target. As a result, no workaround or ad-hoc hacks are required. |
There was a problem hiding this comment.
Pull request overview
This pull request removes WASM workaround files and modernizes the build process by updating wasm-bindgen from 0.2.81 to 0.2.108. The changes eliminate custom bundler workarounds in favor of using wasm-bindgen's native target support directly.
Changes:
- Updates wasm-bindgen dependency from 0.2.81 to 0.2.108
- Removes custom workaround files (stylua_lib_bundler.js, stylua_lib.mjs, etc.) and replaces them with direct wasm-bindgen output
- Rewrites build-wasm.sh to use wasm-bindgen CLI with multiple targets (bundler, nodejs, deno, web) instead of wasm-pack
- Updates package.json exports to reference new generated file paths and removes browser field workaround
Reviewed changes
Copilot reviewed 10 out of 12 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| wasm/stylua_lib_bundler_wbg.cjs | Removed workaround file for bundler usage |
| wasm/stylua_lib_bundler.js | Removed custom bundler initialization wrapper |
| wasm/stylua_lib_bundler.d.ts | Removed bundler type definitions |
| wasm/stylua_lib.mjs | Removed Node.js ESM wrapper |
| wasm/stylua_lib.d.mts | Removed Node.js ESM type definitions |
| wasm/stylua_lib.d.cts | Removed Node.js CommonJS type definitions |
| wasm/src/stylua_lib_node.cjs | Removed source file for Node.js CommonJS build |
| wasm/package.json | Updated entry points, exports, and file patterns for new build output |
| wasm/build-wasm.sh | Completely rewritten to use wasm-bindgen CLI directly with multiple targets |
| wasm/.gitignore | Updated patterns to ignore new generated file structure |
| Cargo.toml | Updated wasm-bindgen dependency version |
| Cargo.lock | Updated wasm-bindgen and related dependencies |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # install wasm-bindgen if needed | ||
| command -v wasm-bindgen || cargo install wasm-bindgen-cli |
There was a problem hiding this comment.
The build script uses wasm-bindgen but doesn't ensure version compatibility with the Cargo.toml dependency version (0.2.108). The script installs wasm-bindgen-cli if not found using "cargo install wasm-bindgen-cli" without specifying a version, which could lead to version mismatches between the Rust wasm-bindgen dependency (0.2.108) and the CLI tool. This can cause build failures or runtime issues. Consider installing a specific version: "cargo install wasm-bindgen-cli --version 0.2.108".
| # install wasm-bindgen if needed | |
| command -v wasm-bindgen || cargo install wasm-bindgen-cli | |
| # install wasm-bindgen if needed, pinning the version to match Cargo.toml (0.2.108) | |
| command -v wasm-bindgen >/dev/null 2>&1 || cargo install wasm-bindgen-cli --version 0.2.108 |
89e6d82 to
64e3b9d
Compare
|
There are no Rust changes, but |
|
It's a clippy bug in latest version (rust-lang/rust#147648) and we don't pin to a specific clippy version in CI. I've pushed a commit to main to temporarily ignore it for now. You can rebase. |
JohnnyMorganz
left a comment
There was a problem hiding this comment.
Thanks for this! I took the liberty to add some long-needed smoketests just to help validate the WASM works correctly
No description provided.