Skip to content

Inference not working for union without discriminant properties #34590

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
OliverJAsh opened this issue Oct 19, 2019 · 9 comments
Closed

Inference not working for union without discriminant properties #34590

OliverJAsh opened this issue Oct 19, 2019 · 9 comments
Labels
Working as Intended The behavior described is the intended behavior; this is not a bug

Comments

@OliverJAsh
Copy link
Contributor

OliverJAsh commented Oct 19, 2019

TypeScript Version: 3.6.3

Search Terms:

Code

type AElement = HTMLAnchorElement;
type BElement = HTMLButtonElement;

interface A {
    a: 'a';
    onClick(el: AElement): void;
}

interface B {
    onClick(el: BElement): void;
}

type Props = A | B;
const props: Props = {
    a: 'a',
    // unexpected error, `el` is implicitly `any`
    onClick: el => {}
};

If we add a common discriminant property, it does work:

type AElement = HTMLAnchorElement;
type BElement = HTMLButtonElement;

interface A {
    type: 'a';
    onClick(el: AElement): void;
}

interface B {
    type: 'b';
    onClick(el: BElement): void;
}

type Props = A | B;
const props: Props = {
    type: 'a',
    onClick: el => {},
};

It will also work if the union members do not share the same property:

type AElement = HTMLAnchorElement;

interface A {
    a: 'a';
    onClick(el: AElement): void;
}

interface B {
    foo: number;
}

type Props = A | B;
const props: Props = {
    a: 'a',
    onClick: el => {},
};

Expected behavior:

Actual behavior:

Playground Link:

Related Issues:

@j-oliveras
Copy link
Contributor

As a workaround of your first code (add a optional property a with type undefined):

type AElement = HTMLAnchorElement;
type BElement = HTMLButtonElement;

interface A {
    a: 'a';
    onClick(el: AElement): void;
}

interface B {
    a?: undefined;
    onClick(el: BElement): void;
}

type Props = A | B;
const props: Props = {
    a: 'a',
    onClick: el => {}
};

Playgorund (tested with 3.7-beta, 3.6.3 and nightly).

@RyanCavanaugh
Copy link
Member

I don't understand the intended use of this code. When some code that receives a Props goes to call onClick, how does it determine what to do? What about aliasing? How do you get into this state in a way where callers are known to do the right thing? It seems like a correct error to say that TS can't really predict whether or not onClick will be invoked with an AElement or BElement since there's no runtime way to safely distinguish these objects.

@OliverJAsh
Copy link
Contributor Author

When some code that receives a Props goes to call onClick, how does it determine what to do?
Ideally I could do this:

declare const aElement: AElement;
declare const bElement: BElement;
props.a === 'a' ? props.onClick(aElement) : props.onClick(bElement);

Although as I just discovered, this won't work.

since there's no runtime way to safely distinguish these objects.

Is that because a: 'a' could exist on B? In that case, I guess what I'm really after is exact types?

@ferdaber
Copy link

ferdaber commented Oct 24, 2019

In the context of usage, this is used for polymorphic components in React (where the most popular use case is rendering buttons vs anchors that visually look the same).

If there is a href prop given to this polymorphic component, then the component will render an anchor tag, otherwise it will render a button tag, so the discriminator is the presence of a property in the given props.

// ignore the fact that HTML allows `href` to be optional for anchors (if we pretend)

type ButtonProps = JSX.IntrinsicElements['button']
type AnchorProps = JSX.IntrinsicElements['a'] & { href: string }

declare function PolymorphicButton(props:  ButtonProps | AnchorProps): JSX.Element

// I want `e` to be `MouseEvent<HTMLButtonElement>`
const myButton = <PolymorphicButton onClick={e => e.target} /> 

// I want `e` to be `MouseEvent<HTMLAnchorElement>`
const myAnchor = <PolymorphicButton onClick={e => e.target} href='https://typescriptlang.org' /> 

@karol-majewski
Copy link

karol-majewski commented Oct 24, 2019

We could use solid discriminants or a?: undefined/a?: never for B as @j-oliveras suggested, so at least we have a way out @OliverJAsh 👍

Just like in: https://twitter.com/WrocTypeScript/status/1122952350178643968

@RyanCavanaugh
Copy link
Member

The problem with a nonpresent discriminant is that it's subject to aliasing effects:

type AElement = HTMLAnchorElement;
type BElement = HTMLButtonElement;

interface A {
    a: 'a';
    onClick(el: AElement): void;
}

interface B {
    onClick(el: BElement): void;
}

type Props = A | B;

function fail(x: A | B) {
  if (x.a === "a") {
    x.onClick(someAnchor);
  }
}

// Clearly OK, by definition
let tricky1 = { a: "a", onClick: (el: BElement) => e.buttonThing() };
// Clearly OK - 'a' property is aliased away, 'onClick' property matches
let tricky2: B = tricky1;
// Clearly OK by the definition of union types
let tricky3: A | B = tricky2;
// Crashes
fail(tricky3);

This is why you need an undefined or never placeholder property to make the assignment into tricky2 illegal

@ferdaber
Copy link

For use cases of polymorphic components, overload signatures will work fine but they will break in cases where a conditional type attempts to infer its props, so I was hoping this would be a solution but your reasoning makes sense to me.

@SlurpTheo
Copy link

// Clearly OK by the definition of union types
let tricky3: A | B = tricky2;

Blocking this (w/o having to manually sprinkle never props) is another place that could benefit from ts-toolbelt's Union.Strict<T>:

import { Union } from "ts-toolbelt";
const tricky4 = { a: "a", onClick: (el: BElement) => el.checkValidity() };
const tricky5: Union.Strict<A | B> = tricky4; // ts(2322)

image

@RyanCavanaugh RyanCavanaugh added the Working as Intended The behavior described is the intended behavior; this is not a bug label Oct 30, 2019
@typescript-bot
Copy link
Collaborator

This issue has been marked 'Working as Intended' and has seen no recent activity. It has been automatically closed for house-keeping purposes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Working as Intended The behavior described is the intended behavior; this is not a bug
Projects
None yet
Development

No branches or pull requests

7 participants