Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change renderProp API #1891

Merged
merged 1 commit into from Nov 8, 2018
Merged

Change renderProp API #1891

merged 1 commit into from Nov 8, 2018

Conversation

dferber90
Copy link
Contributor

This PR changes the renderProp API as described in #1863 (comment).

This is merely a suggestion at the moment and I'm happy to close it again if we settle on another approach or even keep the current one.

The motivation for "returning a function" is that the arguments to enzyme's renderProp won't get mixed with the arguments to the render prop function itself. This allows us to eventually add more options (like context) which we can apply to the newly created wrapper.

An example usage (formatted with Prettier) would look like this:

const wrapper = shallow(<App />)
  .find(Mouse)
  .renderProp("render")(10, 20)
  .find(Mouse)
  .renderProp("foo")(3, {
    long: {
      yeah: true
    }
  })
  .find(Bar)
  .renderProp("x")();

Sidenote

At this point, one might wonder what the advantage of wrapper.find(Foo).renderProp('render')() over wrapper.find(Foo).prop('render')() is, and that's a fair question.

When using renderProp, the render props' return value gets nested into a new wrapper which enables this convenient chaining shown above - whereas wrapper.find(Foo).prop('render')() simply returns the React element without an enzyme wrapper. More about this is hinted at in this article.

docs/api/ReactWrapper/renderProp.md Outdated Show resolved Hide resolved
docs/api/ShallowWrapper/renderProp.md Outdated Show resolved Hide resolved
packages/enzyme-test-suite/package.json Outdated Show resolved Hide resolved
packages/enzyme-test-suite/test/ReactWrapper-spec.jsx Outdated Show resolved Hide resolved
packages/enzyme-test-suite/test/ReactWrapper-spec.jsx Outdated Show resolved Hide resolved
packages/enzyme-test-suite/test/ShallowWrapper-spec.jsx Outdated Show resolved Hide resolved
packages/enzyme-test-suite/test/ShallowWrapper-spec.jsx Outdated Show resolved Hide resolved
 This is technically a breaking change, but since it‘s prior to release, it isn‘t.
@dferber90
Copy link
Contributor Author

👍 I amended the last commit with those fixes

Copy link
Member

@ljharb ljharb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

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

Successfully merging this pull request may close these issues.

None yet

2 participants