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/missing references after crawl #11011
Merged
JLHwung
merged 7 commits into
babel:master
from
regiontog:bug/missing-references-after-crawl
Jan 20, 2020
Merged
Changes from all commits
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
804969c
Bug replication test
regiontog 48a7790
Simplify test
regiontog 37af725
Merge branch 'master' into bug/missing-references-after-crawl
regiontog 5c0b0ac
Fixes #10896
regiontog 63a12f2
Merge path.crawl `ClassDeclaration` and `Declaration` visitors
regiontog 92b4192
Move registartion of class id in crawl to BlockScoped
regiontog 725e959
Add some assertions to crawl test
regiontog File filter
Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
There are no files selected for viewing
This file contains 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
This file contains 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
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't the following change all that's necessary?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree.
parent
is the nearest function scope, but classes are block-scoped:There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change
implicitly depends on the fact that the
Declaration
visitor runs beforeClassDeclaration
visitor, so thatpath.scope.parent.getBinding
always points to the new binding which may be initialized inDeclaration
visitor orBlockScoped
visitor.See #10896 (comment)
Besides that, I agree we should do
path.scope.parent
instead ofparent
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the class binding may be(always is?) initialized in
BlockScoped
won't this stop the identifier from being visible inside the class scope if the visitors are merged?babel/packages/babel-traverse/src/scope/index.js
Lines 62 to 64 in 63a12f2
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@regiontog Ah you are right! The
ClassDeclaration
binding is registered inBlockScoped
andBlockScoped
runs beforeClassDeclaration
. I am curious why the test can even get passed.