microsoft / TypeScript

TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
https://www.typescriptlang.org
Apache License 2.0
99.3k stars 12.31k forks source link

visitor of visitEachChild doesn't support returning InferTypeNode #56480

Closed marcus-sa closed 4 months ago

marcus-sa commented 8 months ago

πŸ”Ž Search Terms

False expression: Unexpected node Node InferType did not pass test 'isEntityName'.

πŸ•— Version & Regression Information

⏯ Playground Link

No response

πŸ’» Code

https://github.com/marcus-sa/deepkit-framework/blob/9c9eb001459a4e1046aea94cb531ba5d8ca3b1c9/packages/type-compiler/src/compiler.ts#L2353-L2370

πŸ™ Actual behavior

Visitor callback of visitEachChild doesn't support returning InferTypeNode when the node is a TypeReferenceNode

Verbose Debug Information: Node InferType did not pass test 'isEntityName'.
           at visitNode (/Users/marcus-sa/Git/deepkit/deepkit-framework/node_modules/typescript/lib/typescript.js:87180:11)
           at visitEachChildOfTypeReferenceNode (/Users/marcus-sa/Git/deepkit/deepkit-framework/node_modules/typescript/lib/typescript.js:87589:32)
           at visitEachChild (/Users/marcus-sa/Git/deepkit/deepkit-framework/node_modules/typescript/lib/typescript.js:87421:35)
           at ReflectionTransformer.resolveTypeParameter (file:///Users/marcus-sa/Git/deepkit/deepkit-framework/packages/type-compiler/dist/esm/src/compiler.js:1910:54)

πŸ™‚ Expected behavior

For it to work

marcus-sa commented 8 months ago

The following patch fixes it (or at least works around it):

diff --git a/node_modules/typescript/lib/typescript.js b/node_modules/typescript/lib/typescript.js
index 86ab90b..dc72ecb 100644
--- a/node_modules/typescript/lib/typescript.js
+++ b/node_modules/typescript/lib/typescript.js
@@ -87586,7 +87586,7 @@ ${lanes.join("\n")}
         [183 /* TypeReference */]: function visitEachChildOfTypeReferenceNode(node, visitor, context, nodesVisitor, nodeVisitor, _tokenVisitor) {
           return context.factory.updateTypeReferenceNode(
             node,
-            Debug.checkDefined(nodeVisitor(node.typeName, visitor, isEntityName)),
+            Debug.checkDefined(nodeVisitor(node.typeName, visitor, node => isTypeNode(node) || isEntityName(node))),
             nodesVisitor(node.typeArguments, visitor, isTypeNode)
           );
         },
marcus-sa commented 4 months ago

@andrewbranch where are we with this? I would gladly make a PR.

rbuckton commented 4 months ago

A TypeReferenceNode shouldn't have an InferTypeNode in its typeName property, so the error you see is correct and indicates a problem in your code. Looking at the code you linked to, I believe you are incorrectly updating the tree with a node in an invalid position. You shouldn't be replacing an Identifier with an InferTypeNode, you should be looking for the TypeReferenceNode containing that identifier and replace it instead.

const searchArgument = (node: Node): Node => {
    node = visitEachChild(node, searchArgument, this.context);

    if (isTypeReferenceNode(node) && isIdentifier(node.typeName) && node.typeName.escapedText === argumentName) {
        //transform to infer T
        found = true;
        node = this.f.createInferTypeNode(declaration);
    }

    return node;
};
marcus-sa commented 4 months ago

@rbuckton thanks a lot!