First of all thanks so much for 0.53.0 team! This addressed a lot of the issues my team was facing with using Flow. Specifically we want Flow typing for React component props and I noticed this issue.
If you take the example from your HoC doc and put it in the flow playground (see here) everyting works great. However if you have the HoC have it's own file, import it into a file that exports a component that's wrapped with the HoC and use that component in another file you get an error. Note that if you convert MyComponent
from a React Component class to a stateless functional component the error goes away. Seems like a bug, right? 馃
// hoc.js
// @flow
import * as React from 'react'
export function injectProp<Props: {}>(
Component: React.ComponentType<{ foo: number } & Props>,
): React.ComponentType<Props> {
return function WrapperComponent(props: Props) {
return <Component {...props} foo={42} />;
};
}
// component.js
// @flow
import * as React from 'react';
import { injectProp } from './hoc'
class MyComponent extends React.Component<{
a: number,
b: number,
foo: number,
}> {}
export default injectProp(MyComponent);
// other-component.js
// @flow
import MyComponent from './component.js'
...
<MyComponent a={3} b={4} />
....
$ flow
Error: src/js/apps/main-future/views/rfps/my/view.js:90
v--------------------
90: <MyOpportunitiesTable
91: a={3} b={4}
92: />
-^ React element `MyOpportunitiesTable`
v
6: class MyComponent extends React.Component<{
7: a: number,
8: b: number,
9: foo: number,
10: }> {}
^ property `foo`. Property not found in. See: src/js/apps/main-future/views/rfps/my/components/table/index.js:6
v--------------------
90: <MyOpportunitiesTable
91: a={3} b={4}
92: />
-^ props of React element `MyOpportunitiesTable`
Found 1 error
error Command failed with exit code 2.
~/buildingconnected/client (flow-upgrades) $ yarn flow
yarn flow v0.27.5
$ flow
Error: src/js/apps/main-future/views/rfps/my/components/table/index.js:14
14: <Foo a={3} b={4} />
^ Unexpected identifier
Found 1 error
error Command failed with exit code 2.
I think this is the same issue as in #4644. The solution there might be instructive for you.
@asolove thanks for the pointer, I'll try to work around this for now! That said since it's only happening to React Component classes (not stateless functional components) and only when imported and exported from files (not when all in the same file) I think this seems like something that warrants a fix so :crossed_fingers:!
Hah, you're totally right. But, it seems to work if I change { foo: number } & Props
to { foo: number, ...Props }
. Does that work in your project?
It does work for me :)
Awesome! In that case, I think we can close this issue.
@asolove @samwgoldman thanks for looking into this and giving me feedback so quickly! Good news: no more errors! Bad news: it doesn't error when it should!
So if you change the intersection (React.ComponentType<{ foo: number } & Props>
) to a spread (React.ComponentType<{ foo: number, ...Props }>
) then you somehow lose all type safety in the last mentioned file of the example (see below) as well as in the component class that is enchanced (also see below). Any tips?
// other-component.js
// @flow
import MyComponent from './component.js'
...
<MyComponent b={4} /> // no errors about prop "a" being missing
....
// component.js
// @flow
import * as React from 'react';
import { injectProp } from './hoc'
class MyComponent extends React.Component<{
a: number,
b: number,
// foo: number, // if you comment out foo it no longer errors and complains that there's no "foo" prop in the component passed to "injectProp"
}> {}
export default injectProp(MyComponent);
Ugh, damnit. Let me look some more. Pretty sure this is doable.
@calebmer
I鈥檓 working on a fix for this bug. Thanks for reporting!
You鈥檒l notice that these examples work fine when they are in the same file because Flow does local type inference. However, Flow does not infer types across modules and so converts types that would have been inferred to any
. There are a couple different fixes for this. I鈥檒l update you when we have a fix.
@calebmer any estimations?
BTW, I think it should be explicitly pointed in the docs that there's no possibility to type HoCs currently.
Plus, it seems that we need a bug
label here.
This should have been fixed since 0.55 馃槉. In 0.57 we are going to update the docs to recommend that you use $Diff<>
instead:
import * as React from 'react'
export function injectProp<Props>(
Component: React.ComponentType<Props>,
): React.ComponentType<$Diff<Props, { foo: number }>> {
return function WrapperComponent(props: Props) {
return <Component {...props} foo={42} />;
};
}
Nice!
Thanks for the answer!
@calebmer it's always hard for me to understand what the flow team are working on and what to expect and when. Do you have some publicly available info about it?
Do you have somebody who work as a maintainer of this github repo and who is responsible for issues' statuses / labels / answers / etc?
@calebmer but it seems that such usage of $Diff can't help with the problem (on flow 0.56 at least)
Why was this issue closed? It's not working at all, even with the new example.
I made a example with create-react-app here: https://github.com/kennethaasan/flow-hoc/blob/master/src/components/Link.js#L41
EDIT: Adding react-flow-types works: https://github.com/kennethaasan/flow-hoc/pull/1/files
@calebmer the example from docs works if everything is in one file, but does not when the HOC is exported from a different file: https://github.com/facebook/flow/issues/5064
Most helpful comment
I鈥檓 working on a fix for this bug. Thanks for reporting!
You鈥檒l notice that these examples work fine when they are in the same file because Flow does local type inference. However, Flow does not infer types across modules and so converts types that would have been inferred to
any
. There are a couple different fixes for this. I鈥檒l update you when we have a fix.