flow-typed / flow-typed

A central repository for Flow library definitions
https://flow-typed.github.io/flow-typed/
MIT License
3.76k stars 1.33k forks source link

Error in type definitions for react-intl library #1529

Open blekit opened 6 years ago

blekit commented 6 years ago

After upgrading to the latest type definition for react-intl library (react-intl_v2.x.x.js) in our project we started getting errors when using components wrapped with injectIntl HOC. Example error message states:

Error: src/components/header/Header.jsx:42
                     v-------------
 42:                 <NavigationBar
 43:                     redirectToRoute={this.redirectToRoute}
 44:                     {...this.state}
 45:                 />
                     -^ props of React element `NavigationBar`. This type is incompatible with
163:   IntlInjectedComponentClass<$Diff<OriginalProps, InjectIntlProvidedProps>>;
                                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ empty. See lib: flow-typed/npm/react-intl_v2.x.x.js:163

From what I've managed to investigate it looks like$Diff produces an empty type in following definition:


declare function injectIntl<OriginalProps: InjectIntlProvidedProps>
  (
    component: React$ComponentType<OriginalProps>,
    options?: InjectIntlOtions,
  ):
  IntlInjectedComponentClass<$Diff<OriginalProps, InjectIntlProvidedProps>>;
``` - it seems that the type of `OriginalProps` is incorrect - it's the same as the second parameter to `$Diff` and I think it should be changed to the type of original properties of component passed to this function.
TomiS commented 6 years ago

We just finished updating a fairly large project with most files wrapped in injectIntl to use the latest flow and latest react-intl flow typings. I had to do 2 things to make it work without errors.

1) Fix ComponentWithDefaultProps definitions by changing from:

declare type ComponentWithDefaultProps<DefaultProps: {}, Props: {}> =
    React$ComponentType<Props> & { defaultProps: DefaultProps };

to

declare type ComponentWithDefaultProps<DefaultProps: {}, Props: {}> =
    React$ComponentType<Props> | React$StatelessFunctionalComponent<Props> | React.ChildrenArray<void | null | boolean | string | number | React.Element<any>>;

I.e. More input component variations needed to be allowed than just React.ComponentType.

2) Change all of my injected function components to extend either React.Component or React.PureComponent (we surprisingly only had 2 of them). I think this is because injectIntl is currently typed to return a Class and not function (see: https://github.com/flowtype/flow-typed/blob/cc3eacb5a238cb59b2f9181672a3048b1d56f60d/definitions/npm/react-intl_v2.x.x/flow_v0.57.x-/react-intl_v2.x.x.js#L144-L146). Should be an easy fix but I just didn't have time to experiment with it yet.

jkimbo commented 6 years ago

Just changing the ComponentWithDefaultProps definition from:

declare type ComponentWithDefaultProps<DefaultProps: {}, Props: {}> =
    React$ComponentType<Props> & { defaultProps: DefaultProps };

to

declare type ComponentWithDefaultProps<DefaultProps: {}, Props: {}> =
    React$ComponentType<Props> | React$StatelessFunctionalComponent<Props>;

worked for me

apdaros commented 6 years ago

Thanks, @TomiS! Since it is currently impacting our team, we thought about opening a PR with your suggestion.

callumlocke commented 6 years ago

@TomiS's fix worked for me too. Opened at #1829

callumlocke commented 6 years ago

That PR is merged. Not sure if this can be closed now, or if other aspects of this issue remain unfixed?

So I think this issue is partially fixed now, but the 2nd point in @TomiS's comment is still a problem, i.e. that injectIntl can't be used with stateless functional components without causing type errors (if I've understood correctly).

dashed commented 6 years ago

@callumlocke Do you have a minimal example for something that's supposed to work? I can't really reproduce this anymore.