Repository navigation
Infrastructure and build tooling upgrade - #537
Merged
Merged
Conversation
The previous build pipeline could not be run at all on a current Rust
toolchain. Three of its four tools failed outright:
- cargo build: `undefined symbol: mapping_callback`, because the import
blocks lacked `#[link(wasm_import_module = "env")]` and modern rust-lld
rejects undefined symbols
- wasm-gc 0.1.6: `InvalidTableReference(128)`
- wasm-nm (which drives wasm-snip): `Not an unsigned 1-bit integer: 128`
Only wasm-opt still worked, so the committed binary was not reproducible
from its own sources. CI never noticed because the `wasm` job only ran a
bare `cargo build` and never exercised build.py.
Replace build.py and who-calls.py with build.sh, which drives wasm-pack.
wasm-pack vendors its own wasm-bindgen and wasm-opt, reducing the four
hand-installed binaries to one. Two targets are built, selected by the
existing package.json#browser field: `nodejs` for CommonJS and `web` for
browsers. Both emit the same .wasm; build.sh asserts this. The shipped
filename stays lib/mappings.wasm so URLs passed to
SourceMapConsumer.initialize keep working.
Port the ABI to wasm-bindgen. allocate_mappings, parse_mappings,
free_mappings and get_last_error collapse into parseMappings() plus a
generated free(), so JS no longer hand-manages a buffer or memcpys into
exports.memory.buffer. Callbacks are passed per-call, which removes the
callbackStack and withMappingCallback machinery. Mapping construction
stays in JS to keep every mapping on one hidden class.
Note that a JS exception thrown from a mapping callback unwinds through
the wasm frame and skips Rust's borrow guard, leaving the handle
permanently borrowed so that free() then fails. The callback boundary is
therefore declared `catch` and every query method returns
Result<(), JsValue>, releasing the borrow before the exception
propagates. Any future &mut self method that calls into JS needs the
same treatment.
Also drop the unused `rand` dependency, turn off `debug` in
[profile.release] (it was responsible for 1.6 MB of debuginfo in a
shipped artifact), and set opt-level="z", lto, codegen-units=1 and
panic="abort". lib/mappings.wasm goes from 48,526 to 28,556 bytes.
Commit both Cargo.lock files, pin the toolchain in rust-toolchain.toml,
and add a `wasm-dist` CI job that rebuilds the committed artifacts and
fails if they differ byte-for-byte. Verified that the job exits non-zero
when the Rust sources drift.
Bump eslint's ecmaVersion to 2020 for the dynamic import() in
read-wasm-browser.js, and keep the generated glue out of eslint --fix and
prettier so neither rewrites it and breaks the byte comparison.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`import.meta` is a hard parse error in webpack 4 and other pre-2020
bundlers:
ERROR in ./lib/mappings-glue-browser.mjs 336:56
Module parse failed: Unexpected token (336:56)
> module_or_path = new URL('mappings.wasm', import.meta.url);
This broke `npm run build` in bench/, and would equally break any
consumer of this package that bundles with webpack 4.
The branch it appears in is unreachable here: read-wasm-browser.js
validates its input and always passes the URL or bytes explicitly, so
`module_or_path` is never undefined. Replace it at build time with a
throw carrying the same message read-wasm-browser.js uses, and extend
build.sh's post-build guard to assert no `import.meta` survives.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`lib/url.js` fell back to `require("url").URL` when `URL` was not a
global, which was only the case on Node 8. This package requires Node
>= 12 and `URL` has been global since Node 10, so the fallback was
unreachable.
It was not harmless, though: bundlers that no longer polyfill Node core
modules fail outright on it. Under webpack 5 this breaks `npm run build`
in bench/ with:
Module not found: Error: Can't resolve 'url' in '.../lib'
BREAKING CHANGE: webpack < 5 used to include polyfills for node.js
core modules by default.
The comment above it also claimed the file was overridden via
`package.json#browser` to use `lib/url-browser.js`. That stopped being
true in b95ecf0, which removed `lib/url-browser.js` without removing the
browser mapping's last reader or this comment.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
webpack 4 could not parse the `import.meta.url` that wasm-bindgen emits into the browser glue. That specific blocker is gone now that build.sh strips it, but webpack 4 is six years old and its webpack-cli 3 pairing is long unmaintained, so move the benchmark to webpack 5. The upgrade surfaced one real bug in the library, fixed separately in 64191de: webpack 5 no longer polyfills Node core modules, which exposed a dead `require("url")` fallback in lib/url.js. Commit a lockfile as well. The benchmark had none, so `npm install` here previously resolved whatever was current, which is how the webpack 4 tree drifted far enough to break in the first place. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Reverts the lockfile added in ba82cf4. The benchmark is a private, non-published helper and was never intended to pin its own dependency tree; `package.json`'s ranges are enough for it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Download the official v0.15.0 release binary and verify it against a pinned SHA-256, so the reproducibility check doesn't depend on a third-party action. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The wasm-pack-built lib/mappings.wasm uses WebAssembly multi-value and reference types, which Node.js 12, 14 and 16 cannot load. Bump engines.node to >= 18 and test on Node 18, 20 and 22 in CI. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
wasm-mappings/rust-toolchain.toml pins stable 1.98.1, and rustup prefers it over the directory override actions-rs sets on the repo root. As a result every matrix job ran on 1.98.1, and the nightly-only benches failed with "#![feature] may not be used on the stable release channel". Set RUSTUP_TOOLCHAIN from the matrix in CI, default the bench job to nightly when run locally, and correct the toolchain file's comment. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
wasm-pack uses a wasm-opt from PATH when there is one and otherwise downloads its own (binaryen 117 for wasm-pack 0.15.0). The committed lib/mappings.wasm was optimized with binaryen 132, so the wasm-dist CI job, which has no system wasm-opt, produced different bytes and failed. Make build.sh require binaryen 132 on PATH, install that release in CI (verified against a pinned SHA-256), and document the requirement in CONTRIBUTING.md. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The script relied on CI's working-directory default. Run from the repo root, the wasm-dist job failed with "./build.sh: No such file or directory", and rustup did not pick up wasm-mappings/rust-toolchain.toml. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
hbenl
approved these changes
Oct 9, 2026
|
|
||
| // Use the URL global for Node 10, and the 'url' module for Node 8. | ||
| module.exports = typeof URL === "function" ? URL : require("url").URL; | ||
| // `URL` has been a global since Node 10 and this package requires Node >= 12, |
Collaborator
There was a problem hiding this comment.
nit: it now actually requires Node >= 18
| .then(glue => | ||
| glue.default({ module_or_path: mappingsWasm }).then(() => glue) | ||
| ) | ||
| .then(null, e => { |
Collaborator
There was a problem hiding this comment.
nit: I'd prefer .catch(...) over .then(null, ...).
Collaborator
There was a problem hiding this comment.
This entire section is outdated.
Set `edition` in [package] (under [profile.release] Cargo ignored it and the crate stayed on 2015), name the anonymous parameters in ComparatorFunction::compare, and use `dyn Iterator` in the quickcheck tests. lib/mappings.wasm is byte-for-byte unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- Drop the deprecated std::u32 / std::i64 module imports. - Assign instead of discarding a mem::replace result. - Store the never-read observer as PhantomData<O>. - Spell out elided lifetimes in two return types. - Upgrade quickcheck 0.5 -> 1.1 (0.5 trips a future-incompat warning) and port the Arbitrary impls to the 1.x API. - Set edition 2024 on source-map-mappings-wasm-api, which had none. The source changes alter codegen slightly, so lib/mappings.wasm is rebuilt (28556 -> 28563 bytes) with the pinned toolchain and binaryen. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The job still rebuilds lib/mappings.wasm and its glue, but no longer fails when the result differs from the committed artifacts. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Partially undoes dd28c67: CI again fails if rebuilding from source does not reproduce the committed lib/mappings.wasm. The generated glue files remain unchecked. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Restores the byte comparison for lib/mappings-glue.js, .d.ts and the browser .mjs/.d.mts alongside lib/mappings.wasm, so ci/script.sh is back to how it was before dd28c67. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Panic locations embed absolute source paths in lib/mappings.wasm, which differ between machines (checkout location, $CARGO_HOME, and whether rustup's rust-src is installed). A build on macOS was 28563 bytes while GitHub CI produced 28471, failing the wasm-dist check. Pass --remap-path-prefix for each so the paths are fixed, and rebuild lib/mappings.wasm (now 28443 bytes). trim-paths would do this but is not stable in Cargo 1.98.1. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Bump the Node.js version in CONTRIBUTING.md and the lib/url.js comment to 18, and use .catch() in read-wasm-browser.js. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- Drop the `npm run build` / dist/source-map.js instructions; there is no JS build step since dist/ was removed in mozilla#362. - Refresh the sample `npm test` output. - Pin wasm-pack 0.15.0 in the install command, matching CI. - Document the binaryen 132 requirement in wasm-mappings/CONTRIBUTING.md, which still claimed wasm-pack was the only tool needed. - Point wasm-pack links at wasm-bindgen.github.io; the rustwasm.github.io page is gone. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
bomsy
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Some