Flow: Issue with required prop and JSX prop spread

[samwgoldman]

Repro

/* @flow */
import React from 'react';

function Component(props: {mandatory: string, optional?: number}){
    return <div/>;
}

function test1(){
      let props = {
        mandatory: undefined //I expected: undefined. This type is incompatible with...
      };
      return <Component {...props}/>;     
}

function test2(){
      let props = {
        mandatory: undefined //Like here
      };
      return React.createElement(Component, props);
}

I believe this behavior is caused by the {...props} bit, but haven't investigated.

[asolove]

I have a similar issue nested in React components that I can minimize to just the use of spread or Object.assign. (I'm using flow 0.29)

// @flow
type Thingy = { a: bool }

// flow generates a type error for this:
const t1: Thingy = { a: undefined }

// but not for this:
const t2: Thingy = { ...{ a: undefined } }

At first I thought maybe flow just couldn't "see through" the spread operator. But if you try:

const t3: Thingy = { ... {} }

flow gives you a type error for the missing a key. And if you try:

const t4: Thingy = { ...{ a: 3 } }

flow also gives you a type error for number not unifying with string. So it does detect type errors when using spread/Object.assign.

But, only in that context, it incorrectly unifies undefined with other types.

(Updated: to reflect that if you desugar any of the examples using the spread operator to plain old Object.assign, the same behavior occurs.)

[Foxhoundn]

This fixes your issue, because flow thinks that mandatory will ALWAYS be a string, you have to give it the maybe type.

function Component(props: {mandatory: ?string, optional?: number}){
    return <div/>;
}

Repro

[asolove]

@Foxhoundn I think you misunderstand the problem. We want the property to be mandatory and this issue shows several cases where flow fails to give an error when that property is not present.

[Foxhoundn]

@asolove bahah, sorry 😅

[asolove]

This is a pretty serious issue, more than the current title indicates. Can I recommend changing it to something like "Allows unexpected undefined in uses of Object.assign/spread operator"?

[henrikbjorn]

I am seeing this aswell.

[davidnormo]

This seems pretty serious. I'd like to use flow in our project but this bug is one thing holding us back. I'd contribute a fix but don't know ocaml.

[rstacruz]

This is still an issue. Any workarounds?

[unstubbable]

As a freelance developer with TypeScript experience issues like this deter me from accepting work on Flow projects.

[nmn]

Seems like an issue with object spreads in general. This doesn't catch the error either:

function test2(){
      let props = {
        mandatory: undefined //Like here
      };
      return React.createElement(Component, {...props});
}

[asolove]

Yeah, it's a completely generic problem about Object.assign/spread. Would be nice to get the react label off this, since it's not react-specific, and to instead consider it a core bug in the JS typing.

[rkettering]

Yeah, this is a huge showstopper for us as well - I'm experiencing the same issue, and we use the spread operator almost to the complete exclusion of any direct prop access, in our company's code. We are of course using babel to allow the spread operator.

This basically neutralizes the benefit of flow to check nested react components for us.

[aaronshaf]

Bump. Please address. This is a huge pain.

[asolove]

The changeling for v0.53.1 listed a fix for some case of spread/Object.assign typing so I re-checked this and can confirm the bug is still present in v0.53.1.