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

Object spread not mapping optional types correctly with editing values after spread #54687

Closed
MrSimmmons opened this issue Jun 17, 2023 · 2 comments

Comments

@MrSimmmons
Copy link

Bug Report

🔎 Search Terms

Spread, Object, Type, Optional, Interface

🕗 Version & Regression Information

  • This is the behavior in every version I tried, and I reviewed the FAQ for entries about Interfaces & generics (though this doesnt really fit into any of the FAQs)

⏯ Playground Link

Playground link

💻 Code

interface SourceData {
  id: string;
  // some other data
  start_date: string;
  end_date?: string;
}

interface UseableData {
  id: string;
  // some other data
  start_date: Date;
  end_date?: Date;
}

function transform(props: SourceData): UseableData {
  return { // Error `end_date` is of type `string | Date | undefined`
    ...props,
    start_date: new Date(props.start_date),
    ...(props.end_date && { end_date: new Date(props.end_date) })
  };
}

🙁 Actual behavior

I am getting an error on the return statement saying that it is not assignable to type 'UseableData' where end_date is of type string | Date | undefined.
Why I believe this is a bug is because I am adding the Date version of end_date if the optional end_date string exists in the first place. So if end_date doesnt exist, then it would return the undefined type from ...props, and if the end_date string did exist, it would get parsed through the Date class and be added to the return object. So the only possible return type for end_date is Date | undefined which matches that in UseableData

🙂 Expected behavior

No type error should occur because we are optionally adding the end_date key to the return object only if it has a value

🥸 Partial workaround

I'm aware that I can use the ternary operator like so:

{
  end_date: props.end_date ? new Date(props.end_date) : undefined,
}

Which is fine if you are just grabbing values from the returned object. However if you were to call Object.keys() on the returned object with end_date being specifically defined as undefined by the ternary operator, then end_date would appear in its result.
Where as going:

{
  ...(props.end_date && { end_date: new Date(props.end_date) })
}

Wont include end_date in the returned object.

Hence why this is only a partial workaround

@jcalz
Copy link
Contributor

jcalz commented Jun 17, 2023

Seems like a #30581-type issue, quite similar to #45769. TS doesn't do "distributive control flow analysis"; it doesn't analyze your object literal imagining that props.end_date is a string and then imagining that it is undefined; instead it analyzes the literal once, and if props.end_date is string | undefined when considering ...props, then that won't be affected by the later check implied by props.end_date && { end_date: new Date(props.end_date) }. The control flow analysis there doesn't expand in scope to the whole object literal. It wouldn't be scalable to do it that way.

It's actually even worse than that; there's also #42384: checking props.end_date doesn't narrow props because props isn't of a discriminated union type (or of a union type at all). So even if TS did analyze the whole object literal multiple times, it would fail to do what you want:

if (typeof props.end_date === "undefined") { // remember, empty strings are falsy
  props.end_date // undefined 👍
  const oops = { ...props }    
  oops.end_date // string | undefined, NOT NARROWED 👎
}

So I don't imagine this will be easily fixable or changed.


If I were to try to do what you're doing, the workaround I'd use is to explicitly destructure into separate variables and then reassemble them like

function transform(props: SourceData): UseableData {
  const { end_date, start_date, ...rest } = props;
  return {
    ...rest,
    start_date: new Date(start_date),
    ...(typeof end_date === "string") && { end_date: new Date(end_date) }
  }
}

since that's much easier on the poor, overburdened type checker.

Playground link

@MrSimmmons
Copy link
Author

Thank you very much for your insight @jcalz! I can see why implementing something like this would be problematic at best.

I was not aware that you could create a encompassing ...rest variable that just contained everything not specifically defined in the deconstruction. So thank you for that as well!

since that's much easier on the poor, overburdened type checker.

It is certainly doing a lot haha.

Ill close this as the workaround you provided works well.
Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants