This narrowing typeguard effect bleeds into subsequent statments on a type with bivariant type-parameter

[dragomirtitian]

TypeScript Version: 3.4.0-dev.201xxxxx (although 3.3 is also impacted)
Search Terms: this type guards

If we create a type-guard that narrows this on a type that has a type-parameter that is present only in a bivariant position, the effect of the type guard persists outside of the guarded block.

Code

type GetKnownKeys<G> = G extends GuardedMap<infer KnownKeys>  ? KnownKeys: never;
interface GuardedMap<KnownKeys extends string> {
    get(k: KnownKeys): number;
    has<S extends string>(k: S): this is GuardedMap<S | GetKnownKeys<this>>;
}

declare let map: GuardedMap<never>;

map.get('bar') // err, as expected
if (map.has('foo')) {
    map.get('foo').toExponential(); // ok as expected
    if(map.has('bar')) 
    {
        map.get('foo').toExponential(); // ok as expected
        map.get('bar').toExponential(); // ok as expected
    }
        
    map.get('bar').toExponential(); /// OK!?!?! WHY ?!
    
}
map.get('bar') // OK ?!

Expected behavior:
Type guard only impacts the guarded block.
Actual behavior:
The effect of the type guard bleads into all subsequent statements. (marked with OK!?!?! and OK?!)

Note: With strictFunctionTypes on, declaring get as get: (k: KnownKeys) => number; makes the code work as expected.

Playground Link: link

Related Issues: Similar to #14817

Found this while playing with a solution for #9619

[jack-williams]

I think this is a design limitation and ultimately an issue with bivariance. I think what is going on is that the type of map at the offending line is a combination of flows. The first flow is when map.has('bar') is false, giving a type GuardedMap<"foo">; the second flow is when map.has('bar') is true, giving a type GuardedMap<"foo" | "bar>. The types that arise from these flows are combined in a union type, which are then reduced using subtype reduction.

Under bivariance GuardedMap<"foo"> is a subtype of GuardedMap<"foo" | "bar"> because "foo" is a subtype of "foo" | "bar". Under strict types, GuardedMap<"foo" | "bar"> is a subtype of GuardedMap<"foo"> because the union type occurs in a contravariant position.

As a small way to see this:

type A = GuardedMap<"bar" | "foo">
type B = GuardedMap<"foo">
type C = A | B;

declare const a: A;
declare const b: B;

// w has type GuardedMap<"foo"> under strict checks
// but, GuardedMap<"bar" | "foo"> without.
const w = Math.random() > 0.5 ? a : b;

I'm not sure there is a clean way to fix this without being overly disruptive, but someone who knows the narrowing code better may have something else to say. Perhaps there is some targeted fix specifically for this narrowing?

[dragomirtitian]

@jack-williams Are you sure about the difference between strict on/off and the type of w ? I am seeing the exact same type with my original code in both cases. Since get is a member method I would not expect strictFunctionTypes to impact GuardedMap but I could be wrong.

From my debugging the subsequent statement is indeed impacted by both branches of the if. One branch will be never (the else branch) the other branch will be GuardedMap<"foo">, and since the resulting flow type is just a union of the two, the resulting type will be GuardedMap<"foo">.

Digging a bit deeper the else branch is never because on the else branch isRelated(GuardedMap<never>, GuardedMap<"foo">) will be true and thus the compiler will asume the type in else will be never (since if the types are related the then branch should have been taken and what is left is never)

I understand some of the mechanics of the result, but the result is surprising.

Is the fact that the flow type in subsequent statements is the union of both branch outcomes useful in this scenario? I don't think so. Not sure I can carve out the situations when this is useful from the ones it is not though.

Might be a corner of corner cases and not worth the hassle of dealing with it in any better way, but I though I might document it here in case someone else comes across it and see if anyone thinks it worth fixing :)

[jack-williams]

Are you sure about the difference between strict on/off and the type of w ? I am seeing the exact same type with my original code in both cases. Since get is a member method I would not expect strictFunctionTypes to impact GuardedMap but I could be wrong.

Sorry, I was assuming that get was written as get: (k: KnownKeys) => number;. If it's written as a method then you're stuck with bivariance irrespective of strictness mode so there is no way to see the different behaviour.

Is the fact that the flow type in subsequent statements is the union of both branch outcomes useful in this scenario? I don't think so. Not sure I can carve out the situations when this is useful from the ones it is not though.

It's probably not useful in this scenario, but across the board it's generally useful and because it's compositional it's easier to implement. Where you see the benefit is when you have multiple branches, some of which return. In that case, gathering the flow types in a union is simple and effective. In your example, the problem only arises due to unsoundness in the type system.

Might be a corner of corner cases and not worth the hassle of dealing with it in any better way, but I though I might document it here in case someone else comes across it and see if anyone thinks it worth fixing :)

It sort of feels like that, but maybe there is an easy fix. I think the problem is that any fix effectively amounts to a heuristic. I agree that documenting it, at the least, is worth the time.

[RyanCavanaugh]

Extremely good analysis @jack-williams

Here's a simplified repro with fewer things going on

type Animal = { a: "" };
type Cat = Animal & { c: "" };
type Dog = Animal & { d: "" };

type Box<T> = {
    get(x: T): void;
}

declare function isCatBox(x: any): x is Box<Cat>;

function fn(arg: Box<Cat | Dog>) {
    arg.get({ a: "", d: "" }); // OK
    if (isCatBox(arg)) {

    }
    arg.get({ a: "", d: "" }); // Error
}

As for documentation, I have no idea how you would write this down in a way that anyone could find it unless they already knew what the doc said.

[RyanCavanaugh]

@weswigham @ahejlsberg this is quite the interesting example FYI

[jack-williams]

I'm curious to see what RWC would break if the union from branch joins was reduced using strict function types regardless of the current compiler flag. Though any change such as this would not affect the original example because method typing is beyond the reach of the strict flag.

[weswigham]

I think the fix here might be to make the subtype relationship always behave strictly (even for methods), but still allow bivariance in the assignment relationship.

[RyanCavanaugh]

Prototyping to see what happens

[RyanCavanaugh]

Other places do depend on subtype reduction on methods:

// Now an error
(''.match(/ /) || []).map(s => s.toLowerCase());

[weswigham]

If we fixed our union call signature generic handling to be a bit better, that wouldn't need to reduce, since never | string (assuming [] is typed as a never[]) is just string. We've still got incentive to do that for other reasons (eg, unions of generic react components) - we just need to figure out how we should merge generics across two separate map signatures when constructing the union signature.