No excess property checks when dealing with tagged unions
DanielRosenwasser opened this issue · 12 comments
Version: 2.1.4
interface None {
hasValue: false;
}
interface Some {
hasValue: true;
value: any;
}
var x: None | Some = {
hasValue: false,
value: [],
}Expected: Excess property error on value in the object literal.
Actual: No error.
We will need a proposal for this, since this has to take into consideration the discriminant property in the union and filter on this.
Just ran into this.
interface A {
type: "A";
data: { b: boolean };
}
interface B {
type: "B";
data: { n: number };
};
const x: A | B = {
type: "B",
data: { n: 1, excess: 2 }
};Is the fact that the following is not checked for excess properties also related to this? In other words, should I open another issue on how the behavior is present regardless of the union being tagged?
type Props<T> = PropsBase | PropsWithConvert;
interface PropsBase {
data: string;
}
interface PropsWithConvert extends PropsBase {
convert: (t: number) => string;
}
function doIt<T>(props: Props<T>) {
}
// 'convert' here returns a 'number', so it can't be assignable to 'PropsWithConvert',
// But the check still appears to succeed because the object is assignable to 'PropsBase'.
doIt({
data: "",
convert(x) {
return 10;
}
})I'm not sure if this is completely fixed. I have code that looks like this:
export type Condition = And | Or | Not | {
[type: string]: {
target: string
[key: string]: any
}
}
export interface And {
and: Condition[]
}
export interface Or {
or: Condition[]
}
export interface Not {
not: Condition
}
const c: Condition = {
and: [],
not: { // <-- should be producing an error
equals: {
target: "name",
value: "foo"
},
},
}And there are no errors. Also if I do this there are no errors as well:
const c: Condition = {
foo: "bar", // <--- should be error here. does not exist in any type
not: {
equals: {
target: "name",
value: "foo"
},
},
}@lukescott that's the intended behavior - a property isn't excess if it appears in any of the target types
Ah - I missed hasValue was defined on both.
@RyanCavanaugh ok I get how and and or are allowed on the same object with |. I misunderstood what the fix was about. However, why is foo: "bar" not emitting an error? As far as I know that shouldn't be matching up against:
type Misc = {
[type: string]: {
target: string
[key: string]: any
}
}Your foo: "bar" example looks like a bug, but not one with excess property checks; it's not doing type checking, just properties. With respect to excess properties, foo is not excess because Misc has a string index signature.
However, you should see a type error because "bar" is missing the property target.
oh, never mind. You're falling between two checks.
- The excess property check is satisfied by
Misc, which has a string indexer. No property will be marked as excess. - Structural assignability is satisfied by
Not, since the object literal has the propertynot, but also an extra (excess!) propertyfoo. But this is fine according to the rules of structural assignability.
The basic problem is that excess property checks happen before structural assignability finds the "best match" of the union; it uses a faster, heuristic check that doesn't perfectly match the exhaustive search used by structural assignability.
@lukescott I opened a bug #20060 to track your repro. I can't promise we'll do anything about it because we rely on the early-fail property of excess property checking for some speed in isRelatedTo.