-
-
Notifications
You must be signed in to change notification settings - Fork 32
Merge upstream/master into main #135
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
Conversation
In case of most transforms (for ECMAScript), the original input source map is used only to adjust mappings, and other fields are kept intact. The _lazy_ variant source maps are source maps used only to call `adjust_mappings`. It does not deserialize everything. Instead, it only deserializes the fields required to calculate the new mappings. This is done in the name of the performance, and the justification for introducing a new type is at vercel/next.js#80177 (comment) <img width="881" alt="image" src="https://github.com/user-attachments/assets/7ca4b084-ba46-4384-ac25-fefeae0e2513" />
I added some `skip_serializing_if` to `RawSourceMap`
`BytesStr` is a more efficient variant of `Arc<str>` Since swc-project/swc#10580, SWC also uses it to store the source map strings
cherry pick getsentry#130 Co-authored-by: Sebastian Zivota <[email protected]>
A stupid typo
cherry-pick getsentry#132 --------- Co-authored-by: Sebastian Zivota <[email protected]>
Caused invalid source content: <img width="2018" height="721" alt="image" src="https://github.com/user-attachments/assets/dca54abd-fec5-4f4e-88ac-67e2d255dbfd" />
Integrate changes from upstream getsentry/rust-sourcemap v9.3.0 release: - Update CHANGELOG.md with upstream changes - Merge improvements to decoder, encoder, and type definitions - Update test suite with upstream enhancements Conflicts resolved by keeping swc-project fork modifications: - Cargo.toml: Maintain package name as 'swc_sourcemap' and current version - src/sourceview.rs: Keep BytesStr-based implementation 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
|
Sorry. I didn't intended to open a PR but Claude Code created one towards this original repository instead of the fork |
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 PR merges upstream changes from getsentry/rust-sourcemap v9.3.0 into the swc-project fork, maintaining the fork's specific modifications while integrating upstream improvements. The key change is migrating from Arc<str> to BytesStr for string storage throughout the codebase, alongside adding a new lazy deserialization module and an enhanced adjust_mappings_from_multiple function for complex sourcemap composition.
Key Changes:
- Migrated string storage from
Arc<str>toBytesStracross all sourcemap types for improved memory efficiency - Added new
lazymodule with lazy deserialization support usingserde_json::RawValue - Implemented
adjust_mappings_from_multiplefunction for multi-sourcemap composition scenarios - Updated all test files and examples to use
swc_sourcemappackage name
Reviewed changes
Copilot reviewed 30 out of 33 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| Cargo.toml | Updated package metadata for swc fork, added bytes-str dependency and serde features |
| src/types.rs | Major refactoring: BytesStr migration, extracted adjust_mappings logic, added adjust_mappings_from_multiple |
| src/sourceview.rs | Reimplemented line storage with AtomicUsize and Mutex for improved concurrency |
| src/builder.rs | Updated API to use BytesStr parameters instead of &str |
| src/lazy/mod.rs | New module providing lazy deserialization with MaybeRawValue wrapper |
| src/decoder.rs | Made decode_rmi public(crate) for lazy module |
| src/encoder.rs | Made encode_vlq_diff and encode_rmi public(crate) for lazy module |
| src/hermes.rs | Updated return types to use BytesStr references |
| src/jsontypes.rs | Changed FacebookScopeMapping.names to Vec |
| src/ram_bundle.rs | Updated to use BytesStr::from_utf8_slice for source view creation |
| src/utils.rs | Updated doc example to use swc_sourcemap import |
| src/lib.rs | Added public lazy module |
| tests/*.rs | Updated all imports to use swc_sourcemap package name |
| tests/fixtures/adjust_mappings_from_multiple/* | Added new test fixtures for multi-map composition |
| examples/*.rs | Updated imports to swc_sourcemap |
| cli/src/main.rs | Updated import to swc_sourcemap |
| README.md | Updated code example to use swc_sourcemap |
| .github/workflows/ci.yml | Changed branch from master to main, added merge_group trigger |
| .vscode/settings.json | Added Rust analyzer configuration |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| assert_eq!(name, Some("onFailure")); | ||
| assert_eq!(name.map(|v| &**v), Some("onFailure")); | ||
|
|
||
| // a stacktrae |
Copilot
AI
Dec 4, 2025
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.
Typo in the comment: "stacktrae" should be "stacktrace".
| lines.push(unsafe { | ||
| str::from_utf8_unchecked(slice::from_raw_parts(rv.as_ptr(), rv.len())) | ||
| }); |
Copilot
AI
Dec 4, 2025
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.
Using unsafe code with str::from_utf8_unchecked and slice::from_raw_parts to create a &'static str from a slice that doesn't have a static lifetime is unsound. The lifetime of the returned &str is incorrectly extended to 'static when it should be tied to the lifetime of self.source.
The returned string slice references memory in self.source, but is being cast to 'static. This means the string could outlive the SourceView that owns it, leading to use-after-free issues.
Instead, the code should either:
- Store the actual string data (not just slices) in
lines, or - Use a safer lifetime management approach that ties the lifetime of returned strings to the
SourceViewitself.
| self_range.value.dst_col as i32 - self_range.value.src_col as i32, | ||
| ); | ||
|
|
||
| // Skip `input_ranges` that are entirely before the `_range`. |
Copilot
AI
Dec 4, 2025
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 comment says "Skip input_ranges that are entirely before the _range" but should say "Skip input_ranges that are entirely before the self_range". The underscore prefix appears to be a typo.
| // Skip `input_ranges` that are entirely before the `_range`. | |
| // Skip `input_ranges` that are entirely before the `self_range`. |
| pub fn set_source(&mut self, idx: u32, value: &str) { | ||
| self.sources[idx as usize] = value.into(); | ||
| pub fn set_source(&mut self, idx: u32, value: BytesStr) { | ||
| self.sources[idx as usize] = value.clone(); |
Copilot
AI
Dec 4, 2025
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 clone() call here is unnecessary. Since value is already a BytesStr and it's being moved into the vector, you can just assign it directly without cloning:
self.sources[idx as usize] = value;Then update line 771 to use &self.sources[idx as usize] instead of &value.
| fn assert_raw_value(self) -> &'a RawValue { | ||
| match self { | ||
| MaybeRawValue::RawValue(s) => s, | ||
| MaybeRawValue::Data(_) => unreachable!("Expected RawValue, got Data"), |
Copilot
AI
Dec 4, 2025
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.
[nitpick] Using unreachable!() in assert_raw_value could lead to panics if the assumption is violated. Consider using a Result return type or a more descriptive panic message that includes context about what operation was being performed when this was called, to aid in debugging if this invariant is ever violated.
| fn assert_raw_value(self) -> &'a RawValue { | |
| match self { | |
| MaybeRawValue::RawValue(s) => s, | |
| MaybeRawValue::Data(_) => unreachable!("Expected RawValue, got Data"), | |
| fn assert_raw_value(self) -> Result<&'a RawValue> { | |
| match self { | |
| MaybeRawValue::RawValue(s) => Ok(s), | |
| MaybeRawValue::Data(_) => Err(Error::Custom("assert_raw_value called on Data variant".to_string())), |
| @@ -1,11 +1,13 @@ | |||
| name: CI | |||
|
|
|||
|
|
|||
Copilot
AI
Dec 4, 2025
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.
[nitpick] Extra blank line at the beginning of the file. Consider removing it for consistency.
| self.file.as_mut().map(|f| &*f.as_data()) | ||
| } | ||
|
|
||
| pub fn sources(&mut self) -> impl Iterator<Item = &'_ BytesStr> + use<'_, 'a> { |
Copilot
AI
Dec 4, 2025
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 use<'_, 'a> syntax in the return type is a precise capture clause that was introduced in Rust 2024 edition, but this package is configured to use edition = "2018" in Cargo.toml (line 11). This code will not compile with edition 2018. Either:
- Update the edition to "2024" in Cargo.toml, or
- Remove the
+ use<'_, 'a>clause and rely on automatic lifetime capture.
Note that Rust 2024 edition requires a recent nightly compiler or Rust 1.83+ (stable, released December 2024).
| pub fn sources(&mut self) -> impl Iterator<Item = &'_ BytesStr> + use<'_, 'a> { | |
| pub fn sources(&mut self) -> impl Iterator<Item = &'_ BytesStr> { |
|
|
||
| ```rust | ||
| use sourcemap::SourceMap; | ||
| use swc_sourcemap::SourceMap; |
Copilot
AI
Dec 4, 2025
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.
[nitpick] While the code example on line 31 has been updated correctly to use swc_sourcemap, the installation instructions earlier in the README (lines 7-18, outside the diff) still reference the old sourcemap package name and getsentry repository. These should ideally be updated for consistency, though they fall outside the changed region.
| // MERGED MODULE: [project]/turbopack/crates/turbopack-tests/tests/snapshot/source_maps/input-source-map-merged/input/sourcemapped.js [test] (ecmascript) | ||
| ; | ||
| function runExternalSourceMapped(fn) { | ||
| return fn(); |
Copilot
AI
Dec 4, 2025
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.
Callee is not a function: it has type undefined.
| return fn(); | |
| if (typeof fn === 'function') { | |
| return fn(); | |
| } | |
| // Optionally, handle the case where fn is not a function | |
| return undefined; |
Summary
Conflict Resolution
Conflicts were resolved by keeping swc-project fork modifications:
swc_sourcemapand current versionTest plan
swc_sourcemap🤖 Generated with Claude Code