You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
TypeScript Version: 3.1.6, 3.2.1, the current playground (3.3?), and next as of Feb 28
Search Terms:
conditional types are incorrectly
Code
interfaceA{foo();}interfaceB{bar();}functiontest1<TextendsA>(x: T,y: TextendsB ? number : string){if(typeofy=='string'){y;}else{y;// never ?}constnewY: string|number=y;newY;// just string}functiontest2<TextendsA>(x: T,y: TextendsB ? string : number){if(typeofy=='string'){y;// never ?}else{y;}constnewY: string|number=y;newY;// just number }
Expected behavior: T extends B ? string : number should either be left unchanged, or rounded up to string|number: I think the issue stems from incorrect inference that T extends B is false given T extends A (while they're just unrelated interfaces that have a non-empty intersection). The test case below is as far as I've managed to reduce the problem.
Actual behavior:
The T extends A constraint seems to make TS guess T extends B is always false, and so the a?b:c type behaves as c.
Related Issues: #29939 looks slightly similar, but I don't see the same constraints when playing around with my example, so I'm not sure it's the same.
Ye this looks broken to me (and it's not related to the linked issue). The problem isn't the narrowing but the computed constraint of the conditional type. Here is a slightly smaller repro that shows the unsoundness you get:
Computing the constraint of the conditional type replaces T with its constraint. When resolving the conditional type this makes it always false, so it simplifes to A. Here is the relevant code:
functiongetConstraintOfDistributiveConditionalType(type: ConditionalType): Type|undefined{// Check if we have a conditional type of the form 'T extends U ? X : Y', where T is a constrained// type parameter. If so, create an instantiation of the conditional type where T is replaced// with its constraint. We do this because if the constraint is a union type it will be distributed// over the conditional type and possibly reduced. For example, 'T extends undefined ? never : T'// removes 'undefined' from T.if(type.root.isDistributive){constsimplified=getSimplifiedType(type.checkType);constconstraint=simplified===type.checkType ? getConstraintOfType(simplified) : simplified;if(constraint&&constraint!==type.checkType){constmapper=makeUnaryTypeMapper(type.root.checkType,constraint);constinstantiated=getConditionalTypeInstantiation(type,combineTypeMappers(mapper,type.mapper));if(!(instantiated.flags&TypeFlags.Never)){returninstantiated;}}}returnundefined;}
When replacing T with the constraint it is safe to simplify to the true branch if the constraint in assignable to the extends type (ignoring any), but it is not safe to simplify to the false branch if the constraint is not assignable to the extends type.
Could a flag be passed to getConditionalType (or we change to a different form of conditional type) that encodes the notion that the check type is currently an upper approximation, rather than something we know. When the approximation flag is passed we simplify to the true branch when true, but we don't simplify to the false branch when false. Here is what I mean: 78798cd (it's a hack).
Not sure I'm a huge fan of this though. Effectively there are two sets of behaviours. One when computing the constraint of a conditional type that uses the constraint of the parameter, then the normal instantiation path that uses the restrictive instantiation.
There are also issues when the constraint is related to the extends type. This boils down to the same issues of transitivity, and why the restrictive instantiation was introduced.
Should be mapped to ImmutableMap always, independently of the type of T?
I don't think so. See this commentary on instantiating mapped types.
// For a homomorphic mapped type { [P in keyof T]: X }, where T is some type variable, the mapping// operation depends on T as follows:// * If T is a primitive type no mapping is performed and the result is simply T.// ….
So given ImmutableFromJS<{ [P in keyof T]: { [id: string]: T[P]; }; }> where T is instantiated as a primitive, then it expands to ImmutableFromJS<T>.
I think part of the problem is that in the cases of arg2 and arg3 you are relying on the
check in ImmutableFromJS to get normalised away, but conditional types wont simplify if the check is a generic mapped type.
// We attempt to resolve the conditional type only when the check and extends types are non-genericif(!checkTypeInstantiable&&!maybeTypeOfKind(inferredExtendsType,TypeFlags.Instantiable|TypeFlags.GenericMappedType)){
Because the Primitive clause isn't normalised away you still have the type { [P in keyof T]: { [id: string]: T[P]; }; } hanging around in the union constraint of ImmutableFromJS<{ [P in keyof T]: { [id: string]: T[P]; }; }>, which is why you cant access toJS.
If T is a primitive type no mapping is performed and the result is simply T
Either this is a bug or I don't fully understand what that statement means, but looks like this statement isn't true.
Mapping primitive to something entirely not like a primitive seems to produce results I expect in 3.4:
declareconsttst: {[Pinkeyofnumber]: {[id: string]: number[P];};};declareconstnum: number;num.toString.asfdsdf// Error as expected: toString is a function without that propertytst.toString.asdfdsf;// No error as expected: toString is a key in resulting object which is indexed by any stringtst.toString.asdfdsf.sdfdsf;// Error as expected: that index has a certain type, which doesn't has a propertytst.toString.asdfdsf().charAt(0);// No error as expected tst+num// Fails as expected
If the statement is indeed not true then mapping must happen and Primitive case should never be hit.
Same issue. I've broken this down to the simplest illustration:
functionfoo<Textendsnumber|string>(value: T):
Textendsnumber ? string : number{if(typeofvalue==='number'){returnvalue.toString()// see Branch 1 below}else{returnparseInt(value,10)// see Branch 2 below}}
Branch 1
Expected:value is narrowed to type number
Actual:value is of type T & number.
Type 'string' is not assignable to type 'T extends number ? string : number'.ts(2322)
Branch 2
Expected:value is narrowed to type string
Actual:value is of type T extends string | number
Argument of type 'T' is not assignable to parameter of type 'string'.
Type 'string | number' is not assignable to type 'string'.
Type 'number' is not assignable to type 'string'.
Type '"foo"' is not assignable to type 'T extends true ? string : number'.ts(2322)
Type '42' is not assignable to type 'T extends true ? string : number'.ts(2322)
Interestingly, the code below works, but is quite repetitive!
jedmao TypeScript doesn't infer conditional types by design; I have a basic implementation but it's way off being practical: #30284 See the issue #24929 for reasons why it's not easy to implement.
The issue in the OP is different though. There the conditional type appears on the source side of an assignability check, while your examples have the conditional type appear on the target side of the check. The bug in this issue is regarding how TypeScript simplifies the constraint of a conditional type appearing on the source side.
typeA<T>=Textends'a'|'b' ? number : never;functionfoo<Textends'a'|'b'>(): A<T>{// Typescript error: 5 is not assignable to A<T>;return5;}functionbar(): A<'a'|'b'>{// OKreturn5;}
Looks like it doesn't respect generic type constraints.
Even this one doesn't work:
typeA<Textends'a'|'b'>=Textends'a'|'b' ? number : never;functionfoo<Textends'a'|'b'>(): A<T>{// Typescript error: 5 is not assignable to A<T>;return5;}
typeA<T>=Textends'a'|'b' ? number : never;functionfoo<Textends'a'|'b'>(): A<T>{// Typescript error: 5 is not assignable to A<T>;return5;}functionbar(): A<'a'|'b'>{// OKreturn5;}
Looks like it doesn't respect generic type constraints.
Even this one doesn't work:
typeA<Textends'a'|'b'>=Textends'a'|'b' ? number : never;functionfoo<Textends'a'|'b'>(): A<T>{// Typescript error: 5 is not assignable to A<T>;return5;}
Got same problem here, value cannot be assigned to conditional return type.
interfaceA{}interfaceB{}typeAB<TextendsA|B>=TextendsA ? A : B;functionfoo<TextendsA|B>(param: AB<T>) : AB<T>{returnparam;}functionbar<SextendsA|B>(param : A) : AB<S>{consta=foo(param);// Type 'A' is not assignable to type 'AB<S>'returna;}
While I agree that it is incorrect to compute the constraint of a conditional type applied to a constrained type variable by simply applying the conditional type to the type variable's constraint (because that only considers the upper bound of the type variable), I do actually think the behavior of the original repro is defensible. For example, given variables a and b of type A and B as declared in the original repro, the operation a === b would be an error because we don't consider A and B comparable. In other words, we treat A and B as disjoint types because they share no properties. While it is possible to construct a type that is both an A and a B, it likely wasn't the intent of the conditional type to cover that.
However, when A and Bare comparable, we definitely should compute a constraint that includes both branches of the conditional type. For example
interfaceA{foo();}interfaceB{foo();bar();}functiontest1<TextendsA>(y: TextendsB ? number : string){constn: number=y;// Errorconsts: string=y;// Ok, but should be error}
We currently compute the constraint of T extends B ? number : string to be just string, but it ought to be string | number. A similar, and perhaps more meaningful, example is
typeIsArray<T>=Textendsany[] ? true : false;functionf1<Uextendsobject>(x: IsArray<U>){lett: true=x;// Errorletf: false=x;// Ok, but should be error}
Above, the constraint of IsArray<U> should be boolean (because it could either be true or false), but currently we compute it to be false.
So, I'm going to call this issue a bug, but not for the original repro.
Activity
jack-williams commentedon Feb 28, 2019
Ye this looks broken to me (and it's not related to the linked issue). The problem isn't the narrowing but the computed constraint of the conditional type. Here is a slightly smaller repro that shows the unsoundness you get:
Computing the constraint of the conditional type replaces
Twith its constraint. When resolving the conditional type this makes it always false, so it simplifes toA. Here is the relevant code:jack-williams commentedon Feb 28, 2019
When replacing
Twith the constraint it is safe to simplify to the true branch if the constraint in assignable to the extends type (ignoring any), but it is not safe to simplify to the false branch if the constraint is not assignable to the extends type.Could a flag be passed to
getConditionalType(or we change to a different form of conditional type) that encodes the notion that the check type is currently an upper approximation, rather than something we know. When the approximation flag is passed we simplify to the true branch when true, but we don't simplify to the false branch when false. Here is what I mean: 78798cd (it's a hack).Not sure I'm a huge fan of this though. Effectively there are two sets of behaviours. One when computing the constraint of a conditional type that uses the constraint of the parameter, then the normal instantiation path that uses the restrictive instantiation.
RyanCavanaugh commentedon Feb 28, 2019
Wesley Wigham (@weswigham) Anders Hejlsberg (@ahejlsberg) thoughts?
jack-williams commentedon Feb 28, 2019
There are also issues when the constraint is related to the extends type. This boils down to the same issues of transitivity, and why the restrictive instantiation was introduced.
zpdDG4gta8XKpMCd commentedon Mar 1, 2019
also #29662
remojansen commentedon Apr 11, 2019
We are having an issue at work that could be related:
It seems like when using generics the conditional type is not narrowed down as I would expect.
Should be mapped to
ImmutableMap<T>always, independently of the type of T?Please correct me If I'm not understanding this correctly.
jack-williams commentedon Apr 11, 2019
I don't think so. See this commentary on instantiating mapped types.
So given
ImmutableFromJS<{ [P in keyof T]: { [id: string]: T[P]; }; }>whereTis instantiated as a primitive, then it expands toImmutableFromJS<T>.I think part of the problem is that in the cases of
arg2andarg3you are relying on thecheck in
ImmutableFromJSto get normalised away, but conditional types wont simplify if the check is a generic mapped type.Because the
Primitiveclause isn't normalised away you still have the type{ [P in keyof T]: { [id: string]: T[P]; }; }hanging around in the union constraint ofImmutableFromJS<{ [P in keyof T]: { [id: string]: T[P]; }; }>, which is why you cant accesstoJS.smoogly commentedon Apr 11, 2019
Either this is a bug or I don't fully understand what that statement means, but looks like this statement isn't true.
Mapping primitive to something entirely not like a primitive seems to produce results I expect in 3.4:
If the statement is indeed not true then mapping must happen and
Primitivecase should never be hit.jack-williams commentedon Apr 11, 2019
That commentary exists within the code that instantiates mapped types, that is, replaces type variables with types. Your example:
is a closed type and therefore is not subject to instantiation.
That is to say, when written:
there is more than just a basic inlining of
numberforTgoing on.smoogly commentedon Apr 11, 2019
Right, so I did not understand the meaning of that indeed.
But I think it would make sense if
Behaved like
jednano commentedon May 20, 2019
Same issue. I've broken this down to the simplest illustration:
Branch 1
valueis narrowed to typenumbervalueis of typeT & number.Branch 2
valueis narrowed to typestringvalueis of typeT extends string | numberjednano commentedon Aug 30, 2019
Another super simple example:
Interestingly, the code below works, but is quite repetitive!
Furthermore, it would be super ideal if TS could possibly infer the return type with some kind of narrowing?
I would expect the inferred declaration to be:
Still seeing this issue with TS 3.6.2.
jednano commentedon Aug 30, 2019
If it's helpful, I can submit a PR with a breaking test:
jack-williams commentedon Sep 2, 2019
jedmao TypeScript doesn't infer conditional types by design; I have a basic implementation but it's way off being practical: #30284 See the issue #24929 for reasons why it's not easy to implement.
The issue in the OP is different though. There the conditional type appears on the source side of an assignability check, while your examples have the conditional type appear on the target side of the check. The bug in this issue is regarding how TypeScript simplifies the constraint of a conditional type appearing on the source side.
jednano commentedon Sep 3, 2019
Related: #22735
akozhemiakin commentedon Aug 3, 2020
Another example of incorrectly narrowed type:
Looks like it doesn't respect generic type constraints.
Even this one doesn't work:
icy0307 commentedon Sep 15, 2020
Got same problem here, value cannot be assigned to conditional return type.
ahejlsberg commentedon Oct 3, 2023
While I agree that it is incorrect to compute the constraint of a conditional type applied to a constrained type variable by simply applying the conditional type to the type variable's constraint (because that only considers the upper bound of the type variable), I do actually think the behavior of the original repro is defensible. For example, given variables
aandbof typeAandBas declared in the original repro, the operationa === bwould be an error because we don't considerAandBcomparable. In other words, we treatAandBas disjoint types because they share no properties. While it is possible to construct a type that is both anAand aB, it likely wasn't the intent of the conditional type to cover that.However, when
AandBare comparable, we definitely should compute a constraint that includes both branches of the conditional type. For exampleWe currently compute the constraint of
T extends B ? number : stringto be juststring, but it ought to bestring | number. A similar, and perhaps more meaningful, example isAbove, the constraint of
IsArray<U>should beboolean(because it could either betrueorfalse), but currently we compute it to befalse.So, I'm going to call this issue a bug, but not for the original repro.