Include original function names into GopherJS source maps. #1392
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
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.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 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.
This is a branch off of #1338 so that I could fix the merge conflicts I caused. I also discovered that the issue with Windows is that when I migrated the CircleCI over to GHA, I had copied over the environment variable assignment to disable source maps for Windows. I removed that.
The following is the summary from that PR originally written by @nevkontakte
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.