fix: Identifiers in the loop are not renamed#15361
fix: Identifiers in the loop are not renamed#15361nicolo-ribaudo merged 4 commits intobabel:mainfrom
Conversation
|
Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/53792/ |
| // `index` will be registered into `scope.globals` | ||
| type SomeLookup = { | ||
| [index: number]: any[]; | ||
| }; |
There was a problem hiding this comment.
I don't think this test is very good, but I don't know how to make it better.
Because after we change traverse it will not be registered in globals.
There was a problem hiding this comment.
Can we add a custom plugin to the test that registers a non-existing global? You can create a plugin.js file and add it as "plugins": ["./plugin.js"] in options.json
| } else if ( | ||
| headScope.parent.hasBinding(name) || | ||
| blockScope.parent.hasGlobal(name) || | ||
| (isProgramScope && varScope.hasGlobal(name)) |
There was a problem hiding this comment.
Since hasGlobal checks across all the parent scope, when can it happen that blockScope.parent.hasGlobal(name) is false but isProgramScope && varScope.hasGlobal(name) is true?
There was a problem hiding this comment.
I'm not sure, I actually copy-pasted it from here. 🤦♂️
Because I found the easiest way is to rename this identifier before wrapLoopBody.
babel/packages/babel-plugin-transform-block-scoping/src/index.ts
Lines 213 to 231 in 112169c
There was a problem hiding this comment.
Whops 😅
It looks like deleting the isProgramScope && varScope.hasGlobal(name) check also from that code still makes all the tests pass.
| let varScope = blockScope.getFunctionParent(); | ||
| let isProgramScope = false; | ||
| if (!varScope) { | ||
| varScope = blockScope.getProgramParent(); |
There was a problem hiding this comment.
const varScope = blockScope.getFunctionParent() || blockScope.getProgramParent();
After I tried some wrong methods, I found out that it is so simple. 😅