Skip to content

Comments

feat(sourcemap): support safe scopes round-trip metadata#11581

Merged
kdy1 merged 2 commits intomainfrom
kdy1/fix-11424-sourcemap-scopes
Feb 24, 2026
Merged

feat(sourcemap): support safe scopes round-trip metadata#11581
kdy1 merged 2 commits intomainfrom
kdy1/fix-11424-sourcemap-scopes

Conversation

@kdy1
Copy link
Member

@kdy1 kdy1 commented Feb 24, 2026

Summary

  • add ECMA-426 top-level scopes field support to eager/lazy sourcemap decode/encode
  • preserve scopes for plain read/write round-trip
  • clear scopes in mutating paths where metadata can become stale (remove_names, rewrite_with_mapping, adjust_mappings, lazy adjust_mappings)
  • add tests for eager/lazy round-trip and invalidation behavior

Verification

  • git submodule update --init --recursive
  • cargo fmt --all
  • cargo clippy --all --all-targets -- -D warnings
  • cargo test -p swc_sourcemap

@changeset-bot
Copy link

changeset-bot bot commented Feb 24, 2026

🦋 Changeset detected

Latest commit: 46afe32

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@github-actions
Copy link
Contributor

github-actions bot commented Feb 24, 2026

Binary Sizes

File Size
swc.linux-x64-gnu.node 28M (28585928 bytes)

Commit: 34650d2

@codspeed-hq
Copy link

codspeed-hq bot commented Feb 24, 2026

Merging this PR will not alter performance

✅ 184 untouched benchmarks


Comparing kdy1/fix-11424-sourcemap-scopes (46afe32) with main (81b22c3)

Open in CodSpeed

@kdy1 kdy1 marked this pull request as ready for review February 24, 2026 03:57
@kdy1 kdy1 requested a review from a team as a code owner February 24, 2026 03:57
Copilot AI review requested due to automatic review settings February 24, 2026 03:57
@kdy1 kdy1 merged commit de2a348 into main Feb 24, 2026
23 checks passed
@kdy1 kdy1 deleted the kdy1/fix-11424-sourcemap-scopes branch February 24, 2026 03:57
@github-actions github-actions bot added this to the Planned milestone Feb 24, 2026
@claude
Copy link
Contributor

claude bot commented Feb 24, 2026

PR Review: feat(sourcemap): support safe scopes round-trip metadata

Summary

This PR adds support for the ECMA-426 scopes field in source maps. The approach is sound: treat scopes as an opaque string for round-trip preservation, and explicitly invalidate it in all mutating code paths where the scopes metadata could become stale.

Code Quality

The implementation is clean and consistent with the existing patterns in the codebase:

  • get_scopes/set_scopes follow the naming convention of get_file/set_file, get_source_root/set_source_root, etc.
  • The set_scopes generic signature <T: Into<BytesStr>> is idiomatic for this crate.
  • skip_serializing_if = "Option::is_none" is correctly applied in both eager and lazy JSON types.
  • The borrow serde attribute on the lazy scopes field enables zero-copy deserialization, which is the right call.

Correctness — scopes invalidation coverage

The PR correctly invalidates scopes in three explicit locations:

  1. remove_names (types.rs:865) — correct, since scopes reference names by index.
  2. rewrite_with_mapping (types.rs:943) — correct, rewrite rebuilds the map.
  3. adjust_mappings (types.rs:973 and lazy/mod.rs:461) — correct, mappings are rewritten.

One observation: adjust_mappings_from_multiple (types.rs:1125) builds a fresh SourceMap via SourceMapBuilder::into_sourcemap(), which defaults scopes to None. So scopes are implicitly dropped. This is correct behavior, but every other mutating path has an explicit self.scopes = None; for clarity. Consider adding a comment near the end of adjust_mappings_from_multiple (around line 1325) noting that scopes invalidation is handled by the builder's default, for consistency and to prevent regressions if the builder changes later:

let mut new_map = new_map.into_sourcemap();
// Note: scopes are implicitly None from the builder — this is intentional
// since mapping adjustment invalidates scope metadata.

Test Coverage

Test coverage is good:

  • Eager path: decode round-trip (test_sourcemap_scopes), encode round-trip (test_scopes_roundtrip), rewrite drops scopes, adjust_mappings drops scopes, remove_names drops scopes.
  • Lazy path: decode/encode round-trip (test_scopes_roundtrip), adjust_mappings drops scopes.

Minor suggestion: a test for adjust_mappings_from_multiple confirming scopes are dropped would make the coverage complete, even though it works implicitly today.

Performance

No concerns. The scopes field is stored as a single BytesStr / StrValue (opaque string), so the memory and serialization overhead is minimal. No parsing or allocation beyond what serde does during JSON decode/encode.

Security

No concerns. The scopes string is opaque pass-through data — it is not interpreted, executed, or used in any path-construction logic.

Nit

In lazy/mod.rs:39, the range_mappings field directly below scopes uses #[serde(default, skip_serializing_if = "Option::is_none")] without borrow, while scopes has borrow. This is fine since range_mappings is Option<String> (owned) while scopes is Option<StrValue<'a>> (borrowed) — just noting the intentional asymmetry is correct.

Verdict

This is a well-structured, minimal change that correctly handles the ECMA-426 scopes field with proper invalidation semantics. The approach of treating scopes as opaque metadata is the right one for now. LGTM with the minor suggestions above.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds support for the ECMA-426 top-level scopes sourcemap metadata so it can be decoded/encoded (eager + lazy) and preserved on read/write round-trips, while being invalidated on mutating operations where it may become stale.

Changes:

  • Add scopes storage + getter/setter to the eager SourceMap type, and invalidate it in mutating APIs (remove_names, rewrite_with_mapping, adjust_mappings).
  • Extend eager and lazy JSON (de)serialization paths to include the optional scopes field.
  • Add tests covering eager/lazy scopes round-trip preservation and invalidation behavior.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated no comments.

Show a summary per file
File Description
crates/swc_sourcemap/src/types.rs Stores scopes on eager SourceMap, exposes get/set, and clears it on mutating operations; adds unit tests for invalidation.
crates/swc_sourcemap/src/jsontypes.rs Extends eager JSON RawSourceMap schema with optional scopes.
crates/swc_sourcemap/src/decoder.rs Populates eager SourceMap.scopes during decode.
crates/swc_sourcemap/src/encoder.rs Serializes eager SourceMap.scopes when present.
crates/swc_sourcemap/src/lazy/mod.rs Adds scopes to lazy RawSourceMap/SourceMap and preserves it through lazy decode/encode; clears it on lazy adjust_mappings; adds tests.
crates/swc_sourcemap/tests/test_decoder.rs Integration test verifying eager decode reads scopes.
crates/swc_sourcemap/tests/test_encoder.rs Integration test verifying eager encode/decode round-trips scopes.
.changeset/eleven-papayas-attend.md Publishes a minor changeset for swc_sourcemap / swc_core.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant