Omitted property does not narrow discriminated union contextual type

[andrewbranch]

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.

[weswigham]

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)

[RyanCavanaugh]

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

[safareli]

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

[safareli]

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)

[Andarist]

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

[andrewbranch]

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

[typescript-bot]

👋 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 @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'.

[andrewbranch]

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 😕

[luxaritas]

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

[weswigham]

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