Remove uses of visitNodes and visitNode in visitEachChild #49992
Conversation
typescript-bot
added
Author: Team
For Milestone Bug
|
I don’t understand why this is needed. Don’t synthesized nodes emitted by the TextChanges writer get their |
|
Not some, it seems. I'll go figure out who's making this array and why it's not getting fixed, then. (Marking as draft for now so I don't accidentally click the green button.) |
|
It’s still quite possible this is a good fix, I’d just like to understand it a little more. |
|
Aha, you were good to wonder. There's a number of long-standing typos in |
jakebailey
changed the title
| @@ -616,7 +616,7 @@ namespace ts { | |||
| nodeVisitor(node.argument, visitor, isTypeNode), | |||
| nodeVisitor(node.assertions, visitor, isNode), | |||
| nodeVisitor(node.qualifier, visitor, isEntityName), | |||
| visitNodes(node.typeArguments, visitor, isTypeNode), | |||
| nodesVisitor(node.typeArguments, visitor, isTypeNode), | |||
There was a problem hiding this comment.
This one in particular was the bug.
I tried to come up with a way that would prevent this sort of from happening (like Debug.type<unknown>(visitNodes), or redeclaring it), but wasn't successful in stopping TS from allowing this.
There was a problem hiding this comment.
I really, really wonder how many other bugs there are that were caused by this in the wild...
There was a problem hiding this comment.
wasn't successful in stopping TS from allowing this.
The fix would be to start targeting ES2015 aka ES6, then do const visitNodes = undefined in the body; the only reason that doesn't work now is that we downlevel default parameters to assignments and that interferes with the scoping.
Fixes #49310