Typescript: Union type passes check when each of its members fail.

[sbrow]

TypeScript Version: 3.4.0-dev.201xxxxx

Search Terms:
union type
bug

Code

class Package {
    public name: string;
    public isDev?: boolean;
}

interface Devable extends Package {
    devOnly: false;
}

interface DevOnlyPackage extends Package {
    isDev?: true;
    devOnly: true;
}

// This should throw an error.
const d: Package | Devable | DevOnlyPackage = {
    name: "yarn",
    isDev: false,
    devOnly: true,
};

Expected behavior:
The annotation on const d should throw an error. It can't be a Package because it has additional fields, it can't be Devable because devOnly is false, and it can't be DevOnlyPackage because isDev is false.

Actual behavior:
No error is thrown.

Playground Link: playground

If you try to assert each type individually, it will throw an error as expected, but when a union type is used, it passes the check.

Related Issues:

[fatcerberus]

It can't be a Package because it has additional fields

Just for the record (because a lot of people don’t realize this), that only applies to direct assignment from object literals, and not to assignment between object types in general. There is an object literal here, so you’re right to expect the error in this case.

[dragomirtitian]

I'm not really surprised that assignment passes. Because of these fields isDev: false, devOnly: true the object literal can't be either Devable or DevOnlyPackage. But it can be Package with an extra property (devOnly). The way excess property checks has always worked for unions is that a property is allowed if present on ANY of the union constituents (a design decision I have often seen causes confusion). So if the object literal is compatible with Package and excess property checks are silenced because devOnly is present in one of the union constituent, there is no reason for an error to appear.

The workaround is to either manually add devOnly?: undefined; to Package or use something like StrictUnion see here

[fatcerberus]

Thanks for the insight @dragomirtitian, I wasn’t aware myself that that’s how the excess property check worked - that’s indeed unintuitive.

[sbrow]

@dragomirtitian Thanks, that solves the problem. However, I have to add a forth interface and use that in the type union:

interface StrictPackage extends Package {
  devOnly: undefined;
}

Otherwise Devable and DevOnly can't extend Package.

Maybe this isn't the right way to approach my problem. I'm trying to ensure with these interfaces that isDev can't be false if devOnly is true.

Something like:

interface Package {
  name: string;
  devOnly?: boolean;
  isDev?: (this.devOnly === true) ? true : boolean;
}

Is there a way to write that without using 4 interfaces?

[fatcerberus]

isDev?: (this.devOnly === true) ? true : boolean;

That’s dependent typing, which TS can’t do.

[sbrow]

@fatcerberus I know that TS can't do that, which is why I did what I did. The StrictUnion method does fix the code without any other alterations, so I guess I'll move forward with that.

Since this issue isn't considered to be a bug, I'll close it, but I would like to say that I think the way that excess property checks work for unions is unintuitive. If const a: Foo = x fails and const a: Bar = x fails, the fact that const a: Foo | Bar = x passes is puzzling, and not immediately obvious.

[sbrow]

Actually, I just realized that StrictUnion doesn't solve the problem, because it prevents any extension of the Devable and DevOnly interfaces, effectively turning them into classes.

[fatcerberus]

If I had to take a stab at why the current behavior was chosen: A | B | C when considered as a whole type, is also inhabited by A & B & C, so this lets you use an object literal matching that intersection. That’s just an educated guess though, so don’t take that as gospel :stuck_out_tongue:

[jack-williams]

This is considered to be a bug: tracked here #20863.

[dragomirtitian]

@jack-williams Forgot about that bug. Since that bug has been around for almost 2 years I wonder if fixing it will be a big breaking change at this point :(