brigand / babel-plugin-flow-react-proptypes

A babel plugin to generate React PropTypes definitions from Flow type declarations.
MIT License
432 stars 49 forks source link

callable types unhandled? #26

Closed rosskevin closed 8 years ago

rosskevin commented 8 years ago

I receive Warning: Failed propType: Required prop onBlur was not specified in TextField.

This flow code:

declare function ValueHandler (value:string):void

type Props = {
  value:?any,
  name:string,
  errors:?any,
  onChange:ValueHandler,
  onBlur:ValueHandler
}

type State = {}

class TextField extends Component<void, Props, State> {
  log:Logger = new Logger(this)
  props:Props
  state:State;
}

Generates:

TextField.propTypes = {
      value: __webpack_require__(4).PropTypes.any,
      name: __webpack_require__(4).PropTypes.string.isRequired,
      errors: __webpack_require__(4).PropTypes.any,
      onChange: __webpack_require__(4).PropTypes.any.isRequired,
      onBlur: __webpack_require__(4).PropTypes.any.isRequired
    };

The onChange and onBlur are specified, not null, and of the signature I have specified.

Am I missing something or is this not yet supported?

marcusdarmstrong commented 8 years ago

That warning is in how you're using TextField, not the generated proptypes.

rosskevin commented 8 years ago

hmmm, here's a gist (I'm working on an integration with react-formal): https://gist.github.com/rosskevin/e166b1a174f961ac61a091930dfcefa7

Do you spot the misuse?

rosskevin commented 8 years ago

maybe related: https://github.com/reactjs/react-redux/issues/6

es7 use?

marcusdarmstrong commented 8 years ago

You're not specifying an onChange or onBlur here: https://gist.github.com/rosskevin/e166b1a174f961ac61a091930dfcefa7#file-signin-js-L71

rosskevin commented 8 years ago

That's a react-formal Field. I register my TexField as a handler for password, and Field passes those props to TexField.

Effectively:

<Field>
  <TextField {...this.props} onChange={x} onBlur={x} />
</Field>
marcusdarmstrong commented 8 years ago

Right. But you never pass those props to Form.Field. So it never passes them to TextField.

There's nothing in here: https://github.com/jquense/react-formal/blob/master/src/Field.jsx that's going to pass an onBlur or onChange handler to your TextField without it having been passed to the Form.Field.

rosskevin commented 8 years ago

Maybe I'm not describing it well. I don't want to/intend to pass these handlers...ever. react-formal Form.Field dynamically connects the Form's handlers (onChange && onBlur) and passes them and other information to my TextField. My client code (Signin) needs no knowledge of these handlers.

I know with certainty that TextField is receiving these handlers and they are not null.

Here (my TextField is Widget) is instantiated by Form.Field: https://github.com/jquense/react-formal/blob/master/src/Field.jsx#L426

I don't spot onBlur specified as a required propType in react-formal Field. Is this the source of the Warning? That we have a broken chain of requires? Meaning I am receiving them, but the propTypes don't guarantee it based on them being missing in react-formal Field?

marcusdarmstrong commented 8 years ago

Ah, didn't realize it was context aware to the Form. But even so, your Form doesn't have an onBlur, so I don't know what your password field would be receiving for that prop.

And no, propTypes are runtime checks. There's nothing static about them/no guarantees of any kind, so if you're getting a warning from them (as you are here), the warning is indicating a specific example of a runtime type check (in this case, your onBlur prop not being provided) failing.

rosskevin commented 8 years ago

Ok, thanks much for the look, helps my understanding. I'll figure out where/when/why it is missing, certainly if the check is implemented, it's a good warning!