Fixed generateUid creating clashing ids after scope re-crawling #11375
Conversation
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
Andarist
added
PR: Bug Fix 🐛
Andarist
commented
Apr 3, 2020
| @@ -761,7 +788,10 @@ export default class Scope { | |||
| } | |||
|
|
|||
| init() { | |||
| if (!this.references) this.crawl(); | |||
There was a problem hiding this comment.
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)
Andarist
commented
Apr 3, 2020
|
|
||
| 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.
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
nicolo-ribaudo
approved these changes
Apr 7, 2020
|
|
||
| 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 |
existentialism
approved these changes
Apr 7, 2020
github-actions
bot
added
the
outdated
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
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 firstcrawlcall into the mentionedcollectorVisitor- this should allow them to be always uniformly, each time.