The object spread operator can't be used with union types

[dchambers]

Consider the following code snippet:

type X = Y | Z;
type Y = {
  type: 'Y';
  param: string;
};
type Z = {
  type: 'Z';
  param: string;
};
const f1 = (y: Y): Y => ({param: 'p', ...y});
const f2 = (x: X): X => ({param: 'p', ...x}); // this line errors but shouldn't!

Here, Both Y and Z have a param attribute of type string, so the spread operations is valid, yet it fails. The defitnition of f1 shows us that spread operator works for non-union types.

[avikchaudhuri]

Related: #1735 #1511

[cvoege]

Any progress on this?

[Jym77]

@dchambers Check out my lengthy comments on #5253
I believe this is actually correct behaviour.
The solution here is to give the correct type annotation to your function:

const f3: Y => Y | Z => Z = (x) => ({param: 'p', ...x});

The X => X type is equivalent to Y=>Y | Y=>Z | Z=>Y | Z=>Z. Out of these 4 types, your function can only have 2. There's no need to tell flow "watch out, maybe when I put a Y in I'll get a Z out".

[Hypnosphi]

@Jym77 shouldn't it rather be Y => Y & Z => Z? Because it returns Y for Y AND Z for Z

[Jym77]

Yes, maybe…

And I messed up with the priority of operators. Y => Y | Z => Z is parsed as Y => (Y | (Z => Z)) which is not what is needed, and the correct (Y => Y) | (Z => Z) raises a flow error :-(
(same with & instead of |)…

[dchambers]

@Jym77, really good spot that f2 as originally defined would effectively allow Y => Z and Z => Y, which is not what we want. The (Y => Y) | (Z => Z) union-type idea sounds reasonable in theory, though a bit cumbersome for complex types as it's not very DRY. Unfortunately, when I tried it I just ended up with a different sounding error message, but on largely the same topic.

With nominal types this would be something that generics would normally be used to solve, so I also tried this, which happens to be much DRYer too:

const f2 = <TX: X>(x: TX): TX => ({param: 'p', ...x});

but that just put me back to getting the same error as I started with in the beginning. This is surprising as I would normally read this as saying that TX is of type X, and therefore either Y or Z for any invocation of f2, which intuitively feels like it should work.

Ho hum...

[dchambers]

@Jym77, it just occurred to me that we can see how this should really work by testing the concept with Object.assign (the types for which work) instead of using the spread operator (the types for which don't).

Results from trying this are as follows:

1. const f2 = (x: X): X => Object.assign(x, {param: 'p'}) (no error until you start invoking f2, as per usage code below).
2. const f2 = <TX: X>(x: TX): TX => Object.assign(x, {param: 'p'}) (works 🎉).
3. const f2: (Y => Y) | (Z => Z) = x => Object.assign(x, {param: 'p'}) (always errors, even if you don't invoke f2).

Here's the usage code I used to prove f2:

const y: Y = f2({type: 'Y', param: 'foo'});
const z: Z = f2({type: 'Z', param: 'bar'});

[dchambers]

Updated Code Snippet:

/* @flow */

type X = Y | Z;

type Y = {
  type: 'Y';
  param: string;
};

type Z = {
  type: 'Z';
  param: string;
};

const f1 = (y: Y): Y => ({...y, param: 'p'});
const f2 = <TX: X>(x: TX): TX => ({...x, param: 'p'}); // this line errors but shouldn't!

const y: Y = f2({type: 'Y', param: 'foo'});
const z: Z = f2({type: 'Z', param: 'bar'});

[Jym77]

Awesome! That also gives a decent workaround, especially when there aren't too many fields that change (using Object.assign once is not that bad…)

[Jym77]

Hmmm…
The difference between Object.assign and spread is that the first is changing the original object (x in your example) while the second isn't.
That probably explain why flow handles it better: it knows that x has type TX and changing one field (keeping the correct type) does not changes this. While the spread creates a new object and flow needs to figure out the type and get lost.

Additionally, it makes the Object.assign workaround not suitable for, typically, reducers that should be pure functions. (and we need to first deep clone the object, then assign, which is a bit annoying)

[villesau]

Looks like this fixes the issue: #7298

[nmote]

This code no longer errors on master.