PropTypes error

[brummelte]

Having render prop components with propTypes will result in console errors:
https://codesandbox.io/s/r1ywymx0kn

[jamesplease]

Hi @brummelte ! This doesn’t appear to be an issue with Composer. Maybe asking the question on StackOverflow is a better place to get help on this question. Best of luck!

[brummelte]

It is an issue with Composer, because the only allowed syntax for the components prop is using an empty component:

<Composer components={[<LanguageProvider />, <ThemeProvider />]}>

And the prop type validation then fails. The application works, just the prop types fail.

One way not to have the console error is wrapping the components, so that the prop types are only checked on render:

const wrapComponent = Component => ({ children, ...otherProps }) => (
  <Component {...otherProps}>{children}</Component>
);

const LanguageProvider2 = wrapComponent(LanguageProvider);
const ThemeProvider2 = wrapComponent(ThemeProvider);

<Composer components={[<LanguageProvider2 {...languageProps} />, <ThemeProvider2 {...themeProps} />]}>

That works, but is not really easy to write.

The following is better:

<Composer
  components={[
    <LanguageProvider {...languageProps}>{() => {}}</LanguageProvider>,
    <ThemeProvider {...themeProps} >{() => { }}</ThemeProvider>
  ]}
>

Adding a syntax like the following might be a possibility aswell:

<Composer
    components={[LanguageProvider, ThemeProvider]}
    props={[{
        ...languageProps
    }, {
        ...themeProps
    }]}
>

If the new syntax (3) is not going to be implemented, maybe adding a note to the readme for using 2 would be a good idea.

[jamesplease]

Ah, I see what you mean. I’ll add a note to the README, but I don’t think a new API should be introduced. @erikthedeveloper what do you think?

[erikthedeveloper]

Yeah I see what @brummelte is saying. I think it's fairly common to declare children/render propTypes as isRequired which would result in propType warnings squawking in the console.

As-is, the ideal solution would be to do the suggested option 2 above (#42 (comment)) knowing that props.children is going to be overwritten by Composer.

<Composer
  components={[
    <LanguageProvider {...languageProps} children={() => null} />
  ]}
  // ...
>

I will point out that the proposed change to the components as functions in #39 would enable this such as ⬇️ since the component would not be created/invoked with the missing children / render prop

<Composer
  components={[
    ({render}) => <LanguageProvider {...languageProps} children={render} />,
    ({render}) => <RequiresRender render={render} />
  ]}
  // ...
>

[jamesplease]

Thanks for the input, @erikthedeveloper !

I added a guide about this topic to the documentation over in #44 . I also opened a poll about whether or not we should make the breaking API described by #39 over in #43 .

[jamesplease]

Hi again @brummelte ! It is looking like people are liking the API over in #43 , so this issue will have a really good solution once we merge and deploy that change ( #39 ). I am currently planning to do that release this weekend.

Thank you for opening this issue! I hope you find this solution acceptable ✌️

[brummelte]

Yes, thanks. That change is great.

[jamesplease]

I’m glad you like it! It has been released as v5.0.0 ✌️