From f17b273939b984c5d858522d697bf7d13d1ce43a Mon Sep 17 00:00:00 2001
From: Andrew Clark <acdlite@fb.com>
Date: Tue, 3 Apr 2018 11:50:47 -0700
Subject: [PATCH] getDerivedStateFrom{Props,Catch} should update
 updateQueue.baseState

Based on a bug found in UFI2.

There have been several bugs related to the update queue (and
specifically baseState) recently, so I'm going to follow-up with some
refactoring to clean it up. This is a quick fix so we can ship a
patch release.
---
 .../src/ReactFiberClassComponent.js           | 44 +++++++++++++++++
 .../ReactIncrementalUpdates-test.internal.js  | 48 +++++++++++++++++++
 2 files changed, 92 insertions(+)

diff --git a/packages/react-reconciler/src/ReactFiberClassComponent.js b/packages/react-reconciler/src/ReactFiberClassComponent.js
index 656ae9a055f0b..679c791bd5da2 100644
--- a/packages/react-reconciler/src/ReactFiberClassComponent.js
+++ b/packages/react-reconciler/src/ReactFiberClassComponent.js
@@ -839,6 +839,17 @@ export default function(
         newState === null || newState === undefined
           ? derivedStateFromProps
           : Object.assign({}, newState, derivedStateFromProps);
+
+      // Update the base state of the update queue.
+      // FIXME: This is getting ridiculous. Refactor plz!
+      const updateQueue = workInProgress.updateQueue;
+      if (updateQueue !== null) {
+        updateQueue.baseState = Object.assign(
+          {},
+          updateQueue.baseState,
+          derivedStateFromProps,
+        );
+      }
     }
     if (derivedStateFromCatch !== null && derivedStateFromCatch !== undefined) {
       // Render-phase updates (like this) should not be added to the update queue,
@@ -848,6 +859,17 @@ export default function(
         newState === null || newState === undefined
           ? derivedStateFromCatch
           : Object.assign({}, newState, derivedStateFromCatch);
+
+      // Update the base state of the update queue.
+      // FIXME: This is getting ridiculous. Refactor plz!
+      const updateQueue = workInProgress.updateQueue;
+      if (updateQueue !== null) {
+        updateQueue.baseState = Object.assign(
+          {},
+          updateQueue.baseState,
+          derivedStateFromCatch,
+        );
+      }
     }
 
     if (
@@ -1016,6 +1038,17 @@ export default function(
         newState === null || newState === undefined
           ? derivedStateFromProps
           : Object.assign({}, newState, derivedStateFromProps);
+
+      // Update the base state of the update queue.
+      // FIXME: This is getting ridiculous. Refactor plz!
+      const updateQueue = workInProgress.updateQueue;
+      if (updateQueue !== null) {
+        updateQueue.baseState = Object.assign(
+          {},
+          updateQueue.baseState,
+          derivedStateFromProps,
+        );
+      }
     }
     if (derivedStateFromCatch !== null && derivedStateFromCatch !== undefined) {
       // Render-phase updates (like this) should not be added to the update queue,
@@ -1025,6 +1058,17 @@ export default function(
         newState === null || newState === undefined
           ? derivedStateFromCatch
           : Object.assign({}, newState, derivedStateFromCatch);
+
+      // Update the base state of the update queue.
+      // FIXME: This is getting ridiculous. Refactor plz!
+      const updateQueue = workInProgress.updateQueue;
+      if (updateQueue !== null) {
+        updateQueue.baseState = Object.assign(
+          {},
+          updateQueue.baseState,
+          derivedStateFromCatch,
+        );
+      }
     }
 
     if (
diff --git a/packages/react-reconciler/src/__tests__/ReactIncrementalUpdates-test.internal.js b/packages/react-reconciler/src/__tests__/ReactIncrementalUpdates-test.internal.js
index 2396e658aa95b..c7d0cca072140 100644
--- a/packages/react-reconciler/src/__tests__/ReactIncrementalUpdates-test.internal.js
+++ b/packages/react-reconciler/src/__tests__/ReactIncrementalUpdates-test.internal.js
@@ -373,4 +373,52 @@ describe('ReactIncrementalUpdates', () => {
     });
     ReactNoop.flush();
   });
+
+  it('getDerivedStateFromProps should update base state of updateQueue (based on product bug)', () => {
+    // Based on real-world bug.
+
+    let foo;
+    class Foo extends React.Component {
+      state = {value: 'initial state'};
+      static getDerivedStateFromProps() {
+        return {value: 'derived state'};
+      }
+      render() {
+        foo = this;
+        return (
+          <React.Fragment>
+            <span prop={this.state.value} />
+            <Bar />
+          </React.Fragment>
+        );
+      }
+    }
+
+    let bar;
+    class Bar extends React.Component {
+      render() {
+        bar = this;
+        return null;
+      }
+    }
+
+    ReactNoop.flushSync(() => {
+      ReactNoop.render(<Foo />);
+    });
+    expect(ReactNoop.getChildren()).toEqual([span('derived state')]);
+
+    ReactNoop.flushSync(() => {
+      // Triggers getDerivedStateFromProps again
+      ReactNoop.render(<Foo />);
+      // The noop callback is needed to trigger the specific internal path that
+      // led to this bug. Removing it causes it to "accidentally" work.
+      foo.setState({value: 'update state'}, function noop() {});
+    });
+    expect(ReactNoop.getChildren()).toEqual([span('derived state')]);
+
+    ReactNoop.flushSync(() => {
+      bar.setState({});
+    });
+    expect(ReactNoop.getChildren()).toEqual([span('derived state')]);
+  });
 });