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

incorrect union type inference in function returns #57563

Closed
AlexCaston opened this issue Feb 27, 2024 · 11 comments
Closed

incorrect union type inference in function returns #57563

AlexCaston opened this issue Feb 27, 2024 · 11 comments
Labels
Working as Intended The behavior described is the intended behavior; this is not a bug

Comments

@AlexCaston
Copy link

AlexCaston commented Feb 27, 2024

πŸ”Ž Search Terms

subtype reduction
incorrect union type inference in function returns

πŸ•— Version & Regression Information

This happens on all the typescript versions I tried on the playground.

⏯ Playground Link

https://www.typescriptlang.org/play?ts=5.3.3#code/FAMwrgdgxgLglgewgAgGIIcgFASmQb2GWThGwFkBDGACwDoAnSiAEwQFtdkA+ZABjoBWPIWLEGAUxhgGKfMhAYAXMgDkq5AF8iW4DsnTZBBcrWqANMgBGlBivW7tUJAGcYJzAF40GLsAD0-mLIAHrIMACeAA4SyABE8ooIKm4McBAA5pY2DAD8KpAsEiDpEixayAA+xkkpMGmZ2bZ1DRlaccgAVmBuyJQuyBIAHjGwZXR64NDwSMgAQrbYIjrOEL2UyN6Jpg7axKu9Vps1OxbWzWa6OqQU1PRMrBxcvALCBDriUjIolDra+l8jFZgE5XO4cscFgxcAEgmIwpEYvFtslkKl0m1NHFLHEch0ogwEDEGJFkBkpAMEFYADZwABucGoZWsEXC0QkLigaSiMD0sPCNDgA0o1JcmBolCiMTWyAA7nBaOEJLJbHAOaBILBECh0AgAExLd6fQwoKi0RjMNicPAvITIXInVG7ZAqFH2M45d2OYAHdxJA3eXV6riBQYjCRjcqIiR8qZa2ZQg1cUTIX19Y5uy57VNg6wZjzupp2LMAk3IM33S1PG38O0OjYqYGgtbg2wB+ZtkNBSDDUYwZnRvShkVi5ASqUSGXyxXkiDKuBQFxKYDR5AASQAShywNT3AAeAAqvC2yDpIrAEhUB90cZmKC3Lh3+6P2DP1IvV5wKk3293h94KYGN8xhvhe3oatM2odgwADMhqAYCpp3BajzWjwtaCPayAPk+WCZrseAqDhu54QWZhFl6mg4CCPq5jkcHeFCMFdsgPbhpGbIxHQ67ETAe6ZuimRaNwQA

πŸ’» Code

function Foo () {
  return Math.random() > 0.5 ? { foo: '' } : { foo: '', bar: '' }
}
const foo = Foo()

function Bar () {
  const a = { foo: '' }
  const b = { foo: '', bar: '' }
  return Math.random() > 0.5 ? a : b
}
const bar = Bar()

πŸ™ Actual behavior

foo type is as expected: "{ foo: string, bar?: undefined } | { foo: string, bar: string }"
but bar type is not: "{ foo: string }". bar property gets obliviated by typescript

πŸ™‚ Expected behavior

both of the functions infer the type as the same: "{ foo: string, bar?: undefined } | { foo: string, bar: string }"

Additional information about the issue

I read on another issue this behavior is expected and called subtype reduction.
It still doesn't make any sense to me and cant fathom when this can be useful.

I think a better aproach would've been infering "{ foo: string, bar?: string }" instead of just removing bar completely. Infering "{ foo: string, bar?: undefined } | { foo: string, bar: string }" as Foo does on the example would be awesome, since that would allows us to perform type narrowing later on.

The typescript documentation should be clear about this "subtype reduction" behavior, as I couldn't find anything related to it on the documentation.

I also think the behavior should be consistent, whatever behavior is decided to be the correct behavior, should apply in both cases Foo and Bar on my example

@Andarist
Copy link
Contributor

To quote @RyanCavanaugh (see his comment here):

Optional inference only happens when all input candidates are fresh object literals. See #19513

@RyanCavanaugh RyanCavanaugh added the Working as Intended The behavior described is the intended behavior; this is not a bug label Feb 27, 2024
@RyanCavanaugh
Copy link
Member

RyanCavanaugh commented Feb 27, 2024

The functions are typed differently because, from our perspective, they actually really are different in a way that matters (since there are no exact types).

If you have

const foo = { x: 1 };

this is the same as

function getFoo() {
  return { x: 1 };
}
const foo = getFoo();

which is equally the same as

function getFoo(): { x: number } {
  const obj = { x: 1, y: 2 };
  return obj;
}
const foo = getFoo();

So either we have to do something wrong by forgetting that the foo in the last example possibly has a y property, or the "inconsistency" is moved to a different point on the chain (doesn't fix anything), or we fail to provide the good behavior in #19513

@MichalMarsalek
Copy link

Shouldn't this work with as const though?

function Bar () {
  const a = { foo: '' } as const
  const b = { foo: '', bar: '' } as const
  return Math.random() > 0.5 ? a : b
}
const bar = Bar()

@Andarist
Copy link
Contributor

The same rule applies here - those are not fresh object literals anymore. It keeps the return type as a union if you keep your as const objects within the return statement:

function Bar () {
  return Math.random() > 0.5 ? { foo: '' } as const : { foo: '', bar: '' } as const
}
const bar = Bar()
//    ^? const bar: { readonly foo: ""; readonly bar?: undefined; } | { readonly foo: ""; readonly bar: ""; 

@MichalMarsalek
Copy link

I understand that, let me rephrase: Is there a reason this couldn't be extended to work with variables having literal types?

@Andarist
Copy link
Contributor

The key insight behind this PR is that for object literal types we know that unspecified properties always have the value undefined. This is unlike regular object types where unspecified properties might have any value.

For TypeScript your example above is somewhat equivalent to this:

function Bar(
  a: {
    readonly foo: "";
  },
  b: {
    readonly foo: "";
    readonly bar: "";
  },
) {
  return Math.random() > 0.5 ? a : b;
}

and now - due to the structural nature of TypeScript - we can't guarantee that a doesn't have a bar property with a non-undefined type. This is a valid call:

const a = { foo: "", bar: "not undefined" } as const;
const b = { foo: "", bar: "" } as const;
const bar = Bar(a, b); // ok

@MichalMarsalek
Copy link

MichalMarsalek commented Feb 28, 2024

Gotcha. So it cannot be based just on types, it would have to be based on the fact that the variable is const defined within the function and so we know there isn't anything extra. But it probably doesn't make sense to hardcode handling of a special case like this into ts.

@Andarist
Copy link
Contributor

it would have to be based on the fact that the variable is const defined within the function and so we know there isn't anything extra.

Yes. Some rules could be implemented around this but the proposed rule is too simple:

function mutate(obj: Record<string, unknown>) {
  obj.bar = "not undefined";
}

function test() {
  const a = { foo: "" } as const;
  const b = { foo: "", bar: "" } as const;

  mutate(a);

  return Math.random() > 0.5 ? a : b;
}

So you can see how TS is doing the safe bet here by being conservative about it.

@fatcerberus
Copy link

The direction this discussion took ended up skirting pretty close to #57387 (comment) - the idea of maintaining literal freshness until the actual point of use is an interesting one. The possibility of mutation in the interim throws a wrench in there, but given that TS assumes non-mutating objects in general ({ foo: number } is assignable to { foo: string | number }, etc.) I'm not sure it's a very strong counterargument.

@RyanCavanaugh
Copy link
Member

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 <const T> generic call? Epicycles.

@typescript-bot
Copy link
Collaborator

This issue has been marked as "Working as Intended" 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 Mar 2, 2024
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

6 participants