Conditional types are incorrectly narrowed

[rjamet]

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

interface A { foo(); }
interface B { bar(); }

function test1<T extends A>(x: T, y: T extends B ? number : string) {
    if (typeof y == 'string') {
        y;
    } else {
        y; // never ?
    }
    const newY: string | number = y;
    newY;  // just string
}

function test2<T extends A>(x: T, y: T extends B ? string : number) {
    if (typeof y == 'string') {
        y; // never ?
    } else {
        y; 
    }
    const newY: 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.

Playground Link: (playground)

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.

[jack-williams]

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:

interface A { a: string }
interface B { b: boolean }

function test1<T extends A>(y: T extends B ? B : A): string {
    return y.a;
}

test1<{ a: string, b: boolean }>({ b: true });

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:

function getConstraintOfDistributiveConditionalType(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) {
        const simplified = getSimplifiedType(type.checkType);
        const constraint = simplified === type.checkType ? getConstraintOfType(simplified) : simplified;
        if (constraint && constraint !== type.checkType) {
            const mapper = makeUnaryTypeMapper(type.root.checkType, constraint);
            const instantiated = getConditionalTypeInstantiation(type, combineTypeMappers(mapper, type.mapper));
            if (!(instantiated.flags & TypeFlags.Never)) {
                return instantiated;
            }
        }
    }
    return undefined;
}

[jack-williams]

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.

[RyanCavanaugh]

@weswigham @ahejlsberg thoughts?

[jack-williams]

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.

type HasXNum<T> = T extends { x: number } ? string : number;

function test1<T extends { x: any }>(y: HasXNum<T>): string {
    return y;
}

const fakeString: string = test1<{ x: boolean }>(3);

[zpdDG4gta8XKpMCd]

also #29662

[remojansen]

We are having an issue at work that could be related:

type Primitive = number | string | boolean | null | undefined | Symbol | Function;

export interface ImmutableMap<T> {
    // ...
    toJS(): T;
}

export interface ImmutableList<T extends Array<any>> {
    // ...
    toJS(): T;
}

export type ImmutableFromJS<T> = T extends Primitive ? T
    : T extends Array<any> ? ImmutableList<T>
    : T extends object ? ImmutableMap<T>
    : never;

type Sometype<T> = ImmutableFromJS<{ [P in keyof T]: { [id: string]: T[P]; }; }>;

declare let a: Sometype<object>;
a.toJS(); // OK

declare let b: Sometype<number[]>;
a.toJS(); // OK

declare let c: Sometype<Primitive>;
c!.toJS(); // Error (Not expected)

function test<
    T1,
    T2 extends object,
    T3 extends number[],
    T4 extends Primitive
>(
    arg1: Sometype<T1>,
    arg2: Sometype<T2>,
    arg3: Sometype<T3>,
    arg4: Sometype<T4>
) {
    arg1.toJS(); // Error (Not expected)
    arg2.toJS(); // Error (Not expected)
    arg3.toJS(); // Error (Not expected)
    arg4.toJS(); // Error (Not expected)
}

It seems like when using generics the conditional type is not narrowed down as I would expect.

ImmutableFromJS<{ [P in keyof T]: { [id: string]: T[P]; }; }>

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]

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

{ [P in keyof T]: { [id: string]: T[P]; }; } extends Primitive ?

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-generic
if (!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.

[smoogly]

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:

declare const tst: { [P in keyof number]: { [id: string]: number[P]; }; };
declare const num: number;
num.toString.asfdsdf // Error as expected: toString is a function without that property
tst.toString.asdfdsf; // No error as expected: toString is a key in resulting object which is indexed by any string
tst.toString.asdfdsf.sdfdsf; // Error as expected: that index has a certain type, which doesn't has a property
tst.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.

[jack-williams]

That commentary exists within the code that instantiates mapped types, that is, replaces type variables with types. Your example:

declare const tst: { [P in keyof number]: { [id: string]: number[P]; }; };

is a closed type and therefore is not subject to instantiation.

That is to say, when written:

type Mapped<T> = { [P in keyof T]: { [id: string]: T[P]; }; };
declare const tst: Mapped<number>;

there is more than just a basic inlining of number for T going on.

[smoogly]

Right, so I did not understand the meaning of that indeed.
But I think it would make sense if

type Mapped<T> = { [P in keyof T]: { [id: string]: T[P]; }; };
declare const tst: Mapped<number>;

Behaved like

declare const tst: { [P in keyof number]: { [id: string]: number[P]; }; };

[jednano]

Same issue. I've broken this down to the simplest illustration:

function foo<T extends number | string>(value: T):
	T extends number ? string : number {
	if (typeof value === 'number') {
		return value.toString() // see Branch 1 below
	} else {
		return parseInt(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'.

[jednano]

Another super simple example:

function foo<T extends boolean>(value: T): T extends true ? string : number {
	if (value === true) {
		return 'foo'
	}
	return 42
}

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!

function foo<T extends boolean>(value: T): T extends true ? string : number {
	if (value === true) {
		return 'foo' as T extends true ? string : number
	}
	return 42 as T extends true ? string : number
}

Furthermore, it would be super ideal if TS could possibly infer the return type with some kind of narrowing?

function foo(value: boolean) {
	return (value === true) ? 'foo' : 42
}

I would expect the inferred declaration to be:

declare function foo<T extends boolean>(value: T): T extends true ? string : number;

Still seeing this issue with TS 3.6.2.

[jednano]

If it's helpful, I can submit a PR with a breaking test:

// @strict: true

// Repro from #30152

function f1<T extends boolean>(t: T): T extends true ? string : number {
	return (t === true) ? 's' : 2;
}

function f2<T>(t: T): T extends string ? number : string {
	return (typeof t === 'string') ? +t : t.toString();
}

[jack-williams]

@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]

Related: #22735

[akozhemiakin]

Another example of incorrectly narrowed type:

type A<T> = T extends 'a' | 'b' ? number : never;

function foo<T extends 'a' | 'b'>(): A<T> {
    // Typescript error: 5 is not assignable to A<T>;
    return 5;
}

function bar(): A<'a' | 'b'> {
    // OK
    return 5;
}

Looks like it doesn't respect generic type constraints.

Even this one doesn't work:

type A<T extends 'a' | 'b'> = T extends 'a' | 'b' ? number : never;

function foo<T extends 'a' | 'b'>(): A<T> {
    // Typescript error: 5 is not assignable to A<T>;
    return 5;
}

[icy0307]

Another example of incorrectly narrowed type:

type A<T> = T extends 'a' | 'b' ? number : never;

function foo<T extends 'a' | 'b'>(): A<T> {
    // Typescript error: 5 is not assignable to A<T>;
    return 5;
}

function bar(): A<'a' | 'b'> {
    // OK
    return 5;
}

Looks like it doesn't respect generic type constraints.

Even this one doesn't work:

type A<T extends 'a' | 'b'> = T extends 'a' | 'b' ? number : never;

function foo<T extends 'a' | 'b'>(): A<T> {
    // Typescript error: 5 is not assignable to A<T>;
    return 5;
}

Got same problem here, value cannot be assigned to conditional return type.

interface A {

}

interface B {

}

type AB <T extends A | B > = T extends A ? A : B;


function foo<T extends A | B >(param: AB<T>) : AB<T> {
    return param;
}

function bar <S extends A | B > (param : A) : AB<S>{
    const a = foo(param);
    // Type 'A' is not assignable to type 'AB<S>'
    return a;
}

[ahejlsberg]

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 B are comparable, we definitely should compute a constraint that includes both branches of the conditional type. For example

interface A { foo(); }
interface B { foo(); bar(); }

function test1<T extends A>(y: T extends B ? number : string) {
    const n: number = y;  // Error
    const s: 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

type IsArray<T> = T extends any[] ? true : false;

function f1<U extends object>(x: IsArray<U>) {
    let t: true = x;   // Error
    let f: 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.