react-flow-types icon indicating copy to clipboard operation
react-flow-types copied to clipboard

Issue combining different High order components

Open zachwolf opened this issue 7 years ago • 7 comments

I'm having problems combining react-router-dom and material-ui. I was pointed in this direction from a flow-typed issue.

I've got a component that looks something like:

// @flow

import * as React from 'react'
import { withRouter } from 'react-router-dom'
import { withStyles } from 'material-ui/styles'

type Props = {
  content: string,
  classes: {
    [string]: string,
  },
}

class Button extends React.Component<Props> {
  render () {
    return (
      <button className={this.props.classes.main}>
        {this.props.content}
      </button>
    )
  }
}

export default withStyles({
  main: {
    background: 'red',
  }
})(withRouter(Button))

Which gives me:

$ flow
Launching Flow server for /Users/flow-react-router-dom-test
Spawned flow server (pid=79843)
Logs will go to /private/tmp/flow/XXXXXXXflow-react-router-dom-test.log
Library type error:
flow-typed/npm/react-router-dom_v4.x.x.js:144
144:     Component: React$ComponentType<{| ...ContextRouter, ...P |}>
                                        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ object type. This type is incompatible with
 14: class Button extends React.Component<Props> {
                                          ^^^^^ object type. See: src/Button.js:14
  Property `classes` is incompatible:
     19:           <Button content="click me" />
                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ undefined. This type is incompatible with. See: src/App.js:19
                    v
      9:   classes: {
     10:     [string]: string,
     11:   },
           ^ object type. See: src/Button.js:9

Library type error:
flow-typed/npm/react-router-dom_v4.x.x.js:144
144:     Component: React$ComponentType<{| ...ContextRouter, ...P |}>
                                        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ object type. This type is incompatible with
 14: class Button extends React.Component<Props> {
                                          ^^^^^ object type. See: src/Button.js:14
  Property `content` is incompatible:
     19:           <Button content="click me" />
                                   ^^^^^^^^^^ undefined. This type is incompatible with. See: src/App.js:19
      8:   content: string,
                    ^^^^^^ string. See: src/Button.js:8

Error: src/App.js:19
 19:           <Button content="click me" />
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ React element `Button`
 19:           <Button content="click me" />
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ props of React element `Button`. Inexact type is incompatible with exact type
144:     Component: React$ComponentType<{| ...ContextRouter, ...P |}>
                                        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ object type. See lib: flow-typed/npm/react-router-dom_v4.x.x.js:144

Under the covers, withStyles looks like:

const withStyles = (
  stylesOrCreator: Object,
  options?: Options = {},
): HigherOrderComponent<RequiredProps, InjectedProps> => (Component: any): any => {

If you want to check out a complete example, I've created a reduced example.

Is it possible to fix this issue in my code, or would it require a change to HigherOrderComponent?

Thanks!

zachwolf avatar Oct 17 '17 15:10 zachwolf

I'm guessing the problem comes from the fact that HigherOrderComponent uses intersection types, which don't mix very well with exact types. (| ...ContextRouter, ...P | being an exact type)

But if I flip HigherOrderComponent to use spread types, which should work well with exact types, then it would break non-exact types 😄

Any ideas?

AriaMinaei avatar Oct 17 '17 19:10 AriaMinaei

@AriaMinaei What scenarios are you worried about breaking inexact object types in?

ajhyndman avatar Oct 18 '17 04:10 ajhyndman

@ajhyndman

// @flow
type A = {|a: string|}
type B = {b: string} // << flipping this to an exact type fixes the error

type C = {|...A, ...B|}

function expectC(c: C): string {
   return c.b
}

Try Flow

AriaMinaei avatar Oct 18 '17 07:10 AriaMinaei

Huh, wow. You're right, that's an issue!

That feels like a bug in flow, itself. Maybe related to this one? https://github.com/facebook/flow/issues/2405

Intuitively, I would have expected an inexact object type when spread into an exact object type to produce a new, inexact object type.

ajhyndman avatar Oct 18 '17 11:10 ajhyndman

In the meantime, some possible workarounds.

Try Flow: Make everything inexact

I can't reason about why Flow thinks property b has become optional in this first scenario, though.

Try Flow: Make everything exact

This works more predictably, but seems to prevent you from wrapping a component that wants to permit arbitrary props with an HoC.

Try Flow: Why does this error?

I wrote up some of my findings on the aforementioned flow issue, in case it helps us all move past this.

https://github.com/facebook/flow/issues/2405#issuecomment-337568234

ajhyndman avatar Oct 18 '17 12:10 ajhyndman

Just noting here that after #16 we still have trouble with exact types, and as @ajhyndman demonstrated here, there isn't much we can do about that atm.

AriaMinaei avatar Oct 28 '17 17:10 AriaMinaei

I wrote a wrapper Hack.js component that glues the two together as a solution for the time being ¯\_(ツ)_/¯

zachwolf avatar Oct 30 '17 19:10 zachwolf