Unions without discriminant properties do not perform excess property checking

[cazgp]

trafficstars

TypeScript Version: 2.7.0-dev.201xxxxx

Code

// An object to hold all the possible options
type AllOptions = {
    startDate: Date
    endDate: Date
    author: number
}

// Any combination of startDate, endDate can be used
type DateOptions =
    | Pick<AllOptions, 'startDate'>
    | Pick<AllOptions, 'endDate'>
    | Pick<AllOptions, 'startDate' | 'endDate'>

type AuthorOptions = Pick<AllOptions, 'author'>

type AllowedOptions = DateOptions | AuthorOptions

const options: AllowedOptions = {
    startDate: new Date(),
    author: 1
}

Expected behavior:

An error that options cannot be coerced into type AllowedOptions because startDate and author cannot appear in the same object.

Actual behavior:

It compiles fine.

[DanielRosenwasser]

Technically speaking, the expression is a subtype of almost all the types of AllowedOptions, but we usually perform excess property checking. However, that excess property checking isn't occurring here.

I thought this was related to #12745 but it seems to still be present.

[DanielRosenwasser]

Though according to responses to https://github.com/Microsoft/TypeScript/issues/12745#issuecomment-344759609, this might be working as intended.

[sylvanaar]

Isn't this due to contextual typing merging all the fields. Why was this behavior chosen?

When used as a contextual type (section 4.23), a union type has those members that are present in any of its constituent types, with types that are unions of the respective members in the constituent types. Specifically, a union type used as a contextual type has the apparent members defined in section 3.11.1, except that a particular member need only be present in one or more constituent types instead of all constituent types.

[sandersn]

Contextual typing is not relevant here. Neither is Pick. Here's a smaller repro that avoids contextual typing:

type A = { d: Date, e: Date } | { n: number }
const o = { d: new Date(), n: 1 }
const a: A = o

Excess property checking of unions does not work because all it does is try to find a single member of the union to check for excess properties. It does this based on discriminant properties, but A doesn't have any overlapping properties, so it can't possibly have any discriminant properties. (The same is true of the original AllowedOptions.) So excess property checking is just skipped. I chose this design because it was conservative enough to avoid bogus errors, if I remember correctly.

To get excess property checks for unions without discriminant properties, we could just choose the first union member that has at least one property shared with the source, but that would behave badly with the original example:

const options: AllowedOptions = {
  startDate: new Date(),
  endDate: new Date()
}

Here, we'd choose Pick<AllOptions, 'startDate'> as the first matching union member and then mark endDate as an excess property.

@sylvanaar @DanielRosenwasser Let me know if you have ideas for some other way to get excess property checks for unions without discriminant properties.

[pelotom]

Is this the same issue as #22129? My repro:

export interface A {
  x?: string;
}
export interface B extends A {
  y?: number;
}
export interface C extends A {
  z?: boolean;
}

const passes: B | C = { x: 'hello', z: 42 };

const fails: B | C = { z: 42 };

[jack-williams]

@sandersn

Let me know if you have ideas for some other way to get excess property checks for unions without discriminant properties.

Is it possible to sort the union type first by sub-typing, then choose the first matching member?

[sandersn]

@pelotom No, excess property checking only considers property names, not types.

@jack-williams That would probably work. Here are some problems that might arise:

1. It would be expensive. Admittedly, the current check is expensive, but pays off (at least for the compiler code base) because it discards many members of large unions early on in assignability checking.
2. Would a sort on subtyping be stable? I think subtyping is only a partial order. The first member of union might be surprising.
3. The rule is pretty complex, so we wouldn't want people to have to think about it. That means it should work every time, and every time the same way.

(1) is pretty easy to check given a prototype. Figuring out (2) is a precondition for (3). Working through some examples would help here, to show that the results are not surprising to humans.

[jack-williams]

@sandersn

Yeah I believe the ordering is only partial, so even if the sort were stable, you'd still have to deal with unrelated elements appear somewhere. Bringing in subtyping seems quite a heavyweight approach given that excess property checking only looks at property names--I don't think that my suggestion was that good, on reflection.

I'm wondering whether a solution based on set inclusion of property names would be easier to predict and implement. For instance, use bloom filters to prune union elements that definitely do not contain properties in the initialising object, then use more expensive checks to resolve the remaining cases that might contain all the specified properties. That might involve straight enumeration, or shortcuts like: if a type in the union has no optional properties, then it must have the same bloom filter as the initialiser.

[ghost]

Another example:

declare function f(options: number | { a: { b: number } }): void;
f({ a: { b: 0, c: 1 } });

[mhegazy]

@sandersn the example in https://github.com/Microsoft/TypeScript/issues/20863#issuecomment-367890323 is excess property checking. When comparing { x: 'hello', z: 42 } to B | C, { x: 'hello', z: 42 } is not assignable to C since z is a number and not a boolean. { x: 'hello', z: 42 } should not be assign bale to B since { x: 'hello', z: 42 } has an unknown property z. the later is what is not happening now.

[jack-williams]

@andy-ms I believe your example is essentially the same problem as #13813 (but with union instead). I don't know how to fix it for the general case, but perhaps a special case could be made for unions with a single object type?

[danvk]

I ran into this in TypeScript 3.3 and was quite surprised by the lack of an error in this snippet:

interface Simple {
  a: string;
}

interface Detailed extends Simple {
  b: string;
  c: number;
}

type T = Simple | Detailed;

const mixed: T = {
  a: '',
  b: '',
};  // no error, assignment is OK

function f(x: T) {
  if ('b' in x) {
    x  // type is refined to Detailed
  }
}

As a user, it feels like there's a mismatch between:

• An assignment with a property does not refine the type
• Testing for the presence of a property does refine a type

Neither case is fully justified since mixed is compatible with the union type. But if that's the official line, then it doesn't seem like the type should be refined in the function, either (since it would be incorrect if you call f(mixed)).

[svatal]

Same problem for me with TS 3.4:

type IItem = {a: string} & ({b?: false} | { b: true, requiredWhenB: string })

function x(i: IItem) { }

x({ a: 'a' })   // ok
x({ a: 'a', b: false }) // ok
x({ a: 'a', unknownProp: 1 })   // ok, failed because of unknown property
x({ a: 'a', b: false, requiredWhenB: "x"})  // ok, failed because of unknown property
x({ a: 'a', requiredWhenB: "x"})    // did not failed, but should be same case as previous line
x({ a: 'a', requiredWhenB: true})    // did not failed, even with the wrong type
x({ a: 'a', b: undefined, requiredWhenB: "x"})    // ok, fails with strictNullCheck, does not fail without it - it is intended that true === (true | undefined) in that case?
x({ a: 'a', b: true })  // ok, failed because of missing required property
x({ a: 'a', b: true, requiredWhenB: "x" })  // ok

Strange thing is that if I get the common type {a: string} out of equation, everything starts to work as expected ...

[dragomirtitian]

@svatal I have a workaround for this in the form of StrictUnion. I also have a decent writeup of it here:

type UnionKeys<T> = T extends unknown ? keyof T : never;
type StrictUnionHelper<T, TAll> = T extends unknown ? T & Partial<Record<Exclude<UnionKeys<TAll>, keyof T>, never>> : never;
type StrictUnion<T> = StrictUnionHelper<T, T>

type IItem = StrictUnion<{a: string} & ({b?: false} | { b: true, requiredWhenB: string })>

function x(i: IItem) { }

x({ a: 'a' })   // ok
x({ a: 'a', b: false }) // ok
x({ a: 'a', unknownProp: 1 })   // ok, failed because of unknown property
x({ a: 'a', b: false, requiredWhenB: "x"})  // ok, failed because of unknown property
x({ a: 'a', requiredWhenB: "x"})    // fails now
x({ a: 'a', requiredWhenB: true})    // fails now
x({ a: 'a', b: undefined, requiredWhenB: "x"})    // this one is still ok if strictNullChecks are off.
x({ a: 'a', b: true })  // ok, failed because of missing required property
x({ a: 'a', b: true, requiredWhenB: "x" })  // ok

[Sowed]

@dragomirtitian, your workaround and article were so helpful. Thank you. I went ahead to use that StrictUnion type as a global declaration for a typescript project am working on. One of my team mates warned me about how type guards are a headache for ts compilation. Is there any performance hits I should look into when using type guards?

[dragomirtitian]

@Sowed As far as I know type guards don't cause performance problems in the compiler (at least when used in normal scenarios). Distributive conditional types tend to cause performance problems when dealing with large unions, so I would not use StrictUnion over something like the values of JSX.IntrinsicElements but generally I expect it to perform decently.

Note we are talking about compilation performance not runtime performance. For runtime performance adding a custom type guard is indeed an extra function call where one might not have existed, but worrying about this too much seems like premature optimization to me. In most cases it will not matter

[EduardoRFS]

Someone can confirm for me that this is a bug, it seems like a excess property checking bug, making it validate on the first property to match one seems like the logical thing to happen? If it is I can make a PR @sandersn @DanielRosenwasser ?

[AnyhowStep]

I think I just ran into this issue with this snippet,

type Foo = {
    a : number,
    b : number,
};
type Bar = {
    a : number,
    c : number,
};

const foo : Foo = {
    a : 0,
    b : 0,
    //OK!
    //Object literal may only specify known properties
    c : 0,
};

const bar : Bar = {
    a : 0,
    //OK!
    //Object literal may only specify known properties
    b : 0,
    c : 0,
};

/**
 * Expected: Object literal may only specify known properties
 * Actual  : No errors
 */
const fooOrBar : Foo|Bar = {
    a : 0,
    b : 0,
    c : 0,
};

Playground

I know TS only performs excess property checks on object literals. But I also need to perform excess property checks when the destination type is a union.

Seems like it doesn't do that at the moment.

@dragomirtitian 's workaround works but it does make the resulting type much uglier.

[millsp]

In addition to @dragomirtitian's StrictUnion, I would just add Compute to make it human-readable:

import {A} from 'ts-toolbelt'

type UnionKeys<T> = 
  T extends unknown
  ? keyof T
  : never;

type StrictUnionHelper<T, TAll> =
  T extends unknown
  ? T & Partial<Record<Exclude<UnionKeys<TAll>, keyof T>, never>>
  : never;

type StrictUnion<T> = A.Compute<StrictUnionHelper<T, T>>

type t = StrictUnion<{a: string} | {b: number} | {c: any}>

[AnyhowStep]

Just thought I'd bring up another approach to "human-readability",

type UnionKeys<T> = 
  T extends unknown ?
  keyof T :
  never;
  
type InvalidKeys<K extends string|number|symbol> = { [P in K]? : never };
type StrictUnionHelper<T, TAll> =
  T extends unknown ?
  (
    & T
    & InvalidKeys<Exclude<UnionKeys<TAll>, keyof T>>
  ) :
  never;

type StrictUnion<T> = StrictUnionHelper<T, T>

/*
type t = (
    {a: string;} & InvalidKeys<"b" | "c">) |
    ({b: number;} & InvalidKeys<"a" | "c">) |
    ({c: any;} & InvalidKeys<"a" | "b">)
*/
type t = StrictUnion<{a: string} | {b: number} | {c: any}>

/*
type noisyUnion = ({
    a: string;
    b: string;
    c: number;
} & InvalidKeys<"x" | "y" | "z" | "i" | "j" | "k" | "l" | "m" | "n">) | ({
    x: string;
    y: number;
    z: boolean;
} & InvalidKeys<"a" | "b" | "c" | "i" | "j" | "k" | "l" | "m" | "n">) | ({
    ...;
} & InvalidKeys<...>) | ({
    ...;
} & InvalidKeys<...>)
*/
type noisyUnion = StrictUnion<
    | {a:string,b:string,c:number}
    | {x:string,y:number,z:boolean}
    | {i:Date,j:any,k:unknown}
    | {l:1,m:"hello",n:1337}
>;

Playground

If InvalidKeys is too verbose, you can make the name as short as an underscore, Playground

---

With Compute,

export type Compute<A extends any> =
    {[K in keyof A]: A[K]} extends infer X
    ? X
    : never

type UnionKeys<T> = 
  T extends unknown
  ? keyof T
  : never;

type StrictUnionHelper<T, TAll> =
  T extends unknown
  ? T & Partial<Record<Exclude<UnionKeys<TAll>, keyof T>, never>>
  : never;

type StrictUnion<T> = Compute<StrictUnionHelper<T, T>>

/*
type t = {
    a: string;
    b?: undefined;
    c?: undefined;
} | {
    b: number;
    a?: undefined;
    c?: undefined;
} | {
    c: any;
    a?: undefined;
    b?: undefined;
}
*/
type t = StrictUnion<{a: string} | {b: number} | {c: any}>

/*
type noisyUnion = {
    a: string;
    b: string;
    c: number;
    x?: undefined;
    y?: undefined;
    z?: undefined;
    i?: undefined;
    j?: undefined;
    k?: undefined;
    l?: undefined;
    m?: undefined;
    n?: undefined;
} | {
    x: string;
    y: number;
    z: boolean;
    a?: undefined;
    ... 7 more ...;
    n?: undefined;
} | {
    ...;
} | {
    ...;
}
*/
type noisyUnion = StrictUnion<
    | {a:string,b:string,c:number}
    | {x:string,y:number,z:boolean}
    | {i:Date,j:any,k:unknown}
    | {l:1,m:"hello",n:1337}
>;

Playground

---

I like the Compute approach when the union is not "noisy" (the ratio of invalid to valid keys is closer to 0 or 0.5).

I like the InvalidKeys approach when the union is "noisy" (the ratio of invalid to valid keys is closer to one or higher).

[dragomirtitian]

@AnyhowStep @pirix-gh I always use this type to expand out other types, not necessarily better just another option:

export type Compute<A extends any> = {} & { // I usually call it Id
  [P in keyof A]: A[P]
}

Also I tend to use it sparingly, if the union constituents are named, the expanded version is arguably worse IMO.

[AnyhowStep]

I've found that trying to expand/compute a type can make TS forget that the type is assignable to another type. That problem seems to occur more often with more complex generic types.

So, there's a chance that T is assignable to U but Compute<T> is not assignable to U. The solution is to just... Use Compute<> sparingly.

In a perfect world, I'd use Compute<> as often as possible, if they're not named.

[millsp]

@AnyhowStep, maybe you can use Cast to do that, it kind of refreshes TS. And I would avoid using compute too much as it can affect performance (when combined & re-combined). It would be nice that ts does this by default though.

[SlurpTheo]

@dragomirtitian https://github.com/microsoft/TypeScript/issues/20863#issuecomment-520553541

This doesn't work for me -- as I want errors on aD1 & aD3 as well:

export type Compute2<A extends any> = {} & {
	// I usually call it Id
	[P in keyof A]: A[P];
};

type D = { d: Date; e: Date } | { n: number };
const aD1: Compute2<D> = { d: new Date(), n: 1 };
const aD2: Compute2<D> = { d: new Date() }; // ts(2322)
const aD3: Compute2<D> = { d: new Date(), e: new Date(), n: 1 };
const aD4: Compute2<D> = { d: new Date(), e: new Date() };

---

Now, @AnyhowStep's https://github.com/microsoft/TypeScript/issues/20863#issuecomment-520551758 seems to work to the level of enforcement I'm expecting (error on aC2 and not on aE2):

type Compute<A extends any> = { [K in keyof A]: A[K] } extends infer X ? X : never;
type UnionKeys<T> = T extends unknown ? keyof T : never;
type StrictUnionHelper<T, TAll> = T extends unknown
	? T & Partial<Record<Exclude<UnionKeys<TAll>, keyof T>, never>>
	: never;
type StrictUnion<T> = Compute<StrictUnionHelper<T, T>>;

type C = { d: Date; e: Date } | { n: number };
const aC1: StrictUnion<C> = { d: new Date(), n: 1 }; // ts(2322)
const aC2: StrictUnion<C> = { d: new Date() }; // ts(2322)
const aC3: StrictUnion<C> = { d: new Date(), e: new Date(), n: 1 }; // ts(2322)
const aC4: StrictUnion<C> = { d: new Date(), e: new Date() };

type E = { d: Date; e?: Date } | { n: number };
const aE1: StrictUnion<E> = { d: new Date(), n: 1 }; // ts(2322)
const aE2: StrictUnion<E> = { d: new Date() };
const aE3: StrictUnion<E> = { d: new Date(), e: new Date(), n: 1 }; // ts(2322)
const aE4: StrictUnion<E> = { d: new Date(), e: new Date() };

Although I don't totally follow the StrictUnion --> Compute happenings there.

---

I was going to slightly tweak the example of @sandersn https://github.com/microsoft/TypeScript/issues/20863#issuecomment-354894849

type A = { d: Date, e: Date } | { n: number }
const o = { d: new Date(), n: 1 }
const a: A = o

To add the n?: never property:

type B = { d: Date; e: Date; n?: never } | { n: number };
const aB1: B = { d: new Date(), n: 1 }; // ts(2322)
const aB2: B = { d: new Date() }; // ts(2322)
const aB3: B = { d: new Date(), e: new Date(), n: 1 }; // ts(2322)
const aB4: B = { d: new Date(), e: new Date() };

type F = { d: Date; e?: Date; n?: never } | { n: number };
const aF1: F = { d: new Date(), n: 1 }; // ts(2322)
const aF2: F = { d: new Date() };
const aF3: F = { d: new Date(), e: new Date(), n: 1 }; // ts(2322)
const aF4: F = { d: new Date(), e: new Date() };

But that's a lot more tedious for me to do in my real code-base across all the interfaces that make up the strict union rather than just calling StrictUnion once at the end.

[SlurpTheo]

Although, I can't get StrictUnion to work right on this kind of union (aG1 shouldn't have an error):

type G = { d: Date } | { (): boolean };
const aG1: StrictUnion<G> = () => true; // ts(2322)
const aG2: StrictUnion<G> = { d: new Date() };

[millsp]

@SlurpTheo

import {U} from 'ts-toolbelt'

type SU = U.Strict<{d: Date} | {(): boolean}>

const aG1: SU = () => true; 
const aG2: SU = { d: new Date() };

This happens because we should never compute a function type by intersecting it with an object, Compute from the ts-toolbelt handles this scenario.

[RyanCavanaugh]

On review of this, it seems like the scenario in the OP isn't something we could feasibly touch at this point - it's been the behavior of EPC for unions since either of those features existed, and people write quasioverlapping unions fairly often.

The solutions described above are all pretty good and honestly much more expressive than just the originally-proposed behavior change, so I'd recommend choosing one of those based on what you want to happen.

Overall Exact/Closed types seem like the real underlying request for most of these scenarios, so follow #12936 for next steps there.

[SlurpTheo]

Overall Exact/Closed types seem like the real underlying request for most of these scenarios, so follow #12936 for next steps there.

Wowsa, #12936 is quite the chain there 😺 After skimming I don't immediately follow how it will help with providing something like a Strict Union (i.e., really don't want to allow excess properties), but I have subscribed.

[essenmitsosse]

I think I have found a simpler example, that is also not working correctly. Looks similar to problem @AnyhowStep found.

interface A { foo: any, fuz: any }
interface B { bar: any }

// This correctly gives a missing property 'fuz' error
const nonUnion: (A & B) = {
    bar: 1,
    foo: 1,
}

// This doesn't give a missing property error
const union: (A & B) | B = {
    bar: 1,
    foo: 1,
}

The existence of foo should necessitate the existence of fuz. Especially since this is working if the type is not a union.

<StrictUnion> solves this issue for the above cases

[mrvisser]

Built-in type guards pretend that TS has excess property checking on unions, when really it doesn't.

type Step =
  | { name: string }
  | {
      name: string;
      start: Date;
      data: Record<string, unknown>;
    };

const step: Step = {
  name: 'initializing',
  start: new Date(),
};

if ('start' in step) {
  console.log(step.data.thing);
}

EDIT: I was fooled by comment-compression into thinking the type-guard case was not mentioned. Apologies for the repetition.