[flow] Fix Higher-order Component typing #8419
Conversation
| // Provide the theme object as a property to the input component. | ||
| export default function withTheme<BaseProps: {}>(BaseComponent: ComponentType<BaseProps>) { | ||
| const factory = createEagerFactory(BaseComponent); | ||
| const withTheme = (): HigherOrderComponent<{}, InjectedProps> => (Component: any): any => { |
There was a problem hiding this comment.
I don't get it. Why changing the API?
There was a problem hiding this comment.
It's unlikely that we will ever need configuration options.
There was a problem hiding this comment.
Consistency. Sanity. My sanity. Seriously. If you want to edit it back, you can, but I'd just prefer one chosen HOC pattern.
There was a problem hiding this comment.
Maybe @istarkov could give us some light here. For instance, I can see the pure typing.
@rosskevin Are you saying that we need HigherOrderComponent to get flow going through the HOCs?
There was a problem hiding this comment.
@rosskevin recompose seem to have HOC factories as well as HOCs depending on the use cases. I will work on this if you prefer 👼 .
There was a problem hiding this comment.
HigherOrderComponent solves the default prop not found issue linked in the description.
There was a problem hiding this comment.
Yep, for most enhancers to type them properly they must be a function, pure is bad example as it just do nothing. So based on my experience there are no other ways
|
I'm back, working on the karma tests now. |
oliviertassinari
changed the title
So this one is huge.
Essentially, I mis-typed the HOCs, which was hiding errors for static analysis. Once I corrected the HOC types based on the docs, I then ran into a flow issue where it did not recognize
defaultPropsinside the wrapped component. Luckily, there is a published type in react-flow-types that solves it.The remaining fallout was immense. Over 1700 flow errors. I have fixed all the errors, and there should be no functional changes except one:
withTheme. In the interest of consistency (and these HOCs are not easy to type), I altered the execution ofwithThemefromwithTheme(Component)towithTheme()(Component)to match everything else we have and the same pattern used inrecompose.For release notes, it isn't necessarily a breaking change, but this PR is bound to reveal existing flow errors in the user's code which was previously hidden.
Also:
babel-plugin-flow-react-prop-types$FlowFix Props|Statefrom 0.53 conversionBreaking change