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

Wrong branch on conditional type with ReturnType of T condition #39364

Closed
arogg opened this issue Jul 1, 2020 · 9 comments
Closed

Wrong branch on conditional type with ReturnType of T condition #39364

arogg opened this issue Jul 1, 2020 · 9 comments
Assignees
Labels
Needs Investigation This issue needs a team member to investigate its status.

Comments

@arogg
Copy link

arogg commented Jul 1, 2020

I think I found a bug.

Last non-broken TS version for this is 3.1.6 on typescript playground. All later versions are broken.

Which gets me thinking: is this maybe intended behavior because it is "broken" for so many versions?

But i dont know why it should NOT be a bug. Either way if it's indeed not a bug, i need some serious brain rewiring from you guys please, because cant wrap my head around this behavior...

Let's get started

TypeScript Version: Nightly, all the way down to and including 3.3.3, tested with all these versions on playground

Search Terms: conditional types return type

Code

type StringOrNumber<T extends () => any> = [ReturnType<T>] extends [string]
  ? string
  : number;

type Test = StringOrNumber<() => number>; // oh noes, "string"

Expected behavior:
I expect number. Because [number] just cannot extend [string]

The TS handbook talks about deferred evaluation of conditional types. I seems it is not working here!! Evaluation of "ReturnType" should be deferred until the type is actually used in a concrete way!

Actual behavior:
The conditional type evaluates to string no matter what function type is passed in

type StringOrNumber2<T extends () => number> = [ReturnType<T>] extends [string]
  ? string
  : number;

type Test2 = StringOrNumber2<() => number>; // oh noes, "string"

Expected behavior:
I expect number, even the generic constaint now demands it!

Actual behavior:
Again, always get "string"

type StringOrNumber3<T extends () => number> = [number] extends [string]
  ? string
  : number;

type Test3 = StringOrNumber3<() => number>; // correct, number

This is just for show, I expect number and get number, only difference is I now hardcode "number" instead of "ReturnType"

Playground Link: https://www.typescriptlang.org/play/?ts=Nightly#code/FAkFwTwBwUwAgMpgE4EsB2BzA8sgcgK4C2ARjMgDwAqcMAHmDOgCYDOcAFAJRwC8AfHACG6CIN5wA2gCUYYAsnRVoMavwC6tBkzZTWKDJnXA4cAPxx9aLCbgAuOOmJlkAbnAq4VGPr6IDWLiEpOQU3HyCTiHI-K5wAPTxcAD2ABaOyT4ANHAARFaGuaDAkLD+1jj4zuQATNRajCzs4QKO1TF+MnIKSipqmvSNupIFWMamFqOYtg5RLu4lnt76NX5IFUHtdS2R7bEJSWkZ2XlTRaCl8OuGm9EAzPWDOs08rXPk4lLvyAPaTXoBIy2SaAmZtaILS5eHxgO5rQG3FwPHbglz7RJwADGyWQyBgmLAOW+xRJQA

Related Issues: no, searched the issues

@RyanCavanaugh RyanCavanaugh added the Needs Investigation This issue needs a team member to investigate its status. label Jul 1, 2020
@RyanCavanaugh
Copy link
Member

Normally this is where I say "Ah, here's the subtle thing you missed", but... yeah that seems totally broken and I don't understand how it's been that way for so long without anyone else noticing?

@ahejlsberg
Copy link
Member

The root issue here is the way ReturnType<T> is defined:

type ReturnType<T extends (...args: any) => any> = T extends (...args: any) => infer R ? R : any;

As we're deciding whether to defer resolution of StringOrNumber<T>, we check whether the most restrictive instantiation of [ReturnType<T>] is assignable to [string]. Unfortunately, the constraint of the most restrictive instantiation of ReturnType<T> ends up being the any in the false branch. We take that to mean that ReturnType<T> will be assignable to string for any T, and so we don't defer resolution of the type.

Changing the any in the false branch to unknown causes things to work correctly. Arguably it should have been that way all along. Don't know how much of a breaking change it would be to fix it.

@arogg
Copy link
Author

arogg commented Jul 3, 2020

@ahejlsberg I would understand if ReturnType<T> would unconditionally always resolve to any, no matter the input. But that is not the case. I don't understand why resolution is not deferred if there is the possibility that infer R ? R : any can resolve to R (and not any). If there is the possibility that the whole conditional type can resolve differently, depending on T (and in turn, R), it should always be deferred, IMHO.

@ahejlsberg
Copy link
Member

@arogg Yes, ideally resolution of the type in your example would be deferred. However, it is surprisingly complex to devise rules that accurately measure whether an extends check might be affected by instantiation. The current scheme is an approximation that gets it right in the vast majority of cases while also remaining insensitive to generics that don't affect the outcome. So, it's basically a design limitation, but we should continue to think about it.

@ahejlsberg
Copy link
Member

Looking at this a bit closer, where things go wrong is when we're relating the restrictive instantiation of [ReturnType<T>] to [string]. We have a check in place to not explore the distributive constraint of a restrictive instantiation of a conditional type, but that check doesn't work right when the conditional type is wrapped in a tuple. We really want to rule out distributive constraints of any conditional type, so a solution might be to reintroduce the definitelyAssignableRelation we once had and use that to exclude distributive constraints.

@jcalz
Copy link
Contributor

jcalz commented Nov 20, 2020

Is this the same problem?

type What<K extends string> =
    { x: { y: 0, z: 1 } } extends { x: { [P in K]: 0 } } ? true : false;
// What<K> is eagerly simplified to false before instantiation of K

type Huh = What<"y"> // expected: true, actual: false. 

Playground link

I'm trying to figure out if the above should get its own issue filed or if it belongs on an existing one. From this SO question.

@ahejlsberg
Copy link
Member

@jcalz That appears to be a different issue. When determining whether to defer resolution of the conditional type we relate the "most permissive instantiations" of the check and extends types. The constraint of the most permissive instantiation of the extends type ends up being { x: { [index: string]: 0 } }, but really it should be { x: { } }. It's a simple fix and I'll include it in this PR. But feel free to put up a separate issue for it.

@ahejlsberg
Copy link
Member

@jcalz Decided to put up a separate PR for that issue: #41622.

@jcalz
Copy link
Contributor

jcalz commented Mar 5, 2024

Looks like this got fixed along the way somewhere. Still other issues exist, but not sure if we should open a new one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Investigation This issue needs a team member to investigate its status.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants