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]; }; };