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

Run all component lifecycle methods when shallow rendering #4993

Merged
merged 1 commit into from
Nov 3, 2015

Conversation

jsdf
Copy link
Contributor

@jsdf jsdf commented Sep 28, 2015

Performs transaction around shallow render so remaining component lifecycle methods (componentDidMount, componentDidUpdate) are processed.

Fixes #4919

@sophiebits
Copy link
Collaborator

I think this looks good. Probably we'll wait until after 0.14 is released to merge this though.

@jankuca
Copy link

jankuca commented Nov 2, 2015

+1 This will be so great.

sophiebits added a commit that referenced this pull request Nov 3, 2015
Run all component lifecycle methods when shallow rendering
@sophiebits sophiebits merged commit d288879 into facebook:master Nov 3, 2015
@sophiebits
Copy link
Collaborator

Thanks!

sophiebits added a commit to sophiebits/react that referenced this pull request Nov 3, 2015
sophiebits added a commit that referenced this pull request Nov 3, 2015
chicoxyzzy pushed a commit to chicoxyzzy/react that referenced this pull request Nov 4, 2015
sophiebits added a commit to sophiebits/react that referenced this pull request Nov 5, 2015
We were shallow-rendering a component that used refs at FB so this can't go in as-is. It's a little unclear what we _should_ do though, since there is nothing to hold a ref to (since we're shallowly rendering) and we generally promise that child refs are resolved before a parent's componentDidMount. Also, changing shallow rendering to use the original `_renderValidatedComponent` (instead of `_renderValidatedComponentWithoutOwnerOrContext`) breaks tests because now the `_owner` field doesn't match up for `toEqual` (non-null in `getRenderOutput` but null if constructed in a test).
@sophiebits
Copy link
Collaborator

I'm going to have to revert this in #5394, sorry – more details there.

sophiebits added a commit to sophiebits/react that referenced this pull request Nov 5, 2015
We were shallow-rendering a component that used refs at FB so this can't go in as-is. It's a little unclear what we _should_ do though, since there is nothing to hold a ref to (since we're shallowly rendering) and we generally promise that child refs are resolved before a parent's componentDidMount. Also, changing shallow rendering to use the original `_renderValidatedComponent` (instead of `_renderValidatedComponentWithoutOwnerOrContext`) breaks tests because now the `_owner` field doesn't match up for `toEqual` (non-null in `getRenderOutput` but null if constructed in a test).
sophiebits added a commit that referenced this pull request Nov 5, 2015
Revert #4993 with an added test for refs
jankuca added a commit to jankuca/react-test-render that referenced this pull request Dec 11, 2015
`ReactShallowRenderer` does not call all lifecycle methods – namely
`componentDidMount` and `componentDidUpdate`. This patch fixes this
until real support lands in the official React codebase.

Addresses facebook/react#4993 “Run all component lifecycle methods when
shallow rendering”
@Ma27
Copy link

Ma27 commented Jan 2, 2016

any plan to re-implement such a feature into the shallow renderer?

@sophiebits
Copy link
Collaborator

As I mentioned in the linked pull request, it's a little unclear how shallow rendering should work with lifecycle methods, especially with refs, since there is nothing to hold a ref to (since we're shallowly rendering) and we usually promise that child refs are resolved before a parent's componentDidMount. If someone proposes a solution that doesn't have this problem I'm happy to entertain it.

@jsdf
Copy link
Contributor Author

jsdf commented Jan 2, 2016

I haven't had much time to work on this, but the ideas I had were

  • allow an object of ref values to be passed in to .render() allowing mock values to be injected. ES5 getters could even allow dynamic values, or
  • pass an option to .render (eg {lifecycle: true}) which would toggle the behaviour between that which supports refs and that which supports lifecycle methods

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.

6 participants