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

Problem with object spread syntax where the object type has optional values #6751

Closed
jonleighton opened this issue Aug 17, 2018 · 6 comments
Closed

Comments

@jonleighton
Copy link

Example at Try Flow

The following code:

/* @flow */

type A = { foo: ?string };

function foo(a: A) {
}

const a: A = { foo: 'foo' };
const b = { foo: 'foo' };

foo({ ...a, foo: 'foo' }); // no error
foo({ foo: 'foo', ...a }); // error

foo({ ...b, foo: 'foo' }); // no error
foo({ foo: 'foo', ...b }); // no error

Produces this error:

    3: type A = { foo: ?string };
                       ^ null or undefined [1] is incompatible with string [2].
        References:
        3: type A = { foo: ?string };
                           ^ [1]
        12: foo({ foo: 'foo', ...a }); // error
                       ^ [2]

It also produces an error if I define the type as:

type A = { foo: string | null | void };

In that case I get:

    3: type A = { foo: string | null | void };
                                ^ null [1] is incompatible with string [2].
        References:
        3: type A = { foo: string | null | void };
                                    ^ [1]
        12: foo({ foo: 'foo', ...a }); // error
                       ^ [2]
@jonleighton
Copy link
Author

Here's a workaround:

foo({ ...null, foo: 'foo', ...a });

@richardbarrell-calvium
Copy link

Here's a workaround:

foo({ ...null, foo: 'foo', ...a });

Thank you so much for this, you've saved me from having to put a heinous : any in my codebase.

@levibuzolic
Copy link

Seems like it might be a bug with the type refinement process, it's like the type is being refined too early and then being compared against.

When doing:

type A = { foo: ?string };
const a: A = { foo: 'foo' };
const bar = { foo: 'foo', ...a };

The type of bar is automatically refined from { foo: ?string } to { foo: string }, which then makes it incompatible with the original definition of A. 🤔

Spreading any null or empty object (...{}) will prevent this refinement from happening.

You can even spread the same object twice to prevent the type refinement:

foo({ ...a, foo: 'foo', ...a });

@levibuzolic
Copy link

Looking further this might actually be intentional and expected.

Consider that the following:

type A = { foo: ?string };

# The following are equivalent
const x = { foo: 'foo', ...a };
const x = Object.assign({ foo: 'foo' }, a);

Which means the inferred type of the first argument in the Object.assign is { foo: string } which is indeed incompatible with the type of { foo: ?string }.

When using spread syntax you're actually creating an object, which has its type inferred (because you haven't defined it) and then you're trying to assign an incompatible type object over it.

When really it sounds like the intention is:

const x = Object.assign({}, { foo: 'foo' }, a);
# or the same as
const x = Object.assign(null, { foo: 'foo' }, a);

I can't think of what possible bug this error is protecting us from, but I think it is technically correct in its implementation.

So the solution of first merging null or {} is seemingly correct -- though it'd be great if there was a way to opt opt of this behaviour.

@villesau
Copy link
Contributor

Made a PR to fix this: #7298

@nmote
Copy link
Contributor

nmote commented Oct 25, 2019

This code no longer errors in master

@nmote nmote closed this as completed Oct 25, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants