Flow: Flow does not detect default Props

[DavidBabel]

Short version, why does that happen ? : https://flow.org/try/my_bug

Medium version :

I'm using Eslint with this rules :

• "react/require-default-props"

Which basically means :

• if a props is required, you have to not set a default props for it
• if a props is optional, you have to set a default props for it

And it make a lot of sense for me.

But when i try the following code in flow : https://flow.org/try/my_bug

import React from 'react';

type Props = {
  myFct?: Function       // not required
};

class Form extends React.Component<Props> {
  static defaultProps = {
    myFct: () => {}      // but we set a default value in defaultProps
  };

  render() {
    return (
      <input
        onClick={()=> {
          this.props.myFct()
                //  ^ Cannot call `this.props.myFct` because undefined [1] is not a function.
        }}
      />
    );
  }
}

I got an error on possible undefined function, which make no sense for me at all.

I have the same error with other props which are not functions, but the function example is the more "code compact" to reproduce.

[jamesisaac]

As it says in the docs for default props:

Note: You don’t need to make foo nullable in your Props type. Flow will make sure that foo is optional if you have a default prop for foo.

So, you should be typing myFct as it's expected to be in the component after defaultProps are applied, i.e. non-optional.

[DavidBabel]

So I guess the eslint rule is incompatible with flow behaviour and i have to look 2 differents places for one info.

Not a huge fan :(

[vtereshyn]

It's a really weird issue. I have use defaultProps and all were fine, but now in some cases, flow throws an error and in some cases doesn't throw an error. Is it normal behavior? Maybe you need to re-check your implementation? I've created an issue with my problem.

When I tried to update my current version ( 0.83.0 ) to latest ( 0.89 ) I got tons of errors and exception regarding defaultProps again. But I'm using defaultProps according to your and React documentation and this is a right way.

[TrySound]

@vtereshyn Do you use defaultProps with functions?

[vtereshyn]

@TrySound not only. But I've used defaultProps with functions too. Now, I have issue with functions and with props like inputValue

[TrySound]

I had only problem with function components. And just got rid from defaultProps. It does not really need with functions where we may use default properties with destructuring.

type Props = {|
  inputValue?: string,
  onChange: () => void
|}

const Component = ({ inputValue = '', onChange }: Props) => {
  return null
}

With this way props type is more clean and you see errors about missing default in the component instead of each usage.

defaultProps historically was added to fix the problem with classes where methods consumes props from this and default props cannot be easily assigned like with closures. So I don't see any good reason to use it with function components.

[vtereshyn]

@TrySound , we had missunderstanding, sorry.
I meant that sometimes I use functions as defaultProps, like:

```
type Props = {
onChange?: () => void,
inputValue?: ''
};

class MyComponent extends React.Component{
static defaultProps = {
inputValue: '',
onChange: () => {}
};
.......
}

[TrySound]

@vtereshyn I get it. Try to remove optional type from props. defaultProps already make them optional. Or do you have another issue?

[vtereshyn]

@TrySound , no, it's incorrect. If I remove ? I shouldn't have defaultProps. It will throw flow error. So, I need an optional parameter to define defaultProps in Component

[TrySound]

@vtereshyn I'm correct actually. Look at example
https://flow.org/try/#0JYWwDg9gTgLgBAKjgQwM5wEoFNkGN4BmUEIcA5FDvmQNwBQdMAnmFnAArFjoC8cA3nTjCIAOwDCAC2SiA5lgBccABQBKODwB8cAG4RgAEwA0Q4cFFgArjABqyADaXF5MnQC+9Orntp0AWSZxEkhRLFF4LAAPGDCDdGw8GAA6IPAxMJgAHk4Ibk1BYThUGGQYYFw4AywCZEt7GBzuDQFTQrhzK1sHJyUyMhM24QB6IbgxKRl5JTUNbX43Vo93BlwxYrhUkIzmma0W4UoYSyhROEyAzfTwuCHNdyA

[TrySound]

defaultProps type is merged with props type by flow internals.

[vtereshyn]

@TrySound , yes, sorry, its a conflict between eslint and flow configuration. eslint always throws an error, if prop isn't optional and has a defaultProp

[TrySound]

Well, it means eslint plugin is broken.

[DavidBabel]

We have big problems with this eslint plugin, since it runs it's own flow version, and for what i know, it's not possible to run multiple flow instances of different version ?

[TrySound]

@DavidBabel eslint does not run flow. It just checks code patterns.

[TrySound]

Guys, just disable those defaultProps rules. They come not from react/flow teams. Recommended configs are opinions of these plugins owners.

[DavidBabel]

@DavidBabel eslint does not run flow. It just checks code patterns.

For what i experienced, eslint-plugin-flowtype or eslint-plugin-flowtype-error seems to run it, but it may be editor related. I'm not sure, i have to investigate

[TrySound]

eslint-plugin-flowtype-error does this. However it's not great for performance to proxy flow with eslint. Run flow with editor plugin instead.

[DavidBabel]

Worst than that, it broke any other flow instance you want to run.

In my company we use vscode with official flow plugin, which is not working because of this eslint plugin :(

[TrySound]

@DavidBabel Why do you use it then? It's quite useless.

[DavidBabel]

For the same reason of any legacy nodeJs project. We have a lot of project with not up to date dependancies, including flow, and we cannot rely on a single flow server version.

[TrySound]

Do not rely on a single version. Remove eslint-plugin-flowtype-error. Or disable it in the config at least.
Anyway seems like it's not flow issue at all.

[vtereshyn]

Flow plugin works, but in some cases, flow detects errors with unused props in a parent component, when children component has default props.

[DavidBabel]

I got a question on that point. Since flow runs a server, does opening multiples projects with the plugin (with potentiel different flow versions) may be an issue ?
I cannot figure it out in my day usage.

[barrsan]

Hello! I have the same problem. Solved it as follows:

import React from 'react';

type Props = {
  myFct?: Function       // not required
};

class Form extends React.Component<Props> {
  static defaultProps = {
    myFct: () => {}      // but we set a default value in defaultProps
  };

  handleClick = () => {
    const { myFct } = this.props;
    if (myFct) { // <= make sure myFct is not undefined
      myFct();
    }
  };

  render() {
    return (
      <input
        onClick={this.handleClick}
      />
    );
  }
}

Perhaps this is not an ideal solution. But in this case, both eslint and flow are satisfied :)

[DavidBabel]

It's basically 13 lines just to manage a default props.

Not a huge fan :(

[vtereshyn]

@TrySound , sorry for the suggestion, but I really suggest you install an extension which will close issues if no activity inside because I really confused when I see 2411 issues :)

[goodmind]

@TrySound I don't think it is good idea, there are still valid issues without any activity

[TrySound]

I don't think so too.

[bristoljon]

Running into this as well and reluctant to just disable the lint rule react/require-default-props or remove the ? from the optional props in the Prop type definition as I think it's much more intuitive that the Props type shows which props are optional or not without having to check the defaultProps declaration. i.e. it clearly shows the API for the consumer of the component this way

I would expect flow to infer that an optional prop will never be undefined if there is a defaultProps value set.. I guess this would be quite a major change from what the Props type currently is inside flow though?

[sergiop]

Hi, no news on this?
The only way I can bypass it, is disable the rules in .eslintrc

"rules": {
  "react/default-props-match-prop-types": ["error", { "allowRequiredDefaults": true }],
  "react/require-default-props": ["off"],
},

...but in this way I'm completely losing control on required and not required props. It's really a mess.

[kevinbarabash]

It might make sense to add a flow lint rule for this. Trying to do this within the bounds of an eslint rule in a performant way is difficult at the moment. Eslint provides access to a single AST for the file currently being linted which means if the file imports types from any other files you're responsible for parsing those other files.

[ernestostifano]

Hi guys... I've been following this issue for a while and I have to say that I lean towards @bristoljon and @DavidBabel opinion. Especially now that React team is thinking of removing defaultProps pattern (see here and here).

@jamesisaac

So, you should be typing myFct as it's expected to be in the component after defaultProps are applied, i.e. non-optional.

I know that React/Flow teams are "working together" and that Flow has been specifically designed for React from the beginning, but current behavior makes the two packages unnecessarily version/implementation dependent and favors React's defaultProps over any other alternative.

Excluding the fact that the phrase "You don’t need to make foo nullable in your Props type." sounds more like a recommendation, it makes no sense to then write "Flow will make sure that foo is optional if you have a default prop for foo." if at the end Flow will forget about defaultProps when explicitly setting a prop as optional for your component consumers.

There are multiple ways available of setting default values and many yet to be invented. React defaultProps is just one option.

If I remove defaultProps completely, explicitly set all my props as optional and use ES6 destructuring defaults, everything works. Both patterns should be equivalent from a Flow perspective. So, if tomorrow I change my mind about defaultProps and I start using one of the many alternatives, I will have to change all my types annotations? To manually mark all my props as optional?

Or the other way around, if I don't explicitly set my props as optional, and use destructuring defaults, why won't Flow assume that those props are actually optional?

Current behavior is counterintuitive. That's confirmed by the number of people facing this issue.

Also, internal type checking is important, but I think the priority will always be to deliver useful, consistent and clear types for code consumers.

Conclusion, I would say "you should be typing myFct as it's expected to be passed by the consumer", this is particularly the case for library mantainers.