Use SameValue instead of === to check for dispatchAction equivalence (#14752)
Jessica Franco committed
Feb 4, 2019 at 21:56 UTC
e602b5291cdde721429d4916e50dfa5c19c71707
2 files changed
+47
-3
packages/react-reconciler/src/ReactFiberHooks.js
+3
-3
@@ -612,7 +612,7 @@ function updateReducer<S, I, A>(
612
613
// Mark that the fiber performed work, but only if the new state is
614
// different from the current state.
615
- if (newState !== hook.memoizedState) {
615
+ if (!is(newState, hook.memoizedState)) {
616
markWorkInProgressReceivedUpdate();
617
}
618
@@ -695,7 +695,7 @@ function updateReducer<S, I, A>(
695
696
// Mark that the fiber performed work, but only if the new state is
697
// different from the current state.
698
- if (newState !== hook.memoizedState) {
698
+ if (!is(newState, hook.memoizedState)) {
699
markWorkInProgressReceivedUpdate();
700
}
701
@@ -1105,7 +1105,7 @@ function dispatchAction<S, A>(
1105
// without calling the reducer again.
1106
update.eagerReducer = eagerReducer;
1107
update.eagerState = eagerState;
1108
- if (eagerState === currentState) {
1108
+ if (is(eagerState, currentState)) {
1109
// Fast path. We can bail out without scheduling React to re-render.
1110
// It's still possible that we'll need to rebase this update later,
1111
// if the component re-renders for a different reason and by that
packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js
+44
@@ -114,6 +114,30 @@ describe('ReactHooks', () => {
114
// Because the final values are the same as the current values, the
115
// component bails out.
116
expect(root).toFlushAndYield(['Parent: 1, 2']);
117
+
118
+ // prepare to check SameValue
119
+ setCounter1(0 / -1);
120
+ setCounter2(NaN);
121
+ expect(root).toFlushAndYield([
122
+ 'Parent: 0, NaN',
123
+ 'Child: 0, NaN',
124
+ 'Effect: 0, NaN',
125
+ ]);
126
+
127
+ // check if re-setting to negative 0 / NaN still bails out
128
+ setCounter1(0 / -1);
129
+ setCounter2(NaN);
130
+ setCounter2(Infinity);
131
+ setCounter2(NaN);
132
+ expect(root).toFlushAndYield(['Parent: 0, NaN']);
133
+
134
+ // check if changing negative 0 to positive 0 does not bail out
135
+ setCounter1(0);
136
+ expect(root).toFlushAndYield([
137
+ 'Parent: 0, NaN',
138
+ 'Child: 0, NaN',
139
+ 'Effect: 0, NaN',
140
+ ]);
141
});
142
143
it('bails out in render phase if all the state is the same and props bail out with memo', () => {
@@ -375,6 +399,26 @@ describe('ReactHooks', () => {
399
setCounter(2);
400
expect(root).toFlushAndYield(['Parent: 2', 'Child: 2', 'Effect: 2']);
401
expect(root).toMatchRenderedOutput('2');
402
+
403
+ // prepare to check SameValue
404
+ setCounter(0);
405
+ expect(root).toFlushAndYield(['Parent: 0', 'Child: 0', 'Effect: 0']);
406
+ expect(root).toMatchRenderedOutput('0');
407
+
408
+ // Update to the same state for the first time to flush the queue
409
+ setCounter(0);
410
+ expect(root).toFlushAndYield(['Parent: 0']);
411
+ expect(root).toMatchRenderedOutput('0');
412
+
413
+ // Update again to the same state. Should bail out.
414
+ setCounter(0);
415
+ expect(root).toFlushAndYield([]);
416
+ expect(root).toMatchRenderedOutput('0');
417
+
418
+ // Update to a different state (positive 0 to negative 0)
419
+ setCounter(0 / -1);
420
+ expect(root).toFlushAndYield(['Parent: 0', 'Child: 0', 'Effect: 0']);
421
+ expect(root).toMatchRenderedOutput('0');
422
});
423
424
it('bails out multiple times in a row without entering render phase', () => {