Improves sourcemap path handling with pathdiff#1217
Merged
Dekkonot merged 18 commits intorojo-rbx:masterfrom Feb 13, 2026
Merged
Improves sourcemap path handling with pathdiff#1217Dekkonot merged 18 commits intorojo-rbx:masterfrom
Dekkonot merged 18 commits intorojo-rbx:masterfrom
Conversation
Left untouched because it requires a greater refactor to compare the backend structs
Left untouched because it requires a greater refactor to compare the backend structs
Contributor
|
Fix seems to work! |
ari-party
reviewed
Jan 24, 2026
Member
kennethloeffler
left a comment
kennethloeffler
left a comment
There was a problem hiding this comment.
This PR also closes #1087; please edit the description to reflect this.
Code looks overall good, but I'll dig in and complete my review tomorrow. Thanks for the contributions!
Member
kennethloeffler
left a comment
kennethloeffler
left a comment
There was a problem hiding this comment.
Just some nits, looking good!
Contributor
Author
|
Anything still required to get this merged? @kennethloeffler |
Dekkonot
approved these changes
Feb 13, 2026
Member
Dekkonot
left a comment
Dekkonot
left a comment
There was a problem hiding this comment.
Seems good to me. I let a nitpick but it's more just a note for you in the future rather than a blocker for this being merged.
I'm able to verify that it works locally, and the test seems fine to me.
Comment on lines
+322
to
+323
| assert_eq!(fs_err::canonicalize(path).is_ok(), true, "path was not valid"); | ||
| assert_eq!(Path::new(path).is_absolute(), true, "path was not absolute"); |
Member
There was a problem hiding this comment.
Nitpick: Why not just use assert! here?
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
4 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.
Replaces manual path stripping logic in sourcemap generation with the
pathdiffcrate to reliably calculate relative paths from the project root. This change ensures more robust handling of file paths when generating sourcemaps, particularly when dealing with complex directory structures.This PR closes #1212 which was originally caused due to #1201 changing how paths are internally stored on Windows.
With the modification of how the relative paths are calculated, the issue disappears.
This update also introduces new tests for both relative and absolute path mapping in sourcemap generation, which ensures these kinds of issues arise during the testing phase instead of only after manual testing.
Major changes
pathdiffdependency to accurately calculate relative paths between the project root and file paths during sourcemap construction.jsonfeature to theinstatest dependency.SourcemapNodeto deriveDeserializeand addsdefaultattribute usage for optional fields.maps_relative_pathsandmaps_absolute_paths) to validate correct path generation in sourcemaps. (Closes Add tests forsourcemapcommand #1087)