Skip to content
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

Long conditional type resolves to any #28663

Closed
dsherret opened this issue Nov 24, 2018 · 8 comments · Fixed by #36583
Closed

Long conditional type resolves to any #28663

dsherret opened this issue Nov 24, 2018 · 8 comments · Fixed by #36583
Assignees
Labels
Design Limitation Constraints of the existing architecture prevent this from being fixed

Comments

@dsherret
Copy link
Contributor

dsherret commented Nov 24, 2018

TypeScript Version: 3.3.0-dev.20181122

Search Terms: conditional type resolves any

Code

After adding exactly this many conditions, the type of WrappedJsxNodeType will be any.

import * as ts from "typescript";

type WrappedNodeType = CompilerNodeToWrappedType<ts.JsxExpression>;

type CompilerNodeToWrappedType<T extends ts.Node> = T extends ts.ArrayBindingPattern ? ArrayBindingPattern :
    T extends ts.BindingElement ? BindingElement :
    T extends ts.ObjectBindingPattern ? ObjectBindingPattern :
    T extends ts.ClassDeclaration ? ClassDeclaration :
    T extends ts.ClassExpression ? ClassExpression :
    T extends ts.ConstructorDeclaration ? ConstructorDeclaration :
    T extends ts.GetAccessorDeclaration ? GetAccessorDeclaration :
    T extends ts.MethodDeclaration ? MethodDeclaration :
    T extends ts.PropertyDeclaration ? PropertyDeclaration :
    T extends ts.SetAccessorDeclaration ? SetAccessorDeclaration :
    T extends ts.ComputedPropertyName ? ComputedPropertyName :
    T extends ts.Identifier ? Identifier :
    T extends ts.QualifiedName ? QualifiedName :
    T extends ts.SyntaxList ? SyntaxList :
    T extends ts.Decorator ? Decorator :
    T extends ts.JSDoc ? JSDoc :
    T extends ts.JSDocAugmentsTag ? JSDocAugmentsTag :
    T extends ts.JSDocClassTag ? JSDocClassTag :
    T extends ts.JSDocParameterTag ? JSDocParameterTag :
    T extends ts.JSDocPropertyTag ? JSDocPropertyTag :
    T extends ts.JSDocReturnTag ? JSDocReturnTag :
    T extends ts.JSDocTypedefTag ? JSDocTypedefTag :
    T extends ts.JSDocTypeTag ? JSDocTypeTag :
    T extends ts.JSDocUnknownTag ? JSDocUnknownTag :
    T extends ts.EnumDeclaration ? EnumDeclaration :
    T extends ts.EnumMember ? EnumMember :
    T extends ts.AsExpression ? AsExpression :
    T extends ts.AwaitExpression ? AwaitExpression :
    T extends ts.CallExpression ? CallExpression :
    T extends ts.CommaListExpression ? CommaListExpression :
    T extends ts.ConditionalExpression ? ConditionalExpression :
    T extends ts.DeleteExpression ? DeleteExpression :
    T extends ts.ImportExpression ? ImportExpression :
    T extends ts.MetaProperty ? MetaProperty :
    T extends ts.NewExpression ? NewExpression :
    T extends ts.NonNullExpression ? NonNullExpression :
    T extends ts.OmittedExpression ? OmittedExpression :
    T extends ts.ParenthesizedExpression ? ParenthesizedExpression :
    T extends ts.PartiallyEmittedExpression ? PartiallyEmittedExpression :
    T extends ts.PostfixUnaryExpression ? PostfixUnaryExpression :
    T extends ts.PrefixUnaryExpression ? PrefixUnaryExpression :
    T extends ts.SpreadElement ? SpreadElement :
    T extends ts.SuperElementAccessExpression ? SuperElementAccessExpression :
    T extends ts.SuperExpression ? SuperExpression :
    T extends ts.SuperPropertyAccessExpression ? SuperPropertyAccessExpression :
    T extends ts.ThisExpression ? ThisExpression :
    T extends ts.TypeAssertion ? TypeAssertion :
    T extends ts.TypeOfExpression ? TypeOfExpression :
    T extends ts.VoidExpression ? VoidExpression :
    T extends ts.JsxExpression ? JsxExpression : Node<T>;

class Node<NodeType extends ts.Node = ts.Node> {
    compilerNode!: NodeType;
}

class JsxExpression extends Node<ts.JsxExpression> {}
class ArrayBindingPattern extends Node<ts.ArrayBindingPattern> {}
class BindingElement extends Node<ts.BindingElement> {}
class ObjectBindingPattern extends Node<ts.ObjectBindingPattern> {}
class ClassDeclaration extends Node<ts.ClassDeclaration> {}
class ClassExpression extends Node<ts.ClassExpression> {}
class ConstructorDeclaration extends Node<ts.ConstructorDeclaration> {}
class GetAccessorDeclaration extends Node<ts.GetAccessorDeclaration> {}
class MethodDeclaration extends Node<ts.MethodDeclaration> {}
class PropertyDeclaration extends Node<ts.PropertyDeclaration> {}
class SetAccessorDeclaration extends Node<ts.SetAccessorDeclaration> {}
class ComputedPropertyName extends Node<ts.ComputedPropertyName> {}
class Identifier extends Node<ts.Identifier> {}
class QualifiedName extends Node<ts.QualifiedName> {}
class SyntaxList extends Node<ts.SyntaxList> {}
class Decorator extends Node<ts.Decorator> {}
class JSDoc extends Node<ts.JSDoc> {}
class JSDocAugmentsTag extends Node<ts.JSDocAugmentsTag> {}
class JSDocClassTag extends Node<ts.JSDocClassTag> {}
class JSDocParameterTag extends Node<ts.JSDocParameterTag> {}
class JSDocPropertyTag extends Node<ts.JSDocPropertyTag> {}
class JSDocReturnTag extends Node<ts.JSDocReturnTag> {}
class JSDocTypedefTag extends Node<ts.JSDocTypedefTag> {}
class JSDocTypeTag extends Node<ts.JSDocTypeTag> {}
class JSDocUnknownTag extends Node<ts.JSDocUnknownTag> {}
class EnumDeclaration extends Node<ts.EnumDeclaration> {}
class EnumMember extends Node<ts.EnumMember> {}
class AsExpression extends Node<ts.AsExpression> {}
class AwaitExpression extends Node<ts.AwaitExpression> {}
class CallExpression extends Node<ts.CallExpression> {}
class CommaListExpression extends Node<ts.CommaListExpression> {}
class ConditionalExpression extends Node<ts.ConditionalExpression> {}
class DeleteExpression extends Node<ts.DeleteExpression> {}
class ImportExpression extends Node<ts.ImportExpression> {}
class MetaProperty extends Node<ts.MetaProperty> {}
class NewExpression extends Node<ts.NewExpression> {}
class NonNullExpression extends Node<ts.NonNullExpression> {}
class OmittedExpression extends Node<ts.OmittedExpression> {}
class ParenthesizedExpression extends Node<ts.ParenthesizedExpression> {}
class PartiallyEmittedExpression extends Node<ts.PartiallyEmittedExpression> {}
class PostfixUnaryExpression extends Node<ts.PostfixUnaryExpression> {}
class PrefixUnaryExpression extends Node<ts.PrefixUnaryExpression> {}
class SpreadElement extends Node<ts.SpreadElement> {}
class SuperElementAccessExpression extends Node<ts.SuperElementAccessExpression> {}
class SuperExpression extends Node<ts.SuperExpression> {}
class SuperPropertyAccessExpression extends Node<ts.SuperPropertyAccessExpression> {}
class ThisExpression extends Node<ts.ThisExpression> {}
class TypeAssertion extends Node<ts.TypeAssertion> {}
class TypeOfExpression extends Node<ts.TypeOfExpression> {}
class VoidExpression extends Node<ts.VoidExpression> {}

If you remove the first condition in the type alias (T extends ts.ArrayBindingPattern ? ArrayBindingPattern :) you will see it correctly resolve to JsxExpression.

Expected behavior: Resolves to JsxExpression

Actual behavior: Resolves to any.

I was code generating all this with ts-simple-ast until it failed (see here), so I tried randomly shuffling the array of type names I was generating it with to see if what was in the conditions mattered, but it always seemed to fail after 51 classes. (Edit: Updated example to step through the array of nodes and they all fail at 51 classes. Actually, in some cases when shuffling it would differ, but that was just because it happened to match a base type.)

My declaration files keep being generated with any types and occasionally objects I use internally are typed as any because of this issue. Perhaps there's a better way I can do this mapping? Thanks!

@AnyhowStep
Copy link
Contributor

AnyhowStep commented Nov 25, 2018

What happens if you change them all to be,

| (T extends ts.ThisExpression ? ThisExpression : never)
| (T extends ts.PropertyAccessExpression ? PropertyAccessExpression : never)
//etc.
//etc.
//etc.

So, instead of chaining conditional types, you make it a union of conditional types.

@weswigham
Copy link
Member

Haaaaaaah, this type quietly hits the instantion depth limit we added all on it's own. 50 miiiight be too low if machine-generated types are kept in mind.

@weswigham weswigham added the Bug A bug in TypeScript label Nov 26, 2018
@weswigham
Copy link
Member

weswigham commented Nov 26, 2018

@ahejlsberg think we can increase the limit or be smarter about what we count so structures like this type-based case block can actually work?

@dsherret
Copy link
Contributor Author

dsherret commented Nov 27, 2018

Oh yeah, you're right that it's just a depth of 50. Here's a type with no dependencies that shows the behaviour (generated via this code):

type ToString<T> = T extends 51 ? "51" :
    T extends 50 ? "50" :
    T extends 49 ? "49" :
    T extends 48 ? "48" :
    T extends 47 ? "47" :
    T extends 46 ? "46" :
    T extends 45 ? "45" :
    T extends 44 ? "44" :
    T extends 43 ? "43" :
    T extends 42 ? "42" :
    T extends 41 ? "41" :
    T extends 40 ? "40" :
    T extends 39 ? "39" :
    T extends 38 ? "38" :
    T extends 37 ? "37" :
    T extends 36 ? "36" :
    T extends 35 ? "35" :
    T extends 34 ? "34" :
    T extends 33 ? "33" :
    T extends 32 ? "32" :
    T extends 31 ? "31" :
    T extends 30 ? "30" :
    T extends 29 ? "29" :
    T extends 28 ? "28" :
    T extends 27 ? "27" :
    T extends 26 ? "26" :
    T extends 25 ? "25" :
    T extends 24 ? "24" :
    T extends 23 ? "23" :
    T extends 22 ? "22" :
    T extends 21 ? "21" :
    T extends 20 ? "20" :
    T extends 19 ? "19" :
    T extends 18 ? "18" :
    T extends 17 ? "17" :
    T extends 16 ? "16" :
    T extends 15 ? "15" :
    T extends 14 ? "14" :
    T extends 13 ? "13" :
    T extends 12 ? "12" :
    T extends 11 ? "11" :
    T extends 10 ? "10" :
    T extends 9 ? "9" :
    T extends 8 ? "8" :
    T extends 7 ? "7" :
    T extends 6 ? "6" :
    T extends 5 ? "5" :
    T extends 4 ? "4" :
    T extends 3 ? "3" :
    T extends 2 ? "2" :
    "1";
type Result = ToString<1>;

It would be nice to be warned about this behaviour in some way (maybe resolve to an unknown type instead of any?). In the meantime, I believe I could use @AnyhowStep's recommendation and flatten the type out. I think I'll still need a few conditionals in it to prevent base types from being included in the union though (edit: unfortunately this isn't exactly the same as a nested conditional. I don't believe there is a way to fall back to Node<T> without including it in a union and creating a fallback with an outer conditional type doesn't seem to work because it won't pick up individual types in a passed in union types that fall back to Node<T>)

@dsherret
Copy link
Contributor Author

dsherret commented Dec 4, 2018

Just following up on this and it no longer affects me. @Gerrit0 helped me out and I managed to shorten my conditional type to the type shown in this post: dsherret/ts-morph#507 (comment)

This works in my situation because of the kind property on nodes so the conditional type is right now ~20 conditions. I wouldn't be surprised if someone hit this limit in the future again though.

@sandersn sandersn added this to the TypeScript 3.4.0 milestone Dec 11, 2018
@ahejlsberg
Copy link
Member

With #29435 we now report an error on instantiations that are over 50 levels deep instead of silently resolving to any. The limit itself still seems reasonable, so I'm going to close this as a design limitation.

@ahejlsberg ahejlsberg added Design Limitation Constraints of the existing architecture prevent this from being fixed and removed Bug A bug in TypeScript labels Jan 29, 2019
@ahejlsberg ahejlsberg removed this from the TypeScript 3.4.0 milestone Jan 29, 2019
@AnyhowStep
Copy link
Contributor

AnyhowStep commented Jan 29, 2019

In #29511 , I had problems with the instantiation depth. I've opened up a PR #29602 to add a compiler option to increase the max depth.

The library I'm working on, here (v3 branch is the only good branch), doesn't run into the limit. But projects that use it would probably run into the depth limit after having deeply nested expressions.

Of course, the best thing to do would be to not have such complicated types in the first place but, I think, in my case, I couldn't help it because I'm checking MySQL queries for validity during compile time.

@dsherret
Copy link
Contributor Author

@ahejlsberg thanks! Being warned about this limit is much better. When I first encountered it I just assumed the language service was having a moment and the problem remained in the library for several months.

Anyway, doesn't affect me anymore, but I can imagine it will affect others down the road (https://twitter.com/andygocke/status/1067488906763878400).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Design Limitation Constraints of the existing architecture prevent this from being fixed
Projects
None yet
5 participants