Flow: Flow requests types for React Lifecycle Methods

Created on 21 Mar 2016  路  13Comments  路  Source: facebook/flow

In React components, when implementing lifecycle methods such as componentDidUpdate, which is defined in Flow's react.js as:

componentDidUpdate(nextProps: Props, nextState: State, nextContext: any, component: any): void;

(which is a typo, should be prevProps, prevState, prevContext, not that it matters for the purposes of typing)

Flow complains about missing annotations:

// @flow
import React from 'react';

export default class SomeComponent extends React.Component {
  props: {foo: string};
  state: {bar: boolean};

  componentDidUpdate(prevProps, prevState) {

  }

  render() {
    return (
      <div></div>
    );
  }
}
$ flow
foo.js:8
  8:   componentDidUpdate(prevProps, prevState) {
                          ^^^^^^^^^ parameter `prevProps`. Missing annotation

foo.js:8
  8:   componentDidUpdate(prevProps, prevState) {
                                     ^^^^^^^^^ parameter `prevState`. Missing annotation


Found 2 errors
$ flow version
Flow, a static type checker for JavaScript, version 0.22.1

Considering the declaration defines the type of these functions, why should we annotate the functions? It's difficult to do so without hoisting the typedefs.

If you annotate the methods 'incorrectly', that is, something like prevProps: number, you get an error:

foo.js:5
  5:   props: {foo: string};
              ^^^^^^^^^^^^^ object type. This type is incompatible with
  8:   componentDidUpdate(prevProps: number, prevState) {
                                     ^^^^^^ number

which suggests Flow knows the underlying type, but it chooses to emit this warning anyway.

Without hoisting the typedefs, there doesn't seem to be a good solution.

I tried:

  componentDidUpdate(prevProps: typeof this.props, prevState: *) {
    var a: boolean = prevProps.foo;
  }

which emits:

foo.js:8
  8:   componentDidUpdate(prevProps: typeof this.props, prevState: *) {
                                            ^^^^ typeof-annotation `this`. Could not resolve name

foo.js:9
  9:     var a: boolean = prevProps.foo;
                          ^^^^^^^^^^^^^ string. This type is incompatible with
  9:     var a: boolean = prevProps.foo;
                ^^^^^^^ boolean

^ is a very strange error, as it appears to be simultaneously understanding the meaning of typeof this.props, but erroring on it as well.

enhancement feature request missing annotations react

Most helpful comment

Wouldn't it be nice if the types were inferred and we didn't have to add annotations for arguments which flow could work out?

All 13 comments

Further, it is strange that return types do not need to be annotated, but parameters do. Not annotating the return type of any lifecycle method, including render(), does not generate a warning.

cc @avikchaudhuri and @bhosmer who have been discussing the annotation requirements recently

Use weak mode instead.
/* @flow weak */

In weak mode, Flow will do much less type inference. It will still infer types within functions, but will otherwise treat unannotated variables as having the any type - meaning no typechecking happens on them.

http://flowtype.org/docs/existing.html#_

@songyouwei hoisting, as @STRML suggested, still gets you type checking. in other words, just do something like:

type Props = {
  myProp: <T>
}

class myComponent extends React.Component {

  props: Props;

  componentDidUpdate( prevProps: Props ) { ... }
}

Alternatively, rather than use this (which simultaneously works and errors as noted above):

class MyComponent extends React.Component {

  props: {a: number, b: string};
  componentDidUpdate(prevProps: typeof MyComponent.prototype.props) { ... }

Edit: Given how Flow >= 0.22 can read propTypes again on React classes, this is the best way to get the type of props if you're still using static propTypes = {...} and haven't written a Flow typedef yet.

For reference, with the new $PropertyType type in core, you can use:

  componentDidUpdate(prevProps: $PropertyType<MyComponent, 'props'>){
    ...

I think this is ok, since public methods in exported class are also exported.

// @flow

import * as React from 'react';

type Props = { foo: string }

type State = { bar: boolean }

export default class SomeComponent extends React.Component<Props, State> {
  componentDidUpdate(prevProps: Props, prevState: State) {

  }

  render() {
    return (
      <div></div>
    );
  }
}

@STRML Is this still a problem or we can close this issue?

Wouldn't it be nice if the types were inferred and we didn't have to add annotations for arguments which flow could work out?

I agree with @lukeapage componentDidUpdate, componentWillReceiveProps etc. should all infer from component PropTypes.

it looks like it should work fine for componentWillReceiveProps

check this https://github.com/facebook/flow/blob/master/lib/react.js#L49

@mrkev could this be a bug? as flow asks to type nextProps?

Not a bug. Flow always asks for type definitions in methods of exported classes, as it's considered part of the "boundary" of the file. Having to infer types across file boundaries is expensive, so they are enforced.

Consider this:

No errors:

// @flow
function f(x) { return x + 1 }
f(4)

But as soon as you add an exports:

// @flow
exports function f(x) { return x + 1 } // Missing type annotation for x.
f(4)

If another file requires this one, we can find the type easily, vs having to depend on inference in this file being complete, potentially having to jump to other files, etc.

I think it should not ask for typed generics components, even if there are boundaries

as they are already typed

Hmmm wait yeah, I wanna think a bit more about this actually.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

mjj2000 picture mjj2000  路  3Comments

glenjamin picture glenjamin  路  3Comments

bennoleslie picture bennoleslie  路  3Comments

cubika picture cubika  路  3Comments

funtaps picture funtaps  路  3Comments