From 2b3b7c64fdf80531e97201046205f9e872665934 Mon Sep 17 00:00:00 2001
From: Sebastian Markbage <sema@fb.com>
Date: Thu, 20 Oct 2016 20:08:58 -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 4250f97bb0856..590332d5ce196 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 467965a8e937b..3a78f15fad57d 100644
--- a/src/renderers/shared/fiber/__tests__/ReactIncremental-test.js
+++ b/src/renderers/shared/fiber/__tests__/ReactIncremental-test.js
@@ -1234,4 +1234,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 componenWillUpdate
+      // 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',
+    ]);
+  });
+
 });