Excess property checking only checks top level when target type is a union

[DHainzl]

When using Discriminated Unions with properties with interfaces as types, TS does not seem to infer the discrimination correctly when defining a variable of that type. See the following example:

TypeScript Version: 2.7.0-dev.201xxxxx

Code

interface FooA {
    kind: 'A';
    settings: FooASettings;
}
interface FooB {
    kind: 'B';
    someB: string;
    settings: FooBSettings;
}
interface FooASettings {
    foo: string;
}
interface FooBSettings {
    bar: string;
}
type Foo = FooA | FooB;

const x: Foo = {
    kind: 'A',
//  someB: 'XX',            // WORKS Would error as expected as `FooA` does not contain `someB`
    settings: {             // FAILS Expected: `FooASettings`, Got: `FooASettings | FooBSettings`
        foo: 'foo',
        bar: 'bar',         // FAILS Expected: Failure
    }
}

if (x.kind === 'A') {
    x.settings;             // WORKS `FooASettings`, as expected
}

Expected behavior:

x.settings should be FooASettings in the type, and not deducted as FooASettings | FooBSettings. It works correctly when using the variable, but not when defining it.

Related:
I was unable to find any directly related issue for this case, the only thing I found was #12745 (comment) which might has the same root issue.

[mhegazy]

@sandersn any thoughts about what can be done here?

[sandersn]

@DHainzl Unfortunately, this is intentional. The excess property check only checks the top level when the target type is a union. That's because of examples like this:

interface CommonA {
  a: string
  b: number
  m1(): string
}
interface CommonB {
  a: string
  b: number
  m2(): number
}
var both: CommonA | CommonB = {
  a: 'hi'
  b: 1
  m1() { return this.a }
  m2() { return this.b }
}

To check whether the object literal is assignable to CommonA | CommonB, isRelatedTo first checks whether it is assignable to CommonA, then to CommonB if it's not. Unfortunately, this particular object literal is assignable to both, but m2 appears excess to CommonA and m1 appears excess to CommonB. So the current approach is just to turn off excess property checking after the initial, top-level check happens.

One cause of this is that union is not exclusive union. By analogy to binary operators, it's CommonA | CommonB, not CommonA ^ CommonB, even though when you have discriminants, it behaves like the latter. Perhaps we could keep the nested excess property checks when there is a discriminant.

[sandersn]

Later: No, this doesn't work for overlapping discriminants:

    type Ambiguous = {
        tag: "A",
        x: string
    } | {
        tag: "A",
        y: number
    } | {
        tag: "B",
        z: boolean
    } | {
        tag: "C"
    }
    let amb: Ambiguous
    // should have no error for ambiguous tag, even when it could satisfy both constituents at once
    amb = { tag: "A", x: "hi" }
    amb = { tag: "A", y: 12 }
    amb = { tag: "A", x: "hi", y: 12 }

[sandersn]

Never mind, I got the previous error because I still had a previous fix attempt in the build. I have a fix up at #21285.

[DHainzl]

Can confirm, the main issue is now fixed with typescript@next. Thanks for the quick help!

Just a side note: VSCode still shows the highlight for the settings property of my original snippet as FooASettings | FooBSettings, but then rightfully complains that bar does not exist on FooASettings:

const x: Foo = {
    kind: 'A',
//  someB: 'XX',            // WORKS Would error as expected as `FooA` does not contain `someB`
    settings: {             // FAILS Expected: `FooASettings`, Got: `FooASettings | FooBSettings`
        foo: 'foo',
        bar: 'bar',         // WORKS now! Object literal may only specify known properties, and 'bar' does not exist in type 'FooASettings'.
    }
}

Not sure if this is something that can be fixed or will be fixed as part of this ticket? Or would you prefer that I open a new ticket for this one?

[sandersn]

Opening a new ticket is better. I’m not as fluent with quick info, so I need something to remind me to go look at it. I think completions might disable excess property checks in the name of providing more completions, but that doesn’t make sense for quick info.