Recompose: Improve fromRenderProps

Created on 11 Jul 2018  Â·  4Comments  Â·  Source: acdlite/recompose

The new feature fromRenderProps looks really nice, but I think it would be even better if we could send props to the inner renderProp component like that:

const RenderPropComponent = ({ name, children }) => children({ text: `Hello ${name}!` });
const component = ({ text }) => <h1>{text}</h1>;

const EnhancedComponent = fromRenderProps(
  RenderPropComponent,
  // or:
  // ({ name, children }) => <RenderPropComponent name={name} children={children} />,
  ({ text }) => ({ text })
)(component);

<EnhancedComponent name={John} />
// Would render <h1>Hello John!</h1>

It would be as simple as adding ...ownerProps to fromRenderProps.js (L14):

  const FromRenderProps = ownerProps =>
    renderPropsFactory({
      ...ownerProps,
      [renderPropName]: (...props) =>
        baseFactory({ ...ownerProps, ...propsMapper(...props) }),
    })

I deal with a lot of "render prop" components, and I won't be able to use fromRenderProps with most of them until such a change has been made.

What do you think about that ? I will be more than happy to send a PR if needed !

Most helpful comment

Any update about this issue ? If anyone is ok with it I could easily submit a PR !

All 4 comments

Considering providing a more general solution, we can change the third argument of fromRenderProps from

renderPropsName :: String

to

renderPropsName :: String
| mapPropsToRenderPropsProps :: OwnerProps -> RenderPropsProps

As a result we can use it like

const EnhancedComponent = fromRenderProps(
  RenderPropComponent,
  ({ text }) => ({ text }),
  ({ name, children }) => ({ name, children }) // or just an identity function
)(component);

However, in my opinion

const EnhancedComponent = fromRenderProps(
  ({ name, children }) => <RenderPropComponent name={name} children={children} />,
  ({ text }) => ({ text })
)(component);

is good enough currently :)

Actually I really like your idea to change renderPropsName, but with a small tweak:

String | (OwnerProps, childrenFn) -> RenderPropsProps
or
String | (childrenFn, OnwerProps) -> RenderPropsProps

As it would be more explicit on which props are sent to the renderProp component, and we won't risk to overload an outter children prop. Btw the signature would recall lodash way of doing things (with the iteratee shorthands: https://lodash.com/docs/4.17.10#map).

This way my example would change to:

const RenderPropComponent = ({ name, children }) => children({ text: `Hello ${name}!` });
const component = ({ text }) => <h1>{text}</h1>;

const EnhancedComponent = fromRenderProps(
  RenderPropComponent,
  ({ text }) => ({ text }),
  ({ name }, children) => ({ name, children })
  // or:
  // (children, { name }) => ({ name, children })
)(component);

<EnhancedComponent name="John" />
// Would render <h1>Hello John!</h1>

Any update about this issue ? If anyone is ok with it I could easily submit a PR !

Was this page helpful?
0 / 5 - 0 ratings

Related issues

isubasti picture isubasti  Â·  3Comments

yellowfrogCN picture yellowfrogCN  Â·  3Comments

astanciu picture astanciu  Â·  3Comments

cdomigan picture cdomigan  Â·  4Comments

uriklar picture uriklar  Â·  4Comments