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
Don't throw when destructuring into a var named as an import #10628
Don't throw when destructuring into a var named as an import #10628
Conversation
nicolo-ribaudo
commented
Nov 2, 2019
Q | A |
---|---|
Fixed Issues? | Fixes #10627 |
Patch: Bug Fix? | Yes |
Major: Breaking Change? | |
Minor: New Feature? | |
Tests Added + Pass? | Yes |
Documentation PR Link | |
Any Dependency Changes? | |
License | MIT |
} = {}); | ||
} | ||
|
||
_x.foo = (2, function () { |
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.
In a follow up PR, this could be left as foo
since we are throwing anyway.
.pop(); | ||
const programScopeIds = Object.keys(ids).filter( | ||
localName => | ||
scope.getBinding(localName) === path.scope.getBinding(localName), |
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.
nit: consider to guard with scope.hasBinding
to avoid unnecessary path.scope.getBinding
.
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 tried to do it and found a bug which, if fixed, would break @babel/plugin-transform-runtime
<= 7.6 😢
This is what currently happens:
- The imports are analyzed
- The imports are removed -> The bindings are removed
- References to imported bindings are rewritten (this file)
i) When we are comparing the bindings, the global one isundefined
ii) If the local binding isundefined
, it hasn't been redefined.
I tried to swap 2 and 3, but I still can optimize away the check for the local binding because @babel/transform-runtime
doesn't register the import it injects in the scope tracker.
14a6afa
to
65c0f1f
Compare