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

Support more fine-grained source location tracking and use it for functions #3463

Merged
merged 3 commits into from Apr 21, 2016

Conversation

loganfsmyth
Copy link
Member

This is a fix for https://phabricator.babeljs.io/T7258 / https://phabricator.babeljs.io/T7255 so @kpdecker I'd love your thoughts if you have a minute.

Basically, this uses the call stack and scoping to track the current ast node's source location as the code is being generated. As code is generated, Babel will read the currently active source location, and create a mapping for the content that is being generated. This allows us to have more direct control over the maps that are generated. We can use this control to clear the source location entirely, or to set a new location part-way through rendering.

In this case, I'm using the ability to change the location to point to the end of the function when we generate the last line of a function. This allows us to solve https://phabricator.babeljs.io/T7258.

@codecov-io
Copy link

Current coverage is 85.81%

Merging #3463 into master will decrease coverage by -0.01% as of d065a40

@@            master   #3463   diff @@
======================================
  Files          197     197       
  Stmts        12171   12186    +15
  Branches      2502    2507     +5
  Methods          0       0       
======================================
+ Hit          10446   10458    +12
- Partial        519     522     +3
  Missed        1206    1206       

Review entire Coverage Diff as of d065a40

Powered by Codecov. Updated on successful CI builds.

@kpdecker
Copy link
Contributor

@loganfsmyth I'm going to try to make time this weekend to test this branch in my project to see if it impacts the negative behavior that I saw. Will report back here.

@loganfsmyth
Copy link
Member Author

@kpdecker Awesome, thanks for taking a look!

@kpdecker
Copy link
Contributor

I believe that I was able to test these changes and they don't regress my particular case. For sanity check, my general process was:

  1. Checkout this branch, build with make bootstrap; make build; cd packages/babel-generator; npm link
  2. In project npm link babel-generator
  3. Purposefully fail a test
  4. Examine stack trace to verify that the expect calls are on the call line, not the import line.

When running with both the linked copy and a fresh install of babel-generator@6.7.5, the stack traces were the same.

I don't really have the time review the changes, but no complaints from me on this PR.

@loganfsmyth loganfsmyth merged commit f6be6e0 into babel:master Apr 21, 2016
@loganfsmyth loganfsmyth deleted the sourcemap-rework branch April 21, 2016 01:57
@lock lock bot added the outdated A closed issue/PR that is archived due to age. Recommended to make a new issue label Oct 7, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Oct 7, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
outdated A closed issue/PR that is archived due to age. Recommended to make a new issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants