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

1862/fix setprops on cdu #2007

Merged
merged 1 commit into from Feb 6, 2019

Conversation

3 participants
@peanutenthusiast
Copy link
Contributor

peanutenthusiast commented Feb 6, 2019

Fixes #1862

@ljharb ljharb force-pushed the peanutenthusiast:1862/fix-setprops-on-cdu branch from 2c623e4 to 89df015 Feb 6, 2019

@ljharb

ljharb approved these changes Feb 6, 2019

Copy link
Member

ljharb left a comment

Thanks, this is great!

@ljharb ljharb added this to Lifecycles in React 16 Feb 6, 2019

@peanutenthusiast

This comment has been minimized.

Copy link
Contributor Author

peanutenthusiast commented Feb 6, 2019

No problem!!

@ljharb ljharb merged commit 89df015 into airbnb:master Feb 6, 2019

1 of 2 checks passed

continuous-integration/travis-ci/push The Travis CI build is in progress
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@willdurand

This comment has been minimized.

Copy link

willdurand commented Feb 18, 2019

We noticed a regression in our test suite (between Enzyme 3.8 and 3.9), likely due to this patch. Downgrading to Enzyme 3.8 makes our test suite pass again.

We have some code that changes a component's state in componentWillReceiveProps() and, depending on this new state, a function is called in componentDidUpdate(). This is not working anymore and I believe this patch is the "culprit".

This PR shows the regression as well as the revert to Enzyme 3.8 to get a green CI again: mozilla/addons-frontend#7603.

I am not sure what you or I could do though. Moving away from CWRP is on our roadmap and might "fix" this incompatibility, but upgrading from 3.8 to 3.9 should not break our test suite. I don't know what's the most "correct" behavior between no batch updates, the <=3.8 behavior and the new one introduced by this patch 🤷‍♂️

@willdurand willdurand referenced this pull request Feb 18, 2019

Merged

Lock file maintenance #7603

0 of 1 task complete
@ljharb

This comment has been minimized.

Copy link
Member

ljharb commented Feb 20, 2019

@willdurand thanks for the report, could you file a new issue and fill out the template?

(Note that whatever react does is going to be "correct"; so it seems like 3.9 either fixed a bug, or introduced one)

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