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

ReactShallowRenderer.render returns the rendered output #5411

Merged
merged 1 commit into from Nov 20, 2015

Conversation

Projects
None yet
7 participants
@simonewebdesign
Contributor

simonewebdesign commented Nov 6, 2015

I think would be nice to be able to chain functions this way:

let {props} = shallowRenderer.render(<MyComponent {...props} />).getRenderOutput();

This is not possible at the moment, as render() doesn't return itself, so I have to do it in two lines:

shallowRenderer.render(<MyComponent {...props} />);
let {props} = shallowRenderer.getRenderOutput();
@zpao

This comment has been minimized.

Show comment
Hide comment
@zpao

zpao Nov 6, 2015

Member

I don't think we have any chaining APIs at the moment and I'm not sure it's worth the inconsistency to add one now. cc @spicyj @sebmarkbage who have more knowledge on the shallow renderer

Member

zpao commented Nov 6, 2015

I don't think we have any chaining APIs at the moment and I'm not sure it's worth the inconsistency to add one now. cc @spicyj @sebmarkbage who have more knowledge on the shallow renderer

@sebmarkbage

This comment has been minimized.

Show comment
Hide comment
@sebmarkbage

sebmarkbage Nov 6, 2015

Member

Maybe render should just return the getRenderOutput?

cc @graue that might remember why we didn't.

Member

sebmarkbage commented Nov 6, 2015

Maybe render should just return the getRenderOutput?

cc @graue that might remember why we didn't.

@zpao

This comment has been minimized.

Show comment
Hide comment
@zpao

zpao Nov 6, 2015

Member

cc @sebmarkbage who merged #4918 which added getRenderOutput (with relatedish discussion in #4056)

Member

zpao commented Nov 6, 2015

cc @sebmarkbage who merged #4918 which added getRenderOutput (with relatedish discussion in #4056)

@sophiebits

This comment has been minimized.

Show comment
Hide comment
@sophiebits

sophiebits Nov 6, 2015

Member

cc @zpao who commented twice on this issue

Member

sophiebits commented Nov 6, 2015

cc @zpao who commented twice on this issue

@zpao

This comment has been minimized.

Show comment
Hide comment
@zpao

zpao Nov 7, 2015

Member

We have fun 😂 (sorry to distract from the main reason we're all here, which is to review this pull request)

Member

zpao commented Nov 7, 2015

We have fun 😂 (sorry to distract from the main reason we're all here, which is to review this pull request)

@sebmarkbage

This comment has been minimized.

Show comment
Hide comment
@sebmarkbage

sebmarkbage Nov 7, 2015

Member

cc @zpao who didn't realize that #4918 added getMountedInstance and not getRenderOutput which was added in #2497.

@simonewebdesign Would you prefer if it just returned getRenderOutput immediately since that seems more convenient for your use case and it's probably fairly common?

Member

sebmarkbage commented Nov 7, 2015

cc @zpao who didn't realize that #4918 added getMountedInstance and not getRenderOutput which was added in #2497.

@simonewebdesign Would you prefer if it just returned getRenderOutput immediately since that seems more convenient for your use case and it's probably fairly common?

@facebook-github-bot

This comment has been minimized.

Show comment
Hide comment
@facebook-github-bot

facebook-github-bot commented Nov 8, 2015

@simonewebdesign updated the pull request.

@simonewebdesign

This comment has been minimized.

Show comment
Hide comment
@simonewebdesign

simonewebdesign Nov 8, 2015

Contributor

@sebmarkbage I agree on that, the API would be more straightforward.

Contributor

simonewebdesign commented Nov 8, 2015

@sebmarkbage I agree on that, the API would be more straightforward.

@simonewebdesign simonewebdesign changed the title from ReactShallowRenderer.render returns itself to ReactShallowRenderer.render returns the rendered output Nov 8, 2015

@hartzis

This comment has been minimized.

Show comment
Hide comment
@hartzis

hartzis Nov 9, 2015

Contributor

Definitely like this change! @jsdf and @glenjamin either of you have concerns on this and how it could effect shallow rendering testing libraries?

Contributor

hartzis commented Nov 9, 2015

Definitely like this change! @jsdf and @glenjamin either of you have concerns on this and how it could effect shallow rendering testing libraries?

@glenjamin

This comment has been minimized.

Show comment
Hide comment
@glenjamin

glenjamin Nov 9, 2015

Contributor

Seems safe enough to me, as mentioned this should be the most common case.

Contributor

glenjamin commented Nov 9, 2015

Seems safe enough to me, as mentioned this should be the most common case.

sebmarkbage added a commit that referenced this pull request Nov 20, 2015

Merge pull request #5411 from simonewebdesign/master
ReactShallowRenderer.render returns the rendered output

@sebmarkbage sebmarkbage merged commit 64f795e into facebook:master Nov 20, 2015

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@zpao

This comment has been minimized.

Show comment
Hide comment
@zpao

zpao Nov 20, 2015

Member

Thanks!

Member

zpao commented Nov 20, 2015

Thanks!

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