From e602b5291cdde721429d4916e50dfa5c19c71707 Mon Sep 17 00:00:00 2001 From: Jessica Franco Date: Mon, 4 Feb 2019 21:56:21 +0900 Subject: [PATCH] Use SameValue instead of === to check for dispatchAction equivalence (#14752) --- .../react-reconciler/src/ReactFiberHooks.js | 6 +-- .../src/__tests__/ReactHooks-test.internal.js | 44 +++++++++++++++++++ 2 files changed, 47 insertions(+), 3 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberHooks.js b/packages/react-reconciler/src/ReactFiberHooks.js index 1a0655807f1e4..fef20fd1ec07c 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.js +++ b/packages/react-reconciler/src/ReactFiberHooks.js @@ -612,7 +612,7 @@ function updateReducer( // Mark that the fiber performed work, but only if the new state is // different from the current state. - if (newState !== hook.memoizedState) { + if (!is(newState, hook.memoizedState)) { markWorkInProgressReceivedUpdate(); } @@ -695,7 +695,7 @@ function updateReducer( // Mark that the fiber performed work, but only if the new state is // different from the current state. - if (newState !== hook.memoizedState) { + if (!is(newState, hook.memoizedState)) { markWorkInProgressReceivedUpdate(); } @@ -1105,7 +1105,7 @@ function dispatchAction( // without calling the reducer again. update.eagerReducer = eagerReducer; update.eagerState = eagerState; - if (eagerState === currentState) { + if (is(eagerState, currentState)) { // Fast path. We can bail out without scheduling React to re-render. // It's still possible that we'll need to rebase this update later, // if the component re-renders for a different reason and by that diff --git a/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js b/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js index 5b1477a20572f..6a8924e668a3d 100644 --- a/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js @@ -114,6 +114,30 @@ describe('ReactHooks', () => { // Because the final values are the same as the current values, the // component bails out. expect(root).toFlushAndYield(['Parent: 1, 2']); + + // prepare to check SameValue + setCounter1(0 / -1); + setCounter2(NaN); + expect(root).toFlushAndYield([ + 'Parent: 0, NaN', + 'Child: 0, NaN', + 'Effect: 0, NaN', + ]); + + // check if re-setting to negative 0 / NaN still bails out + setCounter1(0 / -1); + setCounter2(NaN); + setCounter2(Infinity); + setCounter2(NaN); + expect(root).toFlushAndYield(['Parent: 0, NaN']); + + // check if changing negative 0 to positive 0 does not bail out + setCounter1(0); + expect(root).toFlushAndYield([ + 'Parent: 0, NaN', + 'Child: 0, NaN', + 'Effect: 0, NaN', + ]); }); 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', () => { setCounter(2); expect(root).toFlushAndYield(['Parent: 2', 'Child: 2', 'Effect: 2']); expect(root).toMatchRenderedOutput('2'); + + // prepare to check SameValue + setCounter(0); + expect(root).toFlushAndYield(['Parent: 0', 'Child: 0', 'Effect: 0']); + expect(root).toMatchRenderedOutput('0'); + + // Update to the same state for the first time to flush the queue + setCounter(0); + expect(root).toFlushAndYield(['Parent: 0']); + expect(root).toMatchRenderedOutput('0'); + + // Update again to the same state. Should bail out. + setCounter(0); + expect(root).toFlushAndYield([]); + expect(root).toMatchRenderedOutput('0'); + + // Update to a different state (positive 0 to negative 0) + setCounter(0 / -1); + expect(root).toFlushAndYield(['Parent: 0', 'Child: 0', 'Effect: 0']); + expect(root).toMatchRenderedOutput('0'); }); it('bails out multiple times in a row without entering render phase', () => {