Include original function names into GopherJS source maps. #1338
Conversation
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
flimzy
approved these changes
Aug 5, 2024
1843b57 to
8c118fc
Compare
flimzy
approved these changes
Aug 5, 2024
|
Hmm, those Windows errors are unexpected. I'll look into them a bit later. |
|
Wanted to see if I could help, since my main dev environment lives on Windows. But...
Do you have them copied, by chance? |
The main change in this commit is an ability to use identifier name mapping, which we use to show original function names in the source map. This addresses the long-standing gopherjs#1085, where GopherJS call stacks were somewhat difficult to interpret due to function name mangling, especially in minified form. Now we emit an additional source map hit with the original function name, which Node is able to pick up. While at it, I moved source map hinting logic into its own package with tests and added some documentation on how it works. Now it should be easy to extend this mechanism for even richer source maps if we want to.
The tests that were previously failing should work again with the new function name source maps. There are only two caveats at this point: - For some reasons a function that's currently executing deferrals doesn't map to its original name. - Synthetic names for literal functions differ in format slightly from vanilla Go, which still breaks the test in net/http. The difference is minor enough it's not forth fixing though.
Change seems to be here: d63a9b8#diff-d3cde2dec4b7c2263747b17d43f704418e6657b49583254628bcad1d1052471bR325 Curious, because I don't know much of GopherJS' internals, this seems to be part of the "official" Go compiler itself. Is it hardforked or something...? Would love to know a little more, if you have the time. ^^ Thanks! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Previously GopherJS only emitted mapping between original and generated source locations. With this change it also includes original function names, which fixes #1085.
Before:
After:
While working on this, I moved sourcemap hinting logic into its own package and added some tests. The implementation could still use some improvements, but it should be a significant improvement in the majority of cases.