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

Union inference works in only some cases #23312

Closed
pelotom opened this issue Apr 10, 2018 · 4 comments
Closed

Union inference works in only some cases #23312

pelotom opened this issue Apr 10, 2018 · 4 comments
Labels
Working as Intended The behavior described is the intended behavior; this is not a bug

Comments

@pelotom
Copy link

pelotom commented Apr 10, 2018

TypeScript Version: 2.8.1

Search Terms:

union inference

Code

let result: string | number

// Works
const fromArray = <Z>(cases: Z[]): Z => cases[0];
result = fromArray(['a', 3]);

// Doesn't work
const fromTuple = <Z>(cases: [Z, Z]): Z => cases[0];
result = fromTuple(['a', 3]);

// Doesn't work
const fromVarargs = <Z>(...cases: Z[]): Z => cases[0];
result = fromVarargs('a', 3);

// Works
const fromStringIndex = <Z>(cases: { [k: string]: Z }): Z => cases.x;
result = fromStringIndex({ x: 'a', y: 3 });

// Doesn't work
const fromRecord = <Z>(cases: { [K in 'x' | 'y']: Z }): Z => cases.x;
result = fromRecord({ x: 'a', y: 3 });

Expected behavior:

All of these examples should work, with the type parameter Z being inferred as the union of the types of all values provided.

Actual behavior:

Only the array and string index cases work.

Playground Link

@RyanCavanaugh RyanCavanaugh added the Working as Intended The behavior described is the intended behavior; this is not a bug label Apr 10, 2018
@RyanCavanaugh
Copy link
Member

RyanCavanaugh commented Apr 10, 2018

If you have exactly one place where Z occurs, then you have exactly one inference candidate, and we choose that candidate. There's not something else we even could do.

If you have more than one place where Z occurs, then you have multiple inference candidates; we intentionally don't synthesize a union type to construct a new type such that all candidates are satisfied. See #805 (comment) for an old and long discussion of why. The previous behavior here, inferring a different supertype-of-both (which people hated), was tracked by #360.

@pelotom
Copy link
Author

pelotom commented Apr 10, 2018

If you have more than one place where Z occurs, then you have multiple inference candidates; we intentionally don't synthesize a union type to construct a new type such that all candidates are satisfied.

Ok, but in some cases a union type is being synthesized (the array and string-indexed objects). I could understand if it never worked, but the fact that it works in some specific cases is what's confusing to me.

@RyanCavanaugh
Copy link
Member

More specifically, we don't synthesize union types as part of inference. In any place where the call succeeds, the type (string | number)[] is already present in one of the sourcing expressions. Comments inline:

let result: string | number

// Works
const fromArray = <Z>(cases: Z[]): Z => cases[0];
// The array literal has the type (string | number)[] - we didn't synthesize that during inference
result = fromArray(['a', 3]);

// Doesn't work
const fromTuple = <Z>(cases: [Z, Z]): Z => cases[0];
// Z gets two candidates: string, number
// The array literal's type is not observed during inference
result = fromTuple(['a', 3]);

// Doesn't work
const fromVarargs = <Z>(...cases: Z[]): Z => cases[0];
// Z gets two candidates: string, number
// The union type string | number doesn't exist anywhere
result = fromVarargs('a', 3);

// Works
const fromStringIndex = <Z>(cases: { [k: string]: Z }): Z => cases.x;
// The fresh object literal has an index signature to string|number
result = fromStringIndex({ x: 'a', y: 3 });

// Doesn't work
const fromRecord = <Z>(cases: { [K in 'x' | 'y']: Z }): Z => cases.x;
// Each property maps to a Z so we have two candidates: string, number
result = fromRecord({ x: 'a', y: 3 });

@pelotom
Copy link
Author

pelotom commented Apr 10, 2018

I see, thanks for the explanation!

@pelotom pelotom closed this as completed Apr 10, 2018
@microsoft microsoft locked and limited conversation to collaborators Jul 25, 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

2 participants