-
-
Notifications
You must be signed in to change notification settings - Fork 29
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
fix: set sourcemap root to the workspace relative root_dir #176
Conversation
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.
would be great to have a test where source_map_support
is used.
03ce319
to
bb47c34
Compare
bb47c34
to
5a27d03
Compare
2773de5
to
ae179ed
Compare
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.
LGTM
I think this is a great solution and I added a small extra in #177. Let me know what you think. |
10de226
to
fac5edd
Compare
deb9794
to
c52fdc0
Compare
c52fdc0
to
c61c7c6
Compare
Is this still draft, or is it ready to go? |
I put it into draft state because @thesayyn wasn't sure about the paths, see the ugly second commit which I'd like him to review. Essentially we have to navigate from the .map file, out of the |
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.
looks awesome
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.
Looks like this broke Windows:
C:\tools\msys64\usr\bin\bash.exe -c external/aspect_rules_swc~override~swc~swc_win32-x64-msvc/swc-win32-x64-msvc.exe $@ < /dev/null compile --source-maps false --source-file-name foo.ts --source-root --config-file .swcrc --out-file bazel-out/x64_windows-fastbuild/bin/foo.js foo.ts
--
| # Configuration: d00ace43f601efe48e0f1336609380be0dfba06ec5d3ce9d24364e336cd3c540
| # Execution platform: @local_config_platform//:host
| error: The argument '--source-root <SOURCE_ROOT>' requires a value but none was supplied
https://buildkite.com/bazel/bcr-presubmit/builds/1090#0186b799-ced5-4b38-8404-30c2129b62bb
This makes the sourcemap
sources
andsourceRoot
align with tsc (when invoked fromts_project
EDIT: and not in a worker, and properly sandboxed, and probably other conditions for it to be the "normal" case). This is comparing ats_project
in silo, tsc vs swcThesourcesContent
is relative to theswc(root_dir)
, thesourceRoot
is relative to the workspace root and includes theroot_dir
.The map file
sources
is relative to the map, so when there is 1 .map per .js it is just the filename.The
sourceRoot
is basically just a prefix and we just set it to what the user specifies (#177)