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

If statement return type with union inferred incorrectly #58617

Closed
xixixao opened this issue May 22, 2024 · 5 comments
Closed

If statement return type with union inferred incorrectly #58617

xixixao opened this issue May 22, 2024 · 5 comments
Labels
Duplicate An existing issue was already created

Comments

@xixixao
Copy link

xixixao commented May 22, 2024

πŸ”Ž Search Terms

union, spread, return, inferrence

πŸ•— Version & Regression Information

  • This is the behavior in every version I tried, and I reviewed the FAQ for entries about union, spread return type

⏯ Playground Link

https://www.typescriptlang.org/play/?ts=5.5.0-dev.20240522&ssl=16&ssc=1&pln=1&pc=1#code/PTAEBUAsEsGdQGYFcB2BjALtA9ig5PAE4CmGShKoGAngA7GiyTZIA2AJqAEbEBQIoAN60AXKBRIAtj0IBfUAB8hoidOKEANKEljYGQtBQBzWfzBckGUNCtxlY1TNNpcexKlABeUAAoXKdjEubGxWYgBDFABKLwA+IV5QUH83AA8vIVBRUABGUFkAbkTrBF9-dhjBYqSUqxdpQ2JOb0FQADoO1K0dUAAiSGJWVmxe-KKkpJIyCmTsBpQm8fzQQdgGKonQARJaVnC0BgB3G0hQAAMp8kpeoxD2XrPEbEJZwhJMUEuZmnpqz9IrqBUktTEkBFA7IdngBreAIRrFbYAmblUAAfkyHTaXW0Yn6g2Go3kYmBvEKvAp4Jg8ChhFhiAR7GIaD2JFmKDc5SCITCkSKtSBGWEIhy5IF1AyqIxrSxOJ6+KGI2WJIKQA

πŸ’» Code

// This function's return type should be
// {p: number} | {p:number, m: string}
// but it is {p: number}
const fun = (cond: boolean) => {
  const x = { p: 1 };
  if (cond) {
    const combined = { ...x, m: "hello" };
    return combined;
  } else {
    // replace with `return "good"` for correct return type
    return x;
  }
  // This works fine
  // return cond ? { ...x, m: "hello" } : x;
};

πŸ™ Actual behavior

The inferred function return type doesn't include both branches of the if statement.

πŸ™‚ Expected behavior

The inferred function return type should include both branches of the if statement, as if ternary was used.

Additional information about the issue

No response

@nmain
Copy link

nmain commented May 22, 2024

This is expected subtype reduction behavior. Typescript can only infer your desired type in specific scenarios with fresh objects; see #19513.

@xixixao
Copy link
Author

xixixao commented May 22, 2024

@nmain can you elaborate? TS correctly infers the type in each branch, and correctly infers the return type when a ternary is used.

Edit: If anything I could imagine the reduction would reduce the return type to {p: number, m?: string} but that's never the case.

@nmain
Copy link

nmain commented May 22, 2024

when a ternary is used

Because they're fresh object literals in that case. Search "subtype reduction" and you'll find various previous issues about this; for instance, #52893.

@RyanCavanaugh RyanCavanaugh added the Duplicate An existing issue was already created label May 22, 2024
@xixixao
Copy link
Author

xixixao commented May 23, 2024

@RyanCavanaugh why not infer unknown if undefined cannot be inferred? Why not keep the union? From reading the original PR that's still not clear to me. For my original example, return type of fun could be either be:

  1. {p: number} | {p:number, m: string} - m cannot be used without type narrowing
  2. or {p: number, m: unknown} | {p:number, m: string}, m is unknown without type narrowing.

I would also add to @RyanCavanaugh's comment:

When the complaint is that the behavior is "inconsistent", it seems counterproductive to introduce a new set of rules that make predicting the output much harder than before.

Immediately-returned object? Yes. Anything else? No

Immediately-returned object? Yes. Intermediate const? Maybe, depends on the initializer. Function calls? Method calls? Passing the object somewhere else? What if the object goes into a generic call? Epicycles.

There will be confusion somewhere. But "you have a function call here, passing in the object, so we can't tell whether there are extra properties on it afterwards" is less confusing than the current behavior imo (this was a common issue in Flow, but people working with it learned it). So tracking object literals / "closeness" would be preferable.

@typescript-bot
Copy link
Collaborator

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

@typescript-bot typescript-bot closed this as not planned Won't fix, can't repro, duplicate, stale May 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Duplicate An existing issue was already created
Projects
None yet
Development

No branches or pull requests

4 participants