From 46bde584867b3eaebe917ef0c742badc1f84bcb6 Mon Sep 17 00:00:00 2001
From: Sebastian Markbage <sema@fb.com>
Date: Tue, 25 Oct 2016 15:21:05 -0700
Subject: [PATCH] Don't call componentDidUpdate if shouldComponentUpdate
 returns false

This fix relies on the props and state objects being different
to know if we can avoid a componentDidUpdate. This is not a great
solution because we actually want to use the new props/state
object even if sCU returns false. That's the current semantics
and it can actually be important because new rerenders that are
able to reuse props objects are more likely to have the new props
object so we won't be able to quickly bail out next time.
I don't have a better idea atm though.
---
 .../shared/fiber/ReactFiberCompleteWork.js    |  9 +-
 .../fiber/__tests__/ReactIncremental-test.js  | 99 +++++++++++++++++++
 2 files changed, 107 insertions(+), 1 deletion(-)

diff --git a/src/renderers/shared/fiber/ReactFiberCompleteWork.js b/src/renderers/shared/fiber/ReactFiberCompleteWork.js
index 4250f97bb085..590332d5ce19 100644
--- a/src/renderers/shared/fiber/ReactFiberCompleteWork.js
+++ b/src/renderers/shared/fiber/ReactFiberCompleteWork.js
@@ -129,7 +129,14 @@ module.exports = function<T, P, I, TI, C>(config : HostConfig<T, P, I, TI, C>) {
         // Transfer update queue to callbackList field so callbacks can be
         // called during commit phase.
         workInProgress.callbackList = workInProgress.updateQueue;
-        markUpdate(workInProgress);
+        if (current) {
+          if (current.memoizedProps !== workInProgress.memoizedProps ||
+              current.memoizedState !== workInProgress.memoizedState) {
+            markUpdate(workInProgress);
+          }
+        } else {
+          markUpdate(workInProgress);
+        }
         return null;
       case HostContainer:
         transferOutput(workInProgress.child, workInProgress);
diff --git a/src/renderers/shared/fiber/__tests__/ReactIncremental-test.js b/src/renderers/shared/fiber/__tests__/ReactIncremental-test.js
index 51a2ab70d2d3..21d3e2d4c588 100644
--- a/src/renderers/shared/fiber/__tests__/ReactIncremental-test.js
+++ b/src/renderers/shared/fiber/__tests__/ReactIncremental-test.js
@@ -1281,4 +1281,103 @@ describe('ReactIncremental', () => {
 
   });
 
+  it('skips will/DidUpdate when bailing unless an update was already in progress', () => {
+    var ops = [];
+
+    class LifeCycle extends React.Component {
+      componentWillMount() {
+        ops.push('componentWillMount');
+      }
+      componentDidMount() {
+        ops.push('componentDidMount');
+      }
+      componentWillReceiveProps(nextProps) {
+        ops.push('componentWillReceiveProps');
+      }
+      shouldComponentUpdate(nextProps) {
+        ops.push('shouldComponentUpdate');
+        // Bail
+        return this.props.x !== nextProps.x;
+      }
+      componentWillUpdate(nextProps) {
+        ops.push('componentWillUpdate');
+      }
+      componentDidUpdate(prevProps) {
+        ops.push('componentDidUpdate');
+      }
+      render() {
+        ops.push('render');
+        return <span />;
+      }
+    }
+
+    function Sibling() {
+      ops.push('render sibling');
+      return <span />;
+    }
+
+    function App(props) {
+      return [
+        <LifeCycle x={props.x} />,
+        <Sibling />,
+      ];
+    }
+
+    ReactNoop.render(<App x={0} />);
+    ReactNoop.flush();
+
+    expect(ops).toEqual([
+      'componentWillMount',
+      'render',
+      'render sibling',
+      'componentDidMount',
+    ]);
+
+    ops = [];
+
+    // Update to same props
+    ReactNoop.render(<App x={0} />);
+    ReactNoop.flush();
+
+    expect(ops).toEqual([
+      'componentWillReceiveProps',
+      'shouldComponentUpdate',
+      // no componentWillUpdate
+      // no render
+      'render sibling',
+      // no componentDidUpdate
+    ]);
+
+    ops = [];
+
+    // Begin updating to new props...
+    ReactNoop.render(<App x={1} />);
+    ReactNoop.flushDeferredPri(30);
+
+    expect(ops).toEqual([
+      'componentWillReceiveProps',
+      'shouldComponentUpdate',
+      'componentWillUpdate',
+      'render',
+      'render sibling',
+      // no componentDidUpdate yet
+    ]);
+
+    ops = [];
+
+    // ...but we'll interrupt it to rerender the same props.
+    ReactNoop.render(<App x={1} />);
+    ReactNoop.flush();
+
+    // We can bail out this time, but we must call componentDidUpdate.
+    expect(ops).toEqual([
+      'componentWillReceiveProps',
+      'shouldComponentUpdate',
+      // no componentWillUpdate
+      // no render
+      'render sibling',
+      'componentDidUpdate',
+    ]);
+  });
+
 });