react-mdc / react-material-components-web

React wrapper of Google's Material Components Web
https://react-mdc.github.io/
178 stars 15 forks source link

[common] type checking failure on wrapping component #21

Closed Hardtack closed 7 years ago

Hardtack commented 7 years ago
<Elevation
  wrap={<Themed wrap={Cell} />}
  ...
  />
101:         wrap={<Themed wrap={Cell} />}
                   ^^^^^^^^^^^^^^^^^^^^^^ React element `Themed`
 38:   wrap: Wrappable<P>
             ^^^^^^^^^^^^ class type: type application of Component. This type is incompatible with. See: ../../../../node_modules/@react-mdc/base/lib/wrapper.js.flow:38
 42:   wrap: Wrappable<T>
             ^^^^^^^^^^^^ union: type application of polymorphic type: type `ReactComponent` | type application of Element. See: ../../../../node_modules/@react-mdc/base/lib/wrapper.js.flow:42
  Member 1:
   11: export type Wrappable<P> = ReactComponent<P> | React.Element<P>;
                                  ^^^^^^^^^^^^^^^^^ type application of polymorphic type: type `ReactComponent`. See: ../../../../node_modules/@react-mdc/base/lib/types.js.flow:11
  Error:
   38:   wrap: Wrappable<P>
               ^^^^^^^^^^^^ class type: type application of Component. This type is incompatible with. See: ../../../../node_modules/@react-mdc/base/lib/wrapper.js.flow:38
   11: export type Wrappable<P> = ReactComponent<P> | React.Element<P>;
                                  ^^^^^^^^^^^^^^^^^ union: type application of polymorphic type: type `ClassComponent` | type application of polymorphic type: type `FunctionComponent`. See: ../../../../node_modules/@react-mdc/base/lib/types.js.flow:11
    Member 1:
      8:   ClassComponent<*, P, *> |
           ^^^^^^^^^^^^^^^^^^^^^^^ type application of polymorphic type: type `ClassComponent`. See: ../../../../node_modules/@react-mdc/base/lib/types.js.flow:8
    Error:
    101:         wrap={<Themed wrap={Cell} />}
                       ^^^^^^^^^^^^^^^^^^^^^^ props of React element `Themed`. Could not decide which case to select
     45: export class PropWrapper<T, P: {}, S> extends Wrapper<DefaultProps<T>, Props<P>, S> {
                                                                                ^^^^^^^^ intersection type. See: ../../../../node_modules/@react-mdc/base/lib/wrapper.js.flow:45
      Case 1 may work:
       37: export type Props<P: {}> = {
                                      ^ object type. See: ../../../../node_modules/@react-mdc/base/lib/wrapper.js.flow:37
      But if it doesn't, case 2 looks promising too:
        8:   ClassComponent<*, P, *> |
             ^^^^^^^^^^^^^^^^^^^^^^^ type parameter `P` of class type: type application of Component. See: ../../../../node_modules/@react-mdc/base/lib/types.js.flow:8
      Please provide additional annotation(s) to determine whether case 1 works (or consider merging it with case 2):
      101:         wrap={<Themed wrap={Cell} />}
                                       ^^^^ identifier `Cell`
    Member 2:
      9:   FunctionComponent<P>;
           ^^^^^^^^^^^^^^^^^^^^ type application of polymorphic type: type `FunctionComponent`. See: ../../../../node_modules/@react-mdc/base/lib/types.js.flow:9
    Error:
      9:   FunctionComponent<P>;
           ^^^^^^^^^^^^^^^^^^^^ function type. Callable signature not found in. See: ../../../../node_modules/@react-mdc/base/lib/types.js.flow:9
     38:   wrap: Wrappable<P>
                 ^^^^^^^^^^^^ statics of React$Component. See: ../../../../node_modules/@react-mdc/base/lib/wrapper.js.flow:38
  Member 2:
   11: export type Wrappable<P> = ReactComponent<P> | React.Element<P>;
                                                      ^^^^^^^^^^^^^^^^ type application of Element. See: ../../../../node_modules/@react-mdc/base/lib/types.js.flow:11
  Error:
   38:   wrap: Wrappable<P>
               ^^^^^^^^^^^^ class type: type application of Component. This type is incompatible with. See: ../../../../node_modules/@react-mdc/base/lib/wrapper.js.flow:38
   11: export type Wrappable<P> = ReactComponent<P> | React.Element<P>;
                                                      ^^^^^^^^^^^^^^^^ React$Element. See: ../../../../node_modules/@react-mdc/base/lib/types.js.flow:11
Hardtack commented 7 years ago

This can be fixed by breaking change of wrapper API.

We wanted to provide customization capability of HTML element actually rendered. So I made PropWrapper.

type Wrappable<P> = ReactComponent<P> | React.Element<P>;
type Props<P: {}> = {
  wrap: Wrappable<P>
} & P;

It works with basic wrapping with fancy props API like this

<Themed wrap={<a />} href="/link" backgroundColor="accent">Themed Link</Themed>

But it's too complex and can cause unexpected property collisions.

I'm considering to change it like:

<Themed wrap={<a href="link" />} backgroundColor="accent">Themed Link</Themed>

Key point is Splitting @react-mdc component's props and wrapped component's props

Hardtack commented 7 years ago
<Themed wrap={<a href="link" />} backgroundColor="accent">Themed Link</Themed>

This style is too inconvenient. I'm trying to fix it with original API

Hardtack commented 7 years ago

Fixed