fix: new.target with shadowed class name
#14611
Conversation
liuxingbaoyu
commented
May 28, 2022
•
liuxingbaoyu
commented
| Q | A |
|---|---|
| Fixed Issues? | Fixes #14406 |
| Patch: Bug Fix? | |
| Major: Breaking Change? | |
| Minor: New Feature? | |
| Tests Added + Pass? | Yes |
| Documentation PR Link | |
| Any Dependency Changes? | |
| License | MIT |
|
Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/52172/ |
liuxingbaoyu
force-pushed
the
fix-shadowed-new-target
branch
from
fbc4b1c
to
1b280bd
Compare
liuxingbaoyu
added
the
PR: Bug Fix 🐛
| if (!node.id) { | ||
| node.id = scope.generateUidIdentifier("target"); | ||
| } else { | ||
| // packages/babel-helper-create-class-features-plugin/src/fields.ts#L192 unshadow | ||
| let scope = path.scope; | ||
| const name = node.id.name; | ||
| while (scope !== func.parentPath.scope) { | ||
| if ( | ||
| scope.hasOwnBinding(name) && | ||
| !scope.bindingIdentifierEquals(name, node.id) | ||
| ) { | ||
| scope.rename(name); | ||
| } | ||
| scope = scope.parent; | ||
| } |
There was a problem hiding this comment.
Nit: they can be moved after the if (func.isClass()) branch.
| if (!node.id) { | ||
| node.id = scope.generateUidIdentifier("target"); | ||
| } else { | ||
| // packages/babel-helper-create-class-features-plugin/src/fields.ts#L192 unshadow |
There was a problem hiding this comment.
We could actually move this to a Scope#unshadow(name, targetAncestorScope) method, and use it here like
path.scope.unshadow?.(name, func.parentPath.scope);It will only work when people update their @babel/traverse (usually by updating their @babel/core), but it's not a breaking change because when using older versions new.target will just continue to be broken.
Then, we can use scope.unshadow() in babel-helper-create-class-features-plugin starting from Babel 8 (or better, now with a fallback to the inline unshadow() function when !process.env.BABEL_8_BREAKING).
There was a problem hiding this comment.
I feel a little unnecessary.
Because unshadow is only a few lines of code and may be different in different scenarios.
Another major reason is that our current Scope is a bit messy, eg binding is not always reliable.
Maybe we can do it in the future.
There was a problem hiding this comment.
Oh ok fair enough, we can refactor if needed.
nicolo-ribaudo
changed the title
new.target with the duplicate namenew.target with shadowed class name
github-actions
bot
added
the
outdated
