-
Notifications
You must be signed in to change notification settings - Fork 95
Remove wasm workaround #1060
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
base: main
Are you sure you want to change the base?
Remove wasm 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
| }, | ||
| "./web/*": "./stylua.web/*", | ||
| "./stylua_lib_bg.wasm": "./stylua.web/stylua_lib_bg.wasm", | ||
| "./stylua_lib_bg.wasm": "./stylua_lib_bg.wasm", |
Copilot
AI
Jan 20, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The export path "./stylua_lib_bg.wasm" has changed from "./stylua.web/stylua_lib_bg.wasm", which is a breaking change. The build script copies from stylua.bundler to the current directory (line 24), so this file should exist, but consumers using the old path will break.
| "./stylua_lib_bg.wasm": "./stylua_lib_bg.wasm", | |
| "./stylua_lib_bg.wasm": "./stylua_lib_bg.wasm", | |
| "./stylua.web/stylua_lib_bg.wasm": "./stylua_lib_bg.wasm", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe that if someone uses files under the /web entry, any errors they encounter will be obvious and easy to fix. Therefore, the breaking changes here are considered expected.
| # install wasm-bindgen if needed | ||
| command -v wasm-bindgen || cargo install wasm-bindgen-cli |
Copilot
AI
Jan 20, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 |
No description provided.