Flow: [BUG] 57-59 React HOC docs example (injectProp) does not work when in separate files

[rosskevin]

├─ [email protected]
└─ [email protected]

I have created a repo with a reproduction: https://github.com/rosskevin/test-flow-hocs

This uses the example from docs https://flow.org/en/docs/react/hoc/

(tried with flow versions 57-59)

• The allInOne.js shows that the docs example works when in one file (and in try flow)
• The index.js shows that the same code in a separate file (MyEnhancedComponent.js) fails.

$ flow --show-all-errors
Error: src/index.js:10
 10:         <MyEnhancedComponent a={1} b={2} />
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ props of React element `MyEnhancedComponent`. This type is incompatible with
                                               v
  6: class MyComponent extends React.Component<{
  7:   a: number,
  8:   b: number,
  9:   foo: number,
 10: }> {}
     ^ object type. See: src/MyEnhancedComponent.js:6
  Property `foo` is incompatible:
                                                   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/MyEnhancedComponent.js:6
     10:         <MyEnhancedComponent a={1} b={2} />
                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ props of React element `MyEnhancedComponent`


Found 1 error

TL;DR

Flow is erroring on the HOC example from the docs when used in separate files, while an all-in-one like Try flow works fine.

This is a major bug (reproducible) and there is no workaround.

[rosskevin]

Note: this is the straw that broke the camel's back. Lack of HOC support has caused me real pain (tens of hours), and now that I cannot get it to work at all, I have to abandon flow. Flow has been great in a lot of ways, but lack of HOC support is something I cannot continue to dismiss.

I'm off to typescript, best of luck.

[rsolomon]

Observation: It looks like swapping Props and { foo: number } on line 6 of allInOne.js recreates the issue for the single-file usage of HoC as well. It's noted in the documentation that "...intersection order is important here!"

Seems like whatever is creating Props by diffing elsewhere is lost when the definitions are split between files.

[rsolomon]

@rosskevin I know you said you've moved on, but I maybe found a solution...

diff --git a/src/allInOne.js b/src/allInOne.js
index 8736613..646e96b 100644
--- a/src/allInOne.js
+++ b/src/allInOne.js
@@ -3,7 +3,7 @@
 import * as React from 'react';

 function injectProp<Props: {}>(
-  Component: React.ComponentType<{ foo: number } & Props>,
+  Component: React.ComponentType<{ ...Props, foo: number }>,

Properly type checks. I confirmed it's not just silently ignoring types, either.

I have a feeling it introduces its own set of problems, particularly with composition, but it at least passes this particular scenario.

[rsolomon]

Directly related to https://github.com/facebook/flow/issues/3522

[rosskevin]

Good work @rsolomon and nice find if the spread works. Note that spread changes Props, so it would need to be ...$Exact<Props> or the equivalent to be technically correct. Nonetheless, good work. I also agree that this is impacted or caused by #3522.

I'm far enough gone on to Typescript now (10 days by the looks of it) that I have experienced both type systems and communities, and I'm firmly gone now. In large part due to community and support - I'm mostly on my own with flow and not nearly as much with Typescript. I'm happy to have a discussion with flow committers about what I have seen in both, now that I've been deep into each (as a user), but I don't feel the need to do a comparison here.

What I do want to post, is user sentiment. This is a repost from @AriaMinaei https://github.com/digiaonline/react-flow-types/issues/20#issuecomment-345953452. I think it is relevant and well stated, and I now agree with his perspective.

This is off-topic, but FWIW, I've given up on flow. I've found myself spending way too much time wrestling with the type system, making sense of the cryptic errors, and having to do all of this every few days because I've either hit an edge case or a new version of flow has broken something.

Even though I haven't really measured how much time I spend on these issues precisely, I'm convinced that it's been a net loss of productivity for me.

I've written about it before on HN:

I advocated flow for two years, and I still find it interesting as a typesystem. But what finally led me to give up was the core team's lack of community engagement.

It's very difficult get the team's attention to a bug or a question. As a result, they fail to see the common pain points of actual users.

Typing higher order components is one common pain point that still doesn't have a clear solution. So, unless you're avoiding HOCs altogether, you can't really benefit from type annotations in your react codebase (you'll have too many implicit anys).

You'll find more of these common use cases that flow doesn't optimise for. They're all over the issues section. Unfortunately, issues on Github generally don't get much attention from the team either, so I'm guessing users sometimes give up on reporting them. This has happened to me quite a few times. I didn't report some bugs because putting together a reduced test case takes time, and that time would be wasted if your issue is not gonna be paid attention to.

What makes matters worse, is that there is no public roadmap. You don't know what the team's priorities are. You don't know when the bugs that affect your work are gonna get fixed, or some feature you need is gonna get implemented. You don't even know if it's on the radar.

Of course, the Flow team is under no obligation to do any of that. They have no obligation to fix bugs for us or publish a roadmap. I'm already grateful that they've given us access to their work and without charging money. However, they do have the responsibility to communicate what their priorities are. If they're positioning Flow as an alternative to TypeScript, which is a well-supported, community-oriented project, then they should state clearly that Flow simply isn't. Call it a research project, or a project focused on a single company's use cases. Don't ask people to bet their own projects on it, when they clearly shouldn't. It'll be a big loss of productivity for them down the line, and your messaging is partly responsible for that.
I'll happily keep reviewing and merging PRs on this repo, but personally, I've lost interest in flow.

[calebmer]

I apologize for not responding sooner. The workaround for this bug is to use spread types, as @rsolomo mentioned, or to use $Diff. The latter is preferred. We shipped these features to fix the bugs our users were experiencing with HOCs. When we shipped these features we should have publicized this work and updated the documentation. That’s on me.

The updated example in our docs which should be shipping soon is:

import * as React from 'react';

function injectProp<Props: {}>(
  Component: React.ComponentType<Props>,
): React.ComponentType<$Diff<Props, { foo: number | void }>> {
  return function WrapperComponent(props: Props) {
    return <Component {...props} foo={42} />;
  };
}

class MyComponent extends React.Component<{
  a: number,
  b: number,
  foo: number,
}> {}

const MyEnhancedComponent = injectProp(MyComponent);

// We don't need to pass in `foo` even though `MyComponent` requires it.
<MyEnhancedComponent a={1} b={2} />;

The best way to reach me is on Twitter. I’m happy to provide support there if you have any more questions 👍

We strongly believe that what we find useful at Facebook will also be useful for our community. We got a lot of internal reports for this bug as well. We hope that we can win you back as we continue to evolve the type-checker 😊

[yuchi]

@calebmer Looks like this is not the case for lib defs.

This is a minimally reproducible testcase with a fake framework, but it’s just a slimmer version of material-ui’s withStyle HOC decorator.

// flow-typed/my-framework.js
declare module 'my-framework/withStyle' {
  import type { ComponentType } from 'react';

  declare module.exports: <P: {}>(
    Component: ComponentType<P>
  ) => ComponentType<$Diff<P, { classes: any | void }>>;
}

// MyButton.js
// @flow

import React from 'react';
import withStyle from 'my-framework/withStyle';

type Props = { classes: any, label: string };

function MyButton(props: Props) {
  return <button className={props.classes.root}>{props.label}</button>;
}

export default withStyle(MyButton);

// consumer.js
// @flow
import React from 'react';
import MyButton from './MyButton';
console.log(<MyButton label="ciao" />);

Got this error:

Error: consumer.js:4
  4: console.log(<MyButton label="ciao" />);
                 ^^^^^^^^^^^^^^^^^^^^^^^^^ props. This type is incompatible with
  6:   ) => ComponentType<$Diff<P, { classes: any | void }>>;
                          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ empty. See lib: flow-typed/my-framework.js:6

[yuchi]

I think I found a nice clue. With everything on the same file you get the right error but on the wrong function call (it’s wrong AFAIK):

// @flow

import React from 'react';

export function withStyle<P: {}>(
  Component: React$ComponentType<P>
): React$ComponentType<$Diff<P, { classes: Object | void }>> {
  return function WrapperComponent(props: *): * {
    return <Component classes={{ root: 'root' }} {...props} />;
  };
}

type Props = { classes: Object, label: string };

function MyButton(props: Props): React$Node {
  return <button className={props.classes.root}>{props.label}</button>;
}

const StyledMyButton = withStyle(MyButton);
//                     ^^^^^^^^^ here!

console.log(<StyledMyButton />)

export default StyledMyButton;

[yuchi]

Also note those are only related to functional components, switching to classes “solves” the issue.

I believe this is related to #3521;

[steida]

This is the working solution https://github.com/thejameskyle/create-react-context

[yuchi]

@calebmer Is there any roadmap on these issues?

[yuchi]

@steida It could be from a library-developer point of view, but not when the third party library is not willing to use it. Anyway, very interesting, it could in fact simplify typing HOCs.

[k]

@steida would you mind elaborating how that fixes the separate file issue?

[steida]

@k Using render prop or function as children instead of HOC.

[steida]

I think I got it https://github.com/facebook/flow/issues/5908