Flow: Inexact rest object claims it may have properties it can't possibly have

[nwoltman]

Flow version: 0.112.0

Expected behavior

No errors

Actual behavior

• Link to Try-Flow: https://flow.org/try/#0PQKgBAAgZgNg9gdzCYAoVBLAtgBzgJwBcwAlAUwEMBjYqfOLMAcnOsKYG51CBPHMsAAV6OAM5gAvGADeqMGCowKo0QDkKWMgH4AXGC2jC+DADsA5gBo5YAHR3UAXy6oyADzxEwAEzJQKAVxhafxMaDDgTMABZHgBhBjwTMhNCAApZeUVlNQ0yK3k7GzhCAAsyfGE4MUc9SrEAShlrfDJCf3xI1Ot5AB5RHAoTbvkFJRV1TQlpLPHch2H5aULisoqRUXmRsGAAPmt6rnmgA

/* @flow */

import React from 'React';

type Props = {
  className?: ?string,
  ...
};

export default function MyComponent({
  className,
  ...otherProps
}: Props) {
  return (
    <span
      className={className}
      {...otherProps}
    />
  );
}

17:       {...otherProps}
              ^ Cannot determine a type for props [1]. rest of object pattern [2] is inexact, so it may contain `className` with a type that conflicts with `className`'s definition in props [1]. Can you make rest of object pattern [2] exact?
    References:
    15:     <span       
            ^ [1]
    12:   ...otherProps        
             ^ [2]

Flow thinks that otherProps may contain the className property because it's an inexact object, but it can't possibly contain that property because it was destructured out here:

{
  className,
  ...otherProps
}

[jcready]

I agree this is not desirable behavior, but I guess the question here is what would you expect the type of otherProps to be? We essentially would need flow to provide syntax to specify an object which doesn't have a property.

[nwoltman]

I would expect the type of otherProps to be "an object that may have any property _except_ for className".

There's certainly potential for Flow to provide a syntax to define this type of object, but I'm not sure if that would be required to solve this issue. Flow could initially implement the type internally and then provide a syntax for it later on.

[SebiTimeWaster]

i am running into the same problem, is there something to tell flow to at least ignore this? (apart from a clunky annotation, i mean)

[nwoltman]

@SebiTimeWaster You could use a suppression comment.

[jedwards1211]

@jcready Flow already does have a $Rest<A, B> utility type, and AFAICT Flow already does infer the type @nwoltman expects:

https://flow.org/try/#0PTAEAEDMBsHsHcBQAXAngBwKagAoCdZ0BnUAXlAG9QA7AQwFtMAuUI5PAS2oHMAaUWt2Y0ArvQBGmPKAC+AbkSIAxrGptQ6AsRb5CJchTqMWAcgBSqk-0HCALACZ5y1eoo3+AOi+xkACym6xDJkGlpECj7+eIFEHjZyoKAgoACieAR4QA

// @flow
type Props = { name: string, age: number };

const props: Props = {name: 'Jon', age: 42};
const {age, ...otherProps} = props;
otherProps.age;  // Error

6: otherProps.age;  // Error
              ^ Cannot get `otherProps.age` because property `age` is missing in rest of object pattern [1].
References:
5: const {age, ...otherProps} = props;
               ^ [1]

So the error in the OP is a bug that can definitely be fixed.

[jedwards1211]

I just realized though, the problem is that since Props is inexact, Rest<Props, {|className?: ?string|}> is also inexact...and I'm guessing Flow unfortunately doesn't have a way to model an inexact object that may have additional unknown props but definitely doesn't have className.

[jedwards1211]

Actually the following does work without errors so maybe the $Rest utility type can model an inexact object that definitely lacks certain props...

/* @flow */

import React from 'React';

type Props = {
  className?: ?string,
  ...
};

export default function MyComponent({
  className,
  ...otherProps
}: Props) {
  return (
    <span
      className={className}
      {...(otherProps: $Rest<Props, {|className?: ?string|}>)}
    />
  );
}

[jedwards1211]

Dude Flow is messed up. If you so much as assign otherProps to another variable of type $Rest<...>, it causes the error on otherProps to go away. I think the way Flow is designed to infer the type of a variable based upon how it's used is madness.

/* @flow */

type Props = {
  className?: ?string,
  ...
};

const props: Props = {className: 'c'}
const {className, ...otherProps} = props
// Uncomment this line and the error goes away 😱
// const defOtherProps: $Rest<Props, {|className?: ?string|}> = otherProps
const foo = {className, ...otherProps} // OP Error

[gitonthescene]

FWIW, the following also works, though I'm still struggling to understand how the spread is processed.

type Props = {|
className?: ?string,
|};

const props: Props = {className: 'c'}
const {className, ...otherProps} = props
const foo = {className, ...otherProps} // OP Error

https://flow.org/try/#0PQKgBAAgZgNg9gdzCYAoVBLAtgBzgJwBcwAlAUwEMBjYqfOLMAcnOsKYG51VCBPHMmAAK9HAGcwAXjABvAD6owYKjApixAOQpYyAfgBcYXWML4MAOwDmAGlRyAvl1RU45k2ByixhkXHFTZFTVNbTJDJiome2dXdxkg9S0dazAAOnS4QgALMnxfcXsAzz8xVGBgMABVcxcsHXNibIwJGAtBCnMAEzBswVz6fDBLODIJCgQKXjBAXg3ARj2yipc3Yk6yKAB5XryvQwASchMAHnyxFPkEkJ0DIxMzKwcAPgDMnO2SmOWwKDg4APjVRKhFLpVIvXInQrlMDrIRgACi+AGQA

[pascalduez]

This is sort of a deal breaker for UI components libraries, where the extra properties pattern is highly used. You want to be able to set defaults while still being able to override them. You can't type every possible usable props (inexact).

type Props = {
  type?: 'submit' | 'button' | 'reset',
  ...
}

function Button({ type = 'button', ...extraProps }: Props) {
  return <button type={type} {...extraProps} />
}

https://flow.org/try/#0PQKgBAAgZgNg9gdzCYAoVBLAtgBzgJwBdkwBDAZzACUBTUgY2KnzizAHJ87H31CBPHDTAAFFjkoBeMAG9UYMAKEB+AFwdyAVwBGWDIXZgAPh22bChOADtDJzjXI0DAGnlgAdJ9QBfdFE1WjBjWYABC5pZWABQyioLC0uxmFtbszh6eNAAehPikYnASYN7qBRIAlLJuXISa+FZgADzJkXFCkjJKNN6ynu7Zufni5D3AAHw+qEA

[nwoltman]

@pascalduez For that particular case, you don't actually need object rest/spread for setting defaults. You could do this instead:

type Props = {
  type?: 'submit' | 'button' | 'reset',
  ...
}

function Button(props: Props) {
  return <button type="button" {...props} />
}

You could also use the defaultProps React property:

type Props = {
  type: 'submit' | 'button' | 'reset',
  ...
}

function Button(props: Props) {
  return <button {...props} />
}

Button.defaultProps = {
  type: 'button',
}

[jedwards1211]

Actually what would be nicer in that case is to spread prop types for button into an exact Props type. Though I'm not sure there's a flow type for raw Dom element props anywhere?

[gitonthescene]

I think it would be nice to use $Rest<> as suggested above but put it directly in the type rather than have to do it with casting. Even cooler if there was some syntax for $Rest<> to be relative to the type currently being defined, though in my case I needed it relative to the props on the component I was using it in rather than the props of the component being defined.

[pascalduez]

@nwoltman Thanks, your first example.
But at the end, feels like we are surrounding the issue because Flow can't model a pattern that totally works fine in JS / React.

Furthermore this pattern will most likely fail with documentation tools like react-docgen, the default value won't be documented.

As for defaultProps I now avoid them, they are sort of deprecated (not officially), but either in React or Flow, there's a trend to avoid them.

@jedwards1211 Yes, that would be great, I think I tried some time ago but couldn't achieve anything.
Spreading DOM node types like HTMLButtonElement doesn't seem to be currently possible. (I would love to be proven wrong though).

[jedwards1211]

Yeah defaultProps become a nightmare with React.ComponentType, for instance if you pass components (which have defaultProps) as props to other components and you're trying to Flow type it.

[pascalduez]

Here's another very common case:

// @flow

import * as React from 'react';
import clsx from 'clsx';

import styles from './Foo.css';

export type Props = {
  children: React.Node,
  className?: string,
  ...
};

function Foo({ children, className, ...extraProps }: Props) {
  return (
    <div
      className={clsx(styles.root, className)}
      {...extraProps}
    >
      {children}
    </div>
  );
}

Components most often wants to provide default styling/config while still being able to inject additional classes, styles, whatever...

[Wombbu]

Not commenting on the bug here, but for those who are dealing with this issue:

You can bypass the error by declaring the spread props before other props, like this:

type Props = {
  className?: ?string,
  ...
};

export default function MyComponent({
  className,
  ...otherProps
}: Props) {
  return (
    <span
      {...otherProps} // See the order here
      className={className}
    />
  );
}

I think it's generally a good idea to declare the spread props before other props. This makes sure the props you declare won't get overwritten by the spread props.

[pascalduez]

@Wombbu thanks, in this className case yes, but in many other cases like the ones mentioned earlier, you explicitly want to be able to override...

[jbrown215]

This is not a bug and is working as intended. Flow does not infer object types that "definitely do not have a property." To express that, you should use exact object types.

The reason you are getting an error by spreading the inexact object is to avoid errors like these:

const x: {foo: number, ...} = {foo: 3, bar: 3};
const y: {foo: number, bar: string} = {bar: 'string', ...x}; //bar has type number at runtime

Since we do not track that an object "definitely does not have a property," we error when you spread an inexact object that you get from a rest operation.

@Wombbu's suggestion is the correct way to ensure you get sound behavior here. If you spread first, there is no risk of overwriting the other properties with incompatible types.

For the issue of adding overrides, here is a recommendation that is not (yet) ergonomic but can be made more ergonomic when we provide an alternative for $Shape.

type Properties = {foo: number, bar: number, baz: number};
type Overrides = $Shape<Properties>;
declare var properties: Properties;
function override(overrides: Overrides) {
  properties = {...properties, ...overrides}; // No error
}

I recommend writing out the full object type instead of using $Shape for now because $Shape has several known issues. We have work in the pipeline for Flow mapped types that will replace $Shape eventually.

[jedwards1211]

@jbrown215 why does this not error then?

/* @flow */

import React from 'React';

type Props = {
  className?: ?string,
  ...
};

export default function MyComponent({
  className,
  ...otherProps
}: Props) {
  return (
    <span
      className={className}
      {...(otherProps: $Rest<Props, {|className?: ?string|}>)}
    />
  );
}

[jbrown215]

Looks like a bug with $Rest

[nwoltman]

@jbrown215 What do you think the solution to that "bug with $Rest" should be? The only solution I can think of is what I proposed back in this comment

"an object that may have any property except for className".

[jedwards1211]

yeah it would be nice, I get the impression they don't really want to go to the trouble to model inexact types that definitely don't contain a property though, since exact types are sort of the gold standard these days

[pascalduez]

So I'm trying to go the exact props way for my components, and working a React/DOM properties definitions similar to TS.

But I quickly ran into worries:

import * as React from 'react'

type Props = {| 
  children: React.Node
|}

function Foo({ children, ...extra }: Props) {
  return <p {...extra}>{children}</p>
}

<Foo data-something="...">...</Foo>

https://flow.org/try/#0PQKgBAAgZgNg9gdzCYAoVBLAtgBzgJwBdkwBDAZzACUBTUgY2KnzizAHJ87H31CBPHDTAAFFjkoBeMAG8APmFRgw9ABYYYAEy4A7AFzVuhAHQA5OJpqo5AX3RQArjsYY4OsADE4cABQyV6lq6ADRgxuE0AB6E+KRgNgZicBIAlLJKYFyEDvjuADw4suHGUTGkNgB8Mmoa2jQ6NnnAOBWodqh5XnBgmqSEpAC05Kw0hOo6AOaSAETF0xXFTV0VQA

[jedwards1211]

There will probably always be a need for inexact props for some React components

[danielo515]

I understand that, when you spread two different objects and one is not defined, you may override with incompatible types. But, when you specifically take one property out of an object and then just spread that same object, it is impossible on javascript to override that property.
So flow should be able to track that you are spreading an object that is guaranteed to not have that property.