Flow: Flow doesn't check the defaultProp value definition

[sullenor]

Hello,

I found a small issue with defining defaultProps and props for the react component. In case I have an optional handle (onClick handler for example) and define defaultProp value for the same prop, I get an exception Function cannot be called on possibly undefined value when I try to call it.
See the example below:

// @flow

const React = require('react');

class Foo extends React.Component {
  static defaultProps: {
    onClick: Function,
  }

  props: {
    onClick?: Function, // optional
  }

  _onClick = e => {
    this.props.onClick(e);
  }
}

Foo.defaultProps = {
  onClick: () => {},
};

Foo.propTypes = {
  onClick: React.PropTypes.func,
};

 15:     this.props.onClick(e);
         ^^^^^^^^^^^^^^^^^^^^^ call of method `onClick`. Function cannot be called on possibly undefined value
 11:     onClick?: Function, // optional
                   ^^^^^^^^ undefined


Found 1 error

In my opinion this case should be valid, since the defaultProp value will be used in case the prop value was omitted (undefined).

I use flow v0.41.0

[vkurchatkin]

onClick shouldn't be optional in props

[sullenor]

Why it shouldn't be optional?

[vkurchatkin]

defaultProps doesn't affect type of this.props, it only affects component instantiation

[sullenor]

thanks

[lvmtam]

Hi vkurchatkin,
I'm not understanding what your mean?
As i understand, this.props will be initialized by defaultProps, isn't?
So, if i has already defined defaultProps, this.props.onClick will be always exist (not null or undefined)
It will be nice if you can explained it again. Sorry for my foolish.

[sullenor]

@lvmtam when react creates the component instance it internally merges default component props (defaultProps) and those, that you pass in jsx. So the typings you add in flow for props usually affect the resulting props (the component instance has).

For example, if component Foo has defaultsProps.onClick = defaultHandler, then < Foo/> and < Foo onClick={customHandler}/> will produce the same result. The Foo instance will have a onClick handler in the props (default or custom one).

So in that case the proper definition will be:

// @flow

const React = require('react');

class Foo extends React.Component {
  static defaultProps: {
    onClick: Function,
  }

  props: {
    // without "?" here, cause we would have a function 
    // in the instance anyways (from default props or from the parent component via jsx)
    onClick: Function, 
  }

  _onClick = e => {
    this.props.onClick(e);
  }
}

Foo.defaultProps = {
  onClick: () => {},
};

Foo.propTypes = {
  onClick: React.PropTypes.func,
};

[lvmtam]

Hi @sullenor ,
I'm glad when see your answer. It's so great. But in my case

`
// @flow

import React from "react"
import { Animated } from 'react-native'

type Props = {
optionalNumber?: number,
}

class CustomComponent extends React.Component<Props> {
static defaultProps = {
optionalNumber: 123
}

someMethod() {
  const { optionalNumber } = this.props
  const value = optionalNumber / 2
}

}
`

The Flow is warning me (1):
17: const value = optionalNumber / 2 ^ Cannot perform arithmetic operation because undefined [1] is not a number. References: 7: optionalNumber?: number, ^ [1]

I can not fully understanding why the Flow warns me about that. The optionalNumber as my mind will never be undefined unless the user instance a CustomComponent like that
<CustomComponent optionalNumber=undefined /> (2)
However, the Flow has already warned about that case, isn't? So, I think the warning (1) is not necessary

Have a nice day, sullenor

[sullenor]

I can not fully understanding why the Flow warns me about that. The optionalNumber as my mind will never be undefined unless the user instance a CustomComponent like that
(2)
However, the Flow has already warned about that case, isn't? So, I think the warning (1) is not necessary

Flow warns you cause you typed optionalNumber?: number as a maybe type, so inside someMethod optionalNumber can be any number, null or undefined.

Since you provide a default value to the component:

static defaultProps = {
  optionalNumber: 123
}

you can put type Props = {optionalNumber: number}. That will work fine cause inside someMethod you'll get number for sure, unless you explicitly initialize it with any other type like <CustomComponent optionNumber={null}/>.

So providing type Props = {...} via flow you describe not those props, that are passed to that component, but those props, which you can access via this.props inside component.

[lvmtam]

Hi @sullenor
Thank for your great suggestion, the perfect solution ^^
You saved my life 💃

FYI,
when I changed from optionalNumber?: number to optionalNumber: number. The new warning has come.
error propType "optionalNumber" is required and should not have a defaultProp declaration react/require-default-props
It seem to be eslint warning. So, I had fixed it by add this line
"react/require-default-props": [2, { "forbidDefaultForRequired": true }],
into .eslintrc configuration file.