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

Discriminated union parameter destructuring doesn't work if the fields have defaults #50139

Open
KrischnaGabriel opened this issue Aug 2, 2022 · 11 comments
Labels
Bug A bug in TypeScript Help Wanted You can do this
Milestone

Comments

@KrischnaGabriel
Copy link

KrischnaGabriel commented Aug 2, 2022

Suggestion

Cleverly preserve the conditional type of variables of an de-structured object, which may be of 2 or more object types.

🔍 Search Terms

in:title destructuring assignment condition*
in:title destructuring assignment object
in:title destructuring assignment or

✅ Viability Checklist

My suggestion meets these guidelines:

  • [true] This wouldn't be a breaking change in existing TypeScript/JavaScript code
  • [true] This wouldn't change the runtime behavior of existing JavaScript code
  • [?] This could be implemented without emitting different JS based on the types of the expressions
  • [true] This isn't a runtime feature (e.g. library functionality, non-ECMAScript syntax with JavaScript output, new syntax sugar for JS, etc.)
  • [true] This feature would agree with the rest of TypeScript's Design Goals.

💻 Use Cases

A component or function can work with two or more types. As an example let us say numbers and strings.
For the type string the component may accept additional properties, which are not applicable to numbers and vise versa.
This can be enforced by a type rule by creating multiple object types and allowing the input to only be one or the other

type inputObject = (
    {
        inputType: "number",
        multiply_by?: number
    } | {
        inputType: "string",
        makeUppercase?: boolean
    }
)
const func = (props:inputObject)=>{}

Now, it would be great if it would still be possible to use an destructuring assignment on the input object, wouldn't it?

📃 Motivating Example

The input to the function is an object. The general layout of the object stays the same, but the type rules should enforce a more specific rule, which allows properties to be of a certain type only if other properties are also of a certain specific type.

export default function Eingabefeld({
    isText=true,
    children="",
}:(
    {
        isText: true,
        children:string
    } | {
        isText: false,
        children:number
    }
)) { 
    if (isText === true) {
        let data:string = children
    }else if (isText === false) {
        let data:number = children
    }
}

TypeScript cannot tell that if (isText===true) children can only be of type string. Instead TypeScript assumes that children can be both of type number and string unaffected by the if condition.

Note: This is a oversimplified example to keep the code snipped short and clean. IRL you could use it in more complex cases, where it makes proper sense to use such a feature.

What do you want to use this for?

Being able to also destructure objects which may be of different types.

What workarounds are you using in the meantime?

Instead of destructuring the object and access the properties individually, I have to use the object and access its properties.

export function Eingabefeld(props:(
    {
        isText: true,
        children:string
    } | {
        isText: false,
        children:number
    }
)) { 
    if (props.isText === true) {
        let data:string = props.children
    }else if (props.isText === false) {
        let data:number = props.children
    }
}

Or use two destructuring assignment after the "if"-statement.

export function Eingabefeld(props:(
    {
        isText: true,
        children:string
    } | {
        isText: false,
        children:number
    }
)) { 
    if (props.isText === true) {
        let {children} = props
        let data:string = children
    }else if (props.isText === false) {
        let {children} = props
        let data:number = children
    }
}

What shortcomings exist with current approaches?

Simply more code needs to be written. When wanting to use destructuring assignments, two or more identical destructuring assignments need to be written and the alternative is to access the property from the object, requiring repeating the objects name a bunch of times, making the code more bloated in either way.

@MartinJohns
Copy link
Contributor

FYI, your example does not include any conditional types. It includes union types.

@whzx5byb
Copy link

whzx5byb commented Aug 2, 2022

Your code actually works without the default values. See #44730.

@KrischnaGabriel
Copy link
Author

FYI, your example does not include any conditional types. It includes union types.

I called it "condition" here as children can only be of type string, when isText is also true. It is a condition of sorts, but not a Conditional Type of TypeScript of course.

@fatcerberus
Copy link

I called it "condition" here as children can only be of type string, when isText is also true. It is a condition of sorts, but not a Conditional Type of TypeScript of course.

The proper term for what you are referring to is discriminated union. If you call it a conditional type you'll just confuse everyone because that refers to an entirely different concept.

@jcalz
Copy link
Contributor

jcalz commented Aug 2, 2022

https://stackoverflow.com/questions/73206067/or-expression-with-objects-does-not-behave-as-expected#comment129293189_73206067

Maybe this is a request to support discriminated union destructuring with defaults, but the issue text would need reworking to adequately convey that

@RyanCavanaugh RyanCavanaugh added the Bug A bug in TypeScript label Aug 2, 2022
@RyanCavanaugh RyanCavanaugh added this to the Backlog milestone Aug 2, 2022
@RyanCavanaugh RyanCavanaugh changed the title Preserve conditional types when using destructuring assignments on conditional object types Discriminated union parameter destructuring doesn't work if the fields have defaults Aug 2, 2022
@RyanCavanaugh
Copy link
Member

IMO this is just a bug. The presence of the defaults shouldn't matter.

@RyanCavanaugh RyanCavanaugh added the Help Wanted You can do this label Aug 2, 2022
@whzx5byb
Copy link

whzx5byb commented Aug 2, 2022

IMO this is just a bug. The presence of the defaults shouldn't matter.

Default values may produce unexpected behaviors. Consider this code:

function Eingabefeld({
    isText = false,
    children = 0,
}:(
// The default is { isText: false, children: 0 }, 
// which is a valid branch of the discriminanted union, right?
    {
        isText: true,
        children?: string
    } | {
        isText: false,
        children?: number
    }
)) { 
    console.log(isText, children);
    if (isText === true) {
        let data:string = children
    }else if (isText === false) {
        let data:number = children
    }
}

Eingabefeld({isText: true})
// oops, we got {isText: true, children: 0}

@KrischnaGabriel
Copy link
Author

IMO this is just a bug. The presence of the defaults shouldn't matter.

Default values may produce unexpected behaviors. Consider this code:

function Eingabefeld({
    isText = false,
    children = 0,
}:(
// The default is { isText: false, children: 0 }, 
// which is a valid branch of the discriminanted union, right?
    {
        isText: true,
        children?: string
    } | {
        isText: false,
        children?: number
    }
)) { 
    console.log(isText, children);
    if (isText === true) {
        let data:string = children
    }else if (isText === false) {
        let data:number = children
    }
}

Eingabefeld({isText: true})
// oops, we got {isText: true, children: 0}

TypeScript shall in this case notify the user about the occurring invalid state.

This would be another way of supplying default values, without encountering this scenario.

  function Eingabefeld({
    isText,
    children,
  }:(
    {
        isText: true,
        children?: string
    } | {
        isText: false,
        children?: number
    }
  ) = {
    isText: false,
    children: 0,
  }) { 
    console.log(isText, children);
    if (isText === true) {
        let data:string = children
    }else if (isText === false) {
        let data:number = children
    }
  }}```

@whzx5byb
Copy link

whzx5byb commented Aug 2, 2022

This would be another way of supplying default values, without encountering this scenario.

Yes, the default parameters work well, and in this case the type of children is also narrowed as expected in the branches. I think it is a better practice.

@DeyLak
Copy link

DeyLak commented Aug 27, 2023

I've encountered the same issue, while trying to write some React prop types that includes a discriminated union. Are there any news on this might be fixed?

The code, I have:

type WithIsMultiFieldProps<
  Value,
  Props,
> =
  | ({
      isMulti?: false | undefined;
      value?: Value;
    } & Props)
  | ({
      isMulti: true;
      value?: Value[];
    } & Props);

type SelectProps<I extends Record<string, any>> =
  WithIsMultiFieldProps<I, {
  label?: string;
  items: I[];
  }> 

const Select = <I extends Record<string, any>>(
  {
    isMulti, // If I add '= false' here, it will break
    value,
  }: SelectProps<I>,
) => {
  if (isMulti) {
    value
    // ^? (parameter) value: I[] | undefined
  } else {
    value
    // ^? (parameter) value: I | undefined
  }
}

@MrOxMasTer
Copy link

MrOxMasTer commented Jun 25, 2024

I really hope that this issue will also take into account the default props values that are not passed, that is, undefined for the props types undefined and never . #58976

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug A bug in TypeScript Help Wanted You can do this
Projects
None yet
Development

No branches or pull requests

8 participants