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

Fix source map URLs (as much as possible). Fixes #519 #551

Merged
merged 1 commit into from
Mar 23, 2025

Conversation

raquo
Copy link

@raquo raquo commented Mar 22, 2025

I think this is the best I can do for #519

scala-java-time build does some non-standard things that make supporting fully correct source maps pretty hard. But, we can get pretty close. This PR fixes the URLs for all source maps under core/shared. Unfortunately, the source maps for core/js remain broken, but most files are in shared. Someone strong in sbt may be able to improve on this, but absent of that, my vote is that it's good enough.

Also, scala-java-time build transforms the content of the source files (changes packages / imports / etc.) – FYI, to the extent that this modifies positions of terms in the source code, this could break source maps as well. But, most of the transformations are line-for-line replacements of imports and package names, which should keep the rest of the file unaffected.

In practice, the source maps generated with the new code seem to work just fine for me.

This PR introduces a change in the src_managed folder structure for the core/js project. Instead of copying source files into src_managed/main/org/threeteen/..., we now move files into src_managed/main/scala/org/threeteen/... (the scala part changes to scala-3 for sources that are located in scala-3).

I published the core module locally, and used it from my Scala 3 project, and it works fine, source maps included. It seems that these changes should even be binary compatible, seeing that only the folder structure is changed, not package names?

@cquiroz
Copy link
Owner

cquiroz commented Mar 22, 2025

Thanks, this is great could you update the workflows to run the build?

@raquo
Copy link
Author

raquo commented Mar 23, 2025

Ah right, I didn't notice that the workflow file is generated.
I've just removed my CI = true addition – it's already the default in github actions, so we don't really need it.

@cquiroz
Copy link
Owner

cquiroz commented Mar 23, 2025

Thanks a lot, this is looking great

@cquiroz cquiroz merged commit 86b8686 into cquiroz:master Mar 23, 2025
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants