[Shallow] Implement setState for Hooks and remount on type change #15120
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This worked in function components but was broken for classes. It incorrectly retained the old instance even if the type was different.
We only needed this because we didn't correctly reset based on type. Now we do so this can go away.
There was no particular reason it was set to element.type. We just wanted to check if something is a render phase update.
trueadm
approved these changes
Mar 15, 2019
gaearon
added a commit
to gaearon/react
that referenced
this pull request
Mar 22, 2019
…cebook#15120) * Throw away old shallow renderer state on type change This worked in function components but was broken for classes. It incorrectly retained the old instance even if the type was different. * Remove _previousComponentIdentity We only needed this because we didn't correctly reset based on type. Now we do so this can go away. * Use _reset when unmounting * Use arbitrary componentIdentity There was no particular reason it was set to element.type. We just wanted to check if something is a render phase update. * Support Hook state updates in shallow renderer
Closed
|
@gaearon does this include all hooks compatibility for Shallow Render. Can we take latest version to retest failed testcases in enzymejs/enzyme#2041 ? If I understood wrong, please suggest plan or reference if any |
|
Feel free to test it. If you see an issue please file it. |
4 tasks
This was referenced May 29, 2019
This was referenced Aug 12, 2019
This was referenced Oct 26, 2019
This was referenced Nov 3, 2019
NMinhNguyen
referenced
this pull request
in enzymejs/react-shallow-renderer
Jan 29, 2020
…5120) * Throw away old shallow renderer state on type change This worked in function components but was broken for classes. It incorrectly retained the old instance even if the type was different. * Remove _previousComponentIdentity We only needed this because we didn't correctly reset based on type. Now we do so this can go away. * Use _reset when unmounting * Use arbitrary componentIdentity There was no particular reason it was set to element.type. We just wanted to check if something is a render phase update. * Support Hook state updates in shallow renderer
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #14840.
Alternative to #14841. While reviewing it, I realized some of the complexity in how we implemented it is relates to an existing bug in the shallow renderer. In particularly, rendering twice with different element type kept reusing the old class instance. I fixed this in 50f2262. (Now it behaves consistently on unmount/mount and on rendering a different element type.) While this has been broken for a long while, the old behavior of ignoring the type change plainly doesn't make sense — so if it breaks any tests, it means those tests were testing the wrong thing.
This allowed me to remove a bunch of fields and simplify the logic to match shallow renderer assumptions. In the last commit (4972adb), I'm adding support for state updates to shallow renderer. I think it's a bit simpler than what we tried in #14841.
I checked these changes don't break Enzyme test suite.