Skip to content
This repository has been archived by the owner on Sep 10, 2022. It is now read-only.

feat: add fromRenderProps [BREAKING CHANGES] #677

Merged
merged 4 commits into from
Jun 21, 2018

Conversation

evenchange4
Copy link
Contributor

@evenchange4 evenchange4 commented May 24, 2018

This PR is a proposal for issue #609.
In my opinion, the original withRenderProps HOC should rename to toRenderProps.

  1. HOC → RenderPropsComponent: withRenderProps toRenderProps
  2. HOC ← RenderPropsComponent: withConsumerProps withRenderProps fromRenderProps

UPDATE:

  • rename original withRenderProps to toRenderProps [BREAKING CHANGES]
  • add withConsumerProps fromRenderProps HOC:

Type signature:

fromRenderProps(
  RenderPropsComponent: ReactClass | ReactFunctionalComponent,
  propsMapper: (props: Object) => Object,
  renderPropName?: string
): HigherOrderComponent

Example:

import { fromRenderProps } from 'recompose';
const { Consumer: ThemeConsumer } = React.createContext({ theme: 'dark' });
const { Consumer: I18NConsumer } = React.createContext({ i18n: 'en' });
const RenderPropsComponent = ({ render, value }) => render({ value: 1 });
 
const EnhancedApp = compose(
  // Context (Function as Child Components)
  fromRenderProps(ThemeConsumer, ({ theme }) => ({ theme })),
  fromRenderProps(I18NConsumer, ({ i18n }) => ({ locale: i18n })),
  // Render props
  fromRenderProps(RenderPropsComponent, ({ value }) => ({ value }), 'render'),
)(App);
 
// Same as
const EnhancedApp = () => (
  <ThemeConsumer>
    {({ theme }) => (
      <I18NConsumer>
        {({ i18n }) => (
          <RenderPropsComponent
            render={({ value }) => (
              <App theme={theme} locale={i18n} value={value} />
            )}
          />
        )}
      </I18NConsumer>
    )}
  </ThemeConsumer>
)

@wuct
Copy link
Contributor

wuct commented May 24, 2018

Haha, we almost comment at the same time.

I agree with you about the naming.

@wuct
Copy link
Contributor

wuct commented May 24, 2018

Except that IMO fromRenderProps is better :)

@istarkov
Copy link
Contributor

to and from looks good

@istarkov
Copy link
Contributor

also what the reason in render prop name? any real example please when just children will be not suitable

@evenchange4
Copy link
Contributor Author

Personally, I only use children function for render props component.

@istarkov
Copy link
Contributor

Seems good for me except name.

@evenchange4
Copy link
Contributor Author

evenchange4 commented May 24, 2018

If this PR could be accepted, I can update those changes:

  1. rename original withRenderProps to toRenderProps [BREAKING CHANGES]
  2. add withConsumerProps fromRenderProps HOC without third parameter:
fromRenderProps(
  Component: ReactClass | ReactFunctionalComponent,
  propsMapper: (props: Object) => Object,
- renderPropName?: string
): HigherOrderComponent

What do you think?

cc @pomber

@istarkov
Copy link
Contributor

istarkov commented May 24, 2018

renderPropName is ok.

original withRenderProps to (to)RenderProps
this PR fromRenderProps

@evenchange4 evenchange4 changed the title RFC: add withConsumerProps WIP: add fromRenderProps [BREAKING CHANGES] May 24, 2018
@evenchange4 evenchange4 force-pushed the feature/render-props-hoc branch from 55699a8 to 8003c26 Compare May 24, 2018 13:06
@evenchange4 evenchange4 force-pushed the feature/render-props-hoc branch from 8003c26 to 633efc0 Compare May 24, 2018 13:25
@evenchange4 evenchange4 changed the title WIP: add fromRenderProps [BREAKING CHANGES] feat: add fromRenderProps [BREAKING CHANGES] May 24, 2018
@evenchange4
Copy link
Contributor Author

ready for review :)

@istarkov
Copy link
Contributor

Super! All is fine. LGTM.
@wuct ?

@xzyfer
Copy link

xzyfer commented Jun 7, 2018

This is great 👌

@wuct
Copy link
Contributor

wuct commented Jun 21, 2018

Great work, thanks!

@wuct wuct merged commit 38d637b into acdlite:master Jun 21, 2018
@wuct
Copy link
Contributor

wuct commented Jun 21, 2018

@istarkov so this one will land in 0.28.0, right?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants