Skip to content
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

[Bug]: sourcemap paths are not relative #166

Closed
alexeagle opened this issue Feb 9, 2023 · 13 comments · Fixed by #174
Closed

[Bug]: sourcemap paths are not relative #166

alexeagle opened this issue Feb 9, 2023 · 13 comments · Fixed by #174
Labels
bug Something isn't working help wanted Aspect isn't prioritizing this, but the community could
Milestone

Comments

@alexeagle
Copy link
Member

alexeagle commented Feb 9, 2023

What happened?

When we changed to swcx we changed the sourcemaps:

https://github.com/aspect-build/rules_swc/blob/main/examples/rc/src/expected.js.map
"sources":["examples/rc/src/in.ts"]
https://github.com/aspect-build/rules_swc/blob/v0.20.1/examples/rc/src/expected.js.map
"sources":["in.ts"]

Version

It changed in the commit where we switched from @swc/cli (the nodejs wrapper) to swcx
https://github.com/aspect-build/rules_swc/commit/6358e06ac2a8d6aca2c2ea0cd6ea9dc7b9217a27#diff-90f27955a28ad20f91303a[…]4c55c9ef8b0ccb3255a1bdd1f726

@alexeagle alexeagle added the bug Something isn't working label Feb 9, 2023
@alexeagle alexeagle added this to the 1.0 milestone Feb 9, 2023
@github-actions github-actions bot added the untriaged Requires traige label Feb 9, 2023
@alexeagle
Copy link
Member Author

Note, this is pretty typical under Bazel since tools are run in a working directory at the repository root.

Options:

  1. change swcx to relativize source paths in the sourcemap output
  2. compose with another action to post-process the output of swcx
  3. wrap swcx to run it from a different working directory than Bazel spawn the action

@alexeagle alexeagle moved this to 🔖 Ready in Open Source Feb 9, 2023
@alexeagle alexeagle added help wanted Aspect isn't prioritizing this, but the community could and removed untriaged Requires traige labels Feb 9, 2023
@alexeagle
Copy link
Member Author

@realtimetodie any ideas on whether option 1 is feasible?

@alexeagle
Copy link
Member Author

Maybe it's just that we need source_root which is listed as missing on swc-project/swc#4017

@realtimetodie
Copy link
Contributor

realtimetodie commented Feb 10, 2023

Yes, the source_root option is not implemented in the Rust-based SWC cli. In the Node.js @swc/cli the entire source map is reparsed as JSON again, see swc-project/swc#1388. I mentioned this bug in #154 (comment).

https://github.com/swc-project/cli/blob/461231917181133fd2e98b954fce07b599711502/src/swc/compile.ts#L20

https://github.com/swc-project/cli/blob/461231917181133fd2e98b954fce07b599711502/src/swc/util.ts#L58-L59

I will look into it this weekend

@alexeagle
Copy link
Member Author

Ping @realtimetodie have any time to look? If not maybe @titanous who contributed another swc fix?

@realtimetodie
Copy link
Contributor

Yes, I'm about to complete it. I have some more time tomorrow.

@realtimetodie
Copy link
Contributor

realtimetodie commented Feb 27, 2023

The changes were merged upstream. Here is an example https://github.com/realtimetodie/rules_swc/tree/bug-166

@alexeagle
Copy link
Member Author

Cool, I guess we're waiting on an swc release before we can upgrade here to land the fix.

@alexeagle
Copy link
Member Author

@realtimetodie
Copy link
Contributor

realtimetodie commented Mar 1, 2023

@alexeagle So I went back in the history to test the behaviour when rules_swc used the Node.js @swc/cli.

In general, the sourceRoot value is not set by default. However, in rules_swc we set a path by default now. I think this is wrong. There should be no value set by default. We can't possibly assume the right path. Every project is different and the value could also be an URL.

Instead, the user should to set the value explicitly by either a) by using the --source-root option as a custom argument, or b) we define a new optional variable in the rule source_root. This would follow the default swc behaviour.

Example

swc(
    name = "compile",
    source_maps = True,
    source_root = "./path/to",
)

https://github.com/swc-project/cli/blob/df9b9ec04201ac353c3e794fed6fc29152fc4cb0/src/swc/options.ts#L221

@realtimetodie
Copy link
Contributor

The same goes for the source file name, which is the first entry in the sources JSON array using the --source-file-name option.

Example

swc(
    name = "compile",
    source_maps = True,
    source_file_name = "custom.ts",
)

https://github.com/swc-project/cli/blob/df9b9ec04201ac353c3e794fed6fc29152fc4cb0/src/swc/options.ts#L220

@alexeagle
Copy link
Member Author

TAL at #176 which aligns the behavior with ts_project

@alexeagle
Copy link
Member Author

Seems like the mistake was just that the source-root should have been relative to the rootDir.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working help wanted Aspect isn't prioritizing this, but the community could
Projects
Archived in project
2 participants