Update input pointers even when shouldComponentUpdate returns false

Because memoizedProps and memoizedState are read from the instance, the
instance's input pointers (props, state, context) should be updated even
when shouldComponentUpdate causes a bail-out.

Author: acdlite

scripts/fiber/tests-failing.txt

@@ -1,6 +1,3 @@
-src/addons/__tests__/ReactComponentWithPureRenderMixin-test.js
-* does not do a deep comparison
-
 src/addons/__tests__/ReactFragment-test.js
 * should throw if a plain object is used as a child
 * should throw if a plain object even if it is in an owner

scripts/fiber/tests-passing.txt

@@ -36,6 +36,7 @@ scripts/error-codes/__tests__/invertObject-test.js
 
 src/addons/__tests__/ReactComponentWithPureRenderMixin-test.js
 * provides a default shouldComponentUpdate implementation
+* does not do a deep comparison
 
 src/addons/__tests__/ReactFragment-test.js
 * warns for numeric keys on objects as children
@@ -1138,6 +1139,7 @@ src/renderers/shared/fiber/__tests__/ReactIncremental-test.js
 * can resume work in a bailed subtree within one pass
 * can reuse work done after being preempted
 * can reuse work if shouldComponentUpdate is false, after being preempted
+* memoizes work even if shouldComponentUpdate returns false
 * can update in the middle of a tree using setState
 * can queue multiple state updates
 * can use updater form of setState

src/renderers/shared/fiber/ReactFiberBeginWork.js

@@ -251,9 +251,8 @@ module.exports = function<T, P, I, TI, C, CX>(
       // If an update was already in progress, we should schedule an Update
       // effect even though we're bailing out, so that cWU/cDU are called.
       if (current) {
-        const instance = current.stateNode;
-        if (instance.props !== current.memoizedProps ||
-            instance.state !== current.memoizedState) {
+        if (workInProgress.memoizedProps !== current.memoizedProps ||
+            workInProgress.memoizedState !== current.memoizedState) {
           workInProgress.effectTag |= Update;
         }
       }

src/renderers/shared/fiber/ReactFiberClassComponent.js

@@ -303,6 +303,12 @@ module.exports = function(
       newState,
       newContext
     )) {
+      // Update the existing instance's state, props, and context pointers even
+      // though we're bailing out.
+      const instance = workInProgress.stateNode;
+      instance.props = newProps;
+      instance.state = newState;
+      instance.context = newContext;
       return false;
     }
 
@@ -385,25 +391,27 @@ module.exports = function(
       return false;
     }
 
-    if (!checkShouldComponentUpdate(
+    const shouldUpdate = checkShouldComponentUpdate(
       workInProgress,
       oldProps,
       newProps,
       newState,
       newContext
-    )) {
-      // TODO: Should this get the new props/state updated regardless?
-      return false;
-    }
+    );
 
-    if (typeof instance.componentWillUpdate === 'function') {
+    if (shouldUpdate && typeof instance.componentWillUpdate === 'function') {
       instance.componentWillUpdate(newProps, newState, newContext);
     }
 
+    // Update the existing instance's state, props, and context pointers even
+    // if shouldComponentUpdate returns false. memoizedProps and memoizedState
+    // are read from the instance, so by always updating them, we ensure that
+    // work is memoized even when we bail out.
     instance.props = newProps;
     instance.state = newState;
     instance.context = newContext;
-    return true;
+
+    return shouldUpdate;
   }
 
   return {

src/renderers/shared/fiber/__tests__/ReactIncremental-test.js

@@ -628,6 +628,43 @@ describe('ReactIncremental', () => {
 
   });
 
+  it('memoizes work even if shouldComponentUpdate returns false', () => {
+    let ops = [];
+    class Foo extends React.Component {
+      shouldComponentUpdate(nextProps) {
+        // this.props is the memoized props. So this should return true for
+        // every update except the first one.
+        const shouldUpdate = this.props.step !== 1;
+        ops.push('shouldComponentUpdate: ' + shouldUpdate);
+        return shouldUpdate;
+      }
+      render() {
+        ops.push('render');
+        return <div />;
+      }
+    }
+
+    ReactNoop.render(<Foo step={1} />);
+    ReactNoop.flush();
+
+    ops = [];
+    ReactNoop.render(<Foo step={2} />);
+    ReactNoop.flush();
+    expect(ops).toEqual([
+      'shouldComponentUpdate: false',
+    ]);
+
+    ops = [];
+    ReactNoop.render(<Foo step={3} />);
+    ReactNoop.flush();
+    expect(ops).toEqual([
+      // If the memoized props were not updated during last bail out, sCU will
+      // keep returning false.
+      'shouldComponentUpdate: true',
+      'render',
+    ]);
+  });
+
   it('can update in the middle of a tree using setState', () => {
     let instance;
     class Bar extends React.Component {