Conversation
| } | ||
| } | ||
|
|
||
| function asyncBodyVisitor(node: Node): VisitResult<Node> { |
There was a problem hiding this comment.
Is there a canonical source we can reference in a comment here for the list-of-things-which-may-contain-or-whose-children-may-contain-hoisted-declarations? It seems nonobvious which elements of the statement and declaration list need to be explicitly visited here.
Also: Does LabeledStatement need to be handled? eg
async function fn4(x) {
lbl: {
var x = y;
}
}There was a problem hiding this comment.
I've added isNodeWithPossibleVarDeclaration in 2ba29d8 as a way to verify this list.
|
|
||
| function visitCatchClauseInAsyncBody(node: CatchClause) { | ||
| const catchClauseNames = createUnderscoreEscapedMap<true>(); | ||
| recordDeclarationName(node.variableDeclaration, catchClauseNames); |
There was a problem hiding this comment.
variableDeclaration may be undefined
There was a problem hiding this comment.
Actually, by the time we get to the es2017 transform a CatchClause must have a catch variable, otherwise it's not valid ES2017. Assigning the catch variable is handled in the esnext transform before the es2017 transform is run.
There was a problem hiding this comment.
I've added a test case in e655446 to verify this case.
| case SyntaxKind.LabeledStatement: | ||
| return visitEachChild(node, asyncBodyVisitor, context); | ||
| } | ||
| Debug.assert(!isNodeWithPossibleVarDeclaration(node), "Unhandled node."); |
There was a problem hiding this comment.
Why not make isNodeWithPossibleVarDeclaration into a type guard, start the function with it, then end with an assertNever to get the type checker to help out here?:
if(!isNodeWithPossibleVarDeclaration(node)) {
return visitor(node);
}
// ... switch block
Debug.assertNever(node, "Unhandled node.");
This fixes an issue where the downlevel emit for async functions to ES2015 can result in incorrect runtime semantics when the async function body contains a
vardeclaration that shadows a parameter name.Fixes #20461