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

Shallow renderer: support multiple setState invocation #11167

Merged
merged 1 commit into from
Oct 10, 2017
Merged

Shallow renderer: support multiple setState invocation #11167

merged 1 commit into from
Oct 10, 2017

Conversation

Hypnosphi
Copy link
Contributor

@Hypnosphi Hypnosphi commented Oct 9, 2017

Issue: #11161

This PR ensures that subsequent calls to setState:

  1. Use the result of previous setState as base state when shallow merging
  2. Pass it to updater function if any

@gaearon
Copy link
Collaborator

gaearon commented Oct 9, 2017

cc @bvaughn who I believe originally wrote it. Looks good to you?

Copy link
Contributor

@bvaughn bvaughn left a comment

Choose a reason for hiding this comment

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

It's been a while since I've thought about this component too closely 😁 but this change looks good to me. I appreciate the newly-added tests.

@gaearon
Copy link
Collaborator

gaearon commented Oct 9, 2017

@Hypnosphi Could you please also get a local clone of Enzyme running and verify that patching this up in react-test-utils package manually doesn't break any Enzyme unit tests?

@Hypnosphi
Copy link
Contributor Author

I did:

# in react directory
yarn build
cd build/packages/react-test-renderer
yarn link
cd ../../../.. # parent of react directory

git clone https://github.com/airbnb/enzyme.git
cd enzyme
yarn
yarn react:16
yarn link react-test-renderer
yarn test:only

And it passes

@gaearon gaearon merged commit 2264e3d into facebook:master Oct 10, 2017
@gaearon
Copy link
Collaborator

gaearon commented Oct 10, 2017

Sweet. Thanks.

@Hypnosphi Hypnosphi deleted the fix-shallow-setstate branch October 10, 2017 11:48
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.

4 participants