Skip to content
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

true| false can not be used to create discriminated unions #17882

Closed
sebald opened this issue Aug 18, 2017 · 4 comments
Closed

true| false can not be used to create discriminated unions #17882

sebald opened this issue Aug 18, 2017 · 4 comments
Labels
Working as Intended The behavior described is the intended behavior; this is not a bug

Comments

@sebald
Copy link

sebald commented Aug 18, 2017

TypeScript Version: 2.4.2

Code

Link to the playground

type One = {
  kind: true;
  foo: string;
} 

type Two = {
  kind: false;
  foo: boolean;
}

type Options = One | Two;

const fn = (options: Options) => { /* ... */ };

const optionsOne = { kind: true, foo: 'bar' };

/**
 * ERROR:
 * Types of property 'kind' are incompatible.
 * Type 'boolean' is not assignable to type 'false'.
 */
fn(optionsOne);

/**
 * No Error.
 */
fn({ kind: true, foo: 'bar' })

Expected behavior: Should be possible to use true | false to create a discriminated union. It seems like true | false and boolean are not interchangeable.

Actual behavior: TypeScript does not widen the above kind to boolean and thus optionsOne can not be used as input for fn. (That's just my wild guess :-x) It's not possible to use true and false for discriminated unions.

@ahejlsberg
Copy link
Member

This is working as intended. In the declaration of optionsOne you need some indication that you don't want the kind property widened. Absent that, the compiler widens the type because a property is a mutable location. You can add a type annotation to indicate your intent:

const optionsOne: One = { kind: true, foo: 'bar' };

Once you have a type annotation the object literal is contextually typed by that annotation and the compiler knows not to widen. For the same reason, there is no error in the call where you directly specify an object literal because the literal is contextually typed by the corresponding parameter type (Options) in the function.

@ahejlsberg ahejlsberg added the Working as Intended The behavior described is the intended behavior; this is not a bug label Aug 18, 2017
@sebald
Copy link
Author

sebald commented Aug 18, 2017

Thanks for the quick and throughout explanation!

My actual use case for this was that I created typings for a library and wanted to have a strongly typed API. This is the original code that caused this issue:

// index.d.ts
declare module 'material-ui/List/ListItem' {
  import { ButtonBaseProps } from 'material-ui/internal/ButtonBase';

  interface ListItemCommonProps extends React.LiHTMLAttributes<HTMLLIElement> {
    component?: React.ReactNode;
    dense?: boolean;
    disabled?: boolean;
    disableGutters?: boolean;
    divider?: boolean;
  }

  export type ListItemDefaultProps = {
    button?: false;
  } & ListItemCommonProps;

  export type ListItemButtonProps = {
    button?: true;
  } & ListItemCommonProps &
    ButtonBaseProps;

  export type ListItemProps = ListItemDefaultProps | ListItemButtonProps;

  export default class ListItem extends MaterialUI.Component<ListItemProps> {}
}

And even casting didn't help here:

class DockedDrawer extends React.Component<{}, { docked: boolean }> {
  state = { docked: true }

  render() {
    const docked:true|false = this.state.docked;
    return (
      <Drawer
        anchor="bottom"
        open={true}
        docked={docked}
      >
        List
      </Drawer>
    );
  }

I thought I can use discriminated unions to only allow certain props on the component, depending on other props.

sebald pushed a commit to mui/material-ui that referenced this issue Aug 18, 2017
For now we don't use them, maybe Reason -> microsoft/TypeScript#17882 helps to find a way to make use of them.
sebald added a commit to mui/material-ui that referenced this issue Aug 18, 2017
- Update `<Tabs>` API to match BC introduced in #7741
- Update `withStyle` API to match BC introduced in #7740
- Remove discriminated unions (microsoft/TypeScript#17882)
@mhegazy
Copy link
Contributor

mhegazy commented Sep 5, 2017

Automatically closing this issue for housekeeping purposes. The issue labels indicate that it is unactionable at the moment or has already been addressed.

@michaeljota
Copy link

michaeljota commented Jan 2, 2018

Hi @sebald maybe if you remove the optional operator from the type, then it would work as you want. You know that the parameter could be either true or false, however I think this parameter will always exists, even if it doesn't it would simple will be evaluated to a falsy value.

Update: It works as you expect, and narrow down the type of the props. :).

  export type ListItemDefaultProps = {
    button: false;
  } & ListItemCommonProps;

  export type ListItemButtonProps = {
    button: true;
  } & ListItemCommonProps;

@microsoft microsoft locked and limited conversation to collaborators Jul 3, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
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

4 participants