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
Fixed generateUid creating clashing ids after scope re-crawling #11375
Conversation
@@ -761,7 +788,10 @@ export default class Scope { | |||
} | |||
|
|||
init() { | |||
if (!this.references) this.crawl(); |
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 check was super implicit and confusing (nested scopes don't even gather references
so this object stays always empty for them, but is initialized in .crawl
)
|
||
if (path.isFunctionExpression() && path.has("id")) { | ||
if (!path.get("id").node[t.NOT_LOCAL_BINDING]) { | ||
// TODO: explore removing this as it should be covered by collectorVisitor |
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.
attempt to remove this has resulted in 14~ failing tests, most of the updated snapshots seemed OK, but some didn't. It requires more investigation and I'm afraid that I won't have time to do this
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.
👍
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.
Thank you! I really appreciate that you have spent time digging in our scope tracking logic!
|
||
if (path.isFunctionExpression() && path.has("id")) { | ||
if (!path.get("id").node[t.NOT_LOCAL_BINDING]) { | ||
// TODO: explore removing this as it should be covered by collectorVisitor |
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.
👍
Bunch of references were not registered back on the program parent after re-crawling because they were not visited by the
collectorVisitor
. I've managed (mostly) to move the code parts responsible for registering those references during the firstcrawl
call into the mentionedcollectorVisitor
- this should allow them to be always uniformly, each time.