-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Fields re-render every time due to regression introduced with flow typings. #3758
Conversation
… when determining if a Field should re-render
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we also have a test for this?
const propsEqual = isEqualWith(instance.props, nextProps, customizer) | ||
const stateEqual = isEqualWith(instance.state, nextState, customizer) | ||
|
||
return !propsEqual || !stateEqual |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like how readable this function is now 💯
@erikras is there any way to retry travis-ci? I dont really understand why it failed, since there seems to be no logs :) |
@gustavohenke I attempted for a while to write a test for it. But for some reason the actual component specified to the The test I wrote to check if is basically equal to https://github.com/erikras/redux-form/blob/878176b0e960d58a3277a2bd83703e8da3e82eb6/src/__tests__/Field.spec.js#L874 however I could not make the |
…ed multiple times if neither state nor props have changed
@gustavohenke I figured out a way :) Had to make a sub-class of the Field-class to check it though, so the test is kind of hacky. If you know of any other way of testing this please let me know :) |
Codecov Report
@@ Coverage Diff @@
## master #3758 +/- ##
======================================
Coverage 100% 100%
======================================
Files 70 70
Lines 1587 1589 +2
======================================
+ Hits 1587 1589 +2
Continue to review full report at Codecov.
|
Published in |
This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
This PR fixes a regression introduced in #3138 where the
shouldComponentUpdate
method ofcreateField.js
no longer sentnextState
as an argument toshallowCompare
when checking if a component should update. This regression effectively made every field re-render every time since the default state of a react component isnull
and an unspecified argument isundefined
, makingshallowCompare
do anull === undefined
check to determine if the component should update.