Omitted property does not narrow discriminated union contextual type

TypeScript Version: 4.2.1

Search Terms: discriminated union optional property

Code

type DiscriminatorTrue = {
  disc: true;
  cb: (x: string) => void;
}

type DiscriminatorFalse = {
  disc?: false;
  cb: (x: number) => void;
}

type Props = DiscriminatorTrue | DiscriminatorFalse;

declare function f(options: DiscriminatorTrue | DiscriminatorFalse): any;

f({
  disc: true,
  cb: s => parseInt(s) // Inference works 👍 
});

f({
  disc: false,
  cb: n => n.toFixed() // Inference works 👍 
});

f({
  cb: n => n.toFixed() // Implicit any error 👎 
});

f({
  cb: (n: string) => parseInt(n) // But errors correctly with incorrect type annotation 👍 
});

Expected behavior:
No implicit any error on the third call site

Actual behavior:
Implicit any error on the callback parameter at the third call site

Playground Link

Related Issues: #31404 (comment) is this issue, but the OP there is different.

Notes:
This pattern sometimes appears in React component props, where convention is to make boolean properties optional and only pass them as true (usually with the shorthand <MyComponent boolProp />). I actually suggested using discriminated union props for React components in an old blog post, and noted this issue in a footnote, calling it a possible bug, but didn’t file it at the time because I had low confidence it wasn’t a duplicate or design limitation. It was later mentioned in #31404 (comment), but was probably ignored because it was assumed to be an instance of the OP’s issue, which was determined to be a design limitation. I dove into this again because someone tweeted at me asking about that footnote after reading the post.

In this case, I think this'd be fixable by adjusting discriminateContextualTypeByObjectMembers and discriminateContextualTypeByJSXAttributes to include undefined'able discriminant props which don't appear in the node's properties. (They both currently filter the props which do exist on the input object down to the discriminant ones right now, so the possibly undefined discriminants from the contextual type would need to be appended to those)

Tagging backlog since this is a longstanding shortcoming, but we can take it earlier if someone's eager to take a stab.

This is same issue as this one right?

type Props = {
  mode?: boolean;
  nodes?: string;
}

type B =
  | { enabled?: false; }
  | { enabled: true; foo: string; };


// no error UNEXPECTED ❌
export const a: Props & B = {
  mode: true,
  nodes: ",",
  foo: "",
};


type B2 =
  | { enabled: false; }
  | { enabled: true; foo: string; };

// compiles as expected ✅
export const a2: Props & B2 = {
  mode: true,
  nodes: ",",
  enabled: true,
  foo: "",
};


// err as expected ✅
export const a3: Props & B2 = {
  mode: true,
  nodes: ",",
  foo: "",
};

// err as expected ✅
export const a4: Props & B2 = {
  mode: true,
  nodes: ",",
  enabled: false,
  foo: "",
};

Output

// no error UNEXPECTED ❌
export const a = {
    mode: true,
    nodes: ",",
    foo: "",
};
// compiles as expected ✅
export const a2 = {
    mode: true,
    nodes: ",",
    enabled: true,
    foo: "",
};
// err as expected ✅
export const a3 = {
    mode: true,
    nodes: ",",
    foo: "",
};
// err as expected ✅
export const a4 = {
    mode: true,
    nodes: ",",
    enabled: false,
    foo: "",
};

Compiler Options

{
  "compilerOptions": {
    "noImplicitAny": true,
    "strictNullChecks": true,
    "strictFunctionTypes": true,
    "strictPropertyInitialization": true,
    "strictBindCallApply": true,
    "noImplicitThis": true,
    "noImplicitReturns": true,
    "alwaysStrict": true,
    "esModuleInterop": true,
    "declaration": true,
    "experimentalDecorators": true,
    "emitDecoratorMetadata": true,
    "moduleResolution": 2,
    "target": "ES2017",
    "jsx": "React",
    "module": "ESNext"
  }
}

Playground Link: Provided

Sort of a "solution" is to do this:

// Solution is to specify all props from the non optional `enabled` as optional undefined fields
type B3 =
  | { enabled?: false; foo?: undefined}
  | { enabled: true; foo: string; };

// err as expected ✅
export const a5: Props & B3 = {
  mode: true,
  nodes: ",",
};

but it's not very nice. (I came across this when I was trying to use discriminated unions for react prop types)

update contextual discrimination to include omitted members (#43633)

Fix contextual discrimination for omitted members (#43937)

Andrew Branch (@andrewbranch)

Andrew Branch (@andrewbranch) Is this supposed to be still open? This doesn't error in 4.6, see playground

Huh, the fix was reverted, so I’m not sure what happened.

Andrew Branch (@andrewbranch)

👋 Hi, I'm the Repro bot. I can help narrow down and track compiler bugs across releases! This comment reflects the current state of the repro in the issue body running against the nightly TypeScript.

Issue body code block by Andrew Branch (@andrewbranch)

❌ Failed: -

Argument of type '{ cb: (n: string) => number; }' is not assignable to parameter of type 'DiscriminatorTrue | DiscriminatorFalse'. Type '{ cb: (n: string) => number; }' is not assignable to type 'DiscriminatorFalse'. Types of property 'cb' are incompatible. Type '(n: string) => number' is not assignable to type '(x: number) => void'. Types of parameters 'n' and 'x' are incompatible. Type 'number' is not assignable to type 'string'.

Historical Information

Version	Reproduction Outputs
4.3.2, 4.4.2, 4.5.2, 4.6.2	❌ Failed: - `Argument of type '{ cb: (n: string) => number; }' is not assignable to parameter of type 'DiscriminatorTrue \| DiscriminatorFalse'. Type '{ cb: (n: string) => number; }' is not assignable to type 'DiscriminatorFalse'. Types of property 'cb' are incompatible. Type '(n: string) => number' is not assignable to type '(x: number) => void'. Types of parameters 'n' and 'x' are incompatible. Type 'number' is not assignable to type 'string'.`
4.2.2	❌ Failed: - `Parameter 'n' implicitly has an 'any' type.` `Argument of type '{ cb: (n: string) => number; }' is not assignable to parameter of type 'DiscriminatorTrue \| DiscriminatorFalse'. Type '{ cb: (n: string) => number; }' is not assignable to type 'DiscriminatorFalse'. Types of property 'cb' are incompatible. Type '(n: string) => number' is not assignable to type '(x: number) => void'. Types of parameters 'n' and 'x' are incompatible. Type 'number' is not assignable to type 'string'.`

Hm, so it seems like #43633 originally fixed this but caused a crash and was reverted, and then #43937 promptly fixed it again and caused a performance regression that has not been reverted or fixed 😕

Anyone know the state of this? Is it worth a new issue? Looks to exist in 4.9.5

The OP's issue is still fixed, so if you have something that seems similar, definitely file a new bug.

Navigation Menu

Repository navigation

Omitted property does not narrow discriminated union contextual type #41759

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Omitted property does not narrow discriminated union contextual type #41759

Description

Activity

weswigham commented on Dec 1, 2020

RyanCavanaugh commented on Dec 1, 2020

safareli commented on Feb 5, 2021

safareli commented on Feb 5, 2021

Andarist commented on Apr 28, 2022

andrewbranch commented on Apr 28, 2022

typescript-bot commented on Apr 28, 2022

andrewbranch commented on Apr 28, 2022

luxaritas commented on Mar 4, 2023

weswigham commented on Mar 22, 2023

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions