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 check type annotations when deciding params scope #11349
Conversation
@@ -36,6 +36,10 @@ const iifeVisitor = { | |||
path.stop(); | |||
} | |||
}, | |||
// type annotations don't use or introduce "real" bindings | |||
"TypeAnnotation|TSTypeAnnotation"(path) { |
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.
Ideally we may expose bindingType
of checkLVal
to parseBindingList
so we do not register bindings of lval when they are inside a type annotation.
BTW we should also skip TSTypeParameters.
function a(b = <T extends (c: any) => void>(): void => {}) {
let c;
}
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.
Since identifiers are the most often created/moved nodes, I think that it would be safer to add a skip
like this one to @babel/traverse
rather than relying on what was initially parsed.
let c; | ||
} | ||
|
||
function d(e = <T>() => {}) { |
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: This test will be passing without current change since we are not registering bindings when parsing type parameters.
Type annotations caused unnecessary usage of IIFE, because we checked if they referenced any value binding.