Fixed a batched-state update bug with getDerivedStateFromProps

packages/react-dom/src/__tests__/ReactComponentLifeCycle-test.js

@@ -830,4 +830,49 @@ describe('ReactComponentLifeCycle', () => {
       'UNSAFE_componentWillUpdate',
     ]);
   });
+
+  it('should not override state with stale values if prevState is spread within getDerivedStateFromProps', () => {
+    let childInstance;
+
+    class Child extends React.Component {
+      state = {local: 0};
+      static getDerivedStateFromProps(nextProps, prevState) {
+        return {...prevState, remote: nextProps.remote};
+      }
+      updateState = () => {
+        this.setState(state => ({local: state.local + 1}));
+        this.props.onChange(this.state.remote + 1);
+      };
+      render() {
+        childInstance = this;
+        return (
+          <div onClick={this.updateState}>{`remote:${
+            this.state.remote
+          }, local:${this.state.local}`}</div>
+        );
+      }
+    }
+
+    class Parent extends React.Component {
+      state = {value: 0};
+      handleChange = value => {
+        this.setState({value});
+      };
+      render() {
+        return <Child remote={this.state.value} onChange={this.handleChange} />;
+      }
+    }
+
+    const instance = ReactTestUtils.renderIntoDocument(<Parent />);
+    const element = ReactDOM.findDOMNode(instance);
+    expect(element.textContent).toBe('remote:0, local:0');
+
+    // Trigger setState() calls
+    childInstance.updateState();
+    expect(element.textContent).toBe('remote:1, local:1');
+
+    // Trigger batched setState() calls
+    ReactTestUtils.Simulate.click(element);
+    expect(element.textContent).toBe('remote:2, local:2');
+  });
 });

packages/react-reconciler/src/ReactFiberBeginWork.js

@@ -612,6 +612,7 @@ export default function<T, P, I, TI, HI, PI, C, CC, CX, PL>(
           workInProgress,
           value,
           props,
+          workInProgress.memoizedState,
         );
 
         if (partialState !== null && partialState !== undefined) {

packages/react-reconciler/src/ReactFiberClassComponent.js

@@ -448,6 +448,7 @@ export default function(
       workInProgress,
       instance,
       props,
+      state,
     );
 
     if (partialState !== null && partialState !== undefined) {
@@ -534,7 +535,8 @@ export default function(
   function callGetDerivedStateFromProps(
     workInProgress: Fiber,
     instance: any,
-    props: any,
+    nextProps: any,
+    prevState: any,
   ) {
     const {type} = workInProgress;
 
@@ -567,17 +569,13 @@ export default function(
           workInProgress.mode & StrictMode)
       ) {
         // Invoke method an extra time to help detect side-effects.
-        type.getDerivedStateFromProps.call(
-          null,
-          props,
-          workInProgress.memoizedState,
-        );
+        type.getDerivedStateFromProps.call(null, nextProps, prevState);
       }
 
       const partialState = type.getDerivedStateFromProps.call(
         null,
-        props,
-        workInProgress.memoizedState,
+        nextProps,
+        prevState,
       );
 
       if (__DEV__) {
@@ -698,19 +696,21 @@ export default function(
       }
     }
 
+    // Compute the next state using the memoized state and the update queue.
+    const oldState = workInProgress.memoizedState;
+    // TODO: Previous state can be null.
+    let newState;
+
     let derivedStateFromProps;
     if (oldProps !== newProps) {
       derivedStateFromProps = callGetDerivedStateFromProps(
         workInProgress,
         instance,
         newProps,
+        oldState,
       );
     }
 
-    // Compute the next state using the memoized state and the update queue.
-    const oldState = workInProgress.memoizedState;
-    // TODO: Previous state can be null.
-    let newState;
     let derivedStateFromCatch;
     if (workInProgress.updateQueue !== null) {
       newState = processUpdateQueue(
@@ -867,19 +867,11 @@ export default function(
       }
     }
 
-    let derivedStateFromProps;
-    if (oldProps !== newProps) {
-      derivedStateFromProps = callGetDerivedStateFromProps(
-        workInProgress,
-        instance,
-        newProps,
-      );
-    }
-
     // Compute the next state using the memoized state and the update queue.
     const oldState = workInProgress.memoizedState;
     // TODO: Previous state can be null.
-    let newState;
+    let newState = oldState;
+
     let derivedStateFromCatch;
     if (workInProgress.updateQueue !== null) {
       newState = processUpdateQueue(
@@ -908,27 +900,38 @@ export default function(
           capturedValues,
         );
       }
-    } else {
-      newState = oldState;
     }
 
-    if (derivedStateFromProps !== null && derivedStateFromProps !== undefined) {
+    if (derivedStateFromCatch !== null && derivedStateFromCatch !== undefined) {
       // Render-phase updates (like this) should not be added to the update queue,
       // So that multiple render passes do not enqueue multiple updates.
       // Instead, just synchronously merge the returned state into the instance.
       newState =
         newState === null || newState === undefined
-          ? derivedStateFromProps
-          : Object.assign({}, newState, derivedStateFromProps);
+          ? derivedStateFromCatch
+          : Object.assign({}, newState, derivedStateFromCatch);
     }
-    if (derivedStateFromCatch !== null && derivedStateFromCatch !== undefined) {
+
+    let derivedStateFromProps;
+    if (oldProps !== newProps) {
+      // In this case, the prevState parameter should be the partially updated state,
+      // Otherwise, spreading state in return values could override updates.
+      derivedStateFromProps = callGetDerivedStateFromProps(
+        workInProgress,
+        instance,
+        newProps,
+        newState,
+      );
+    }
+
+    if (derivedStateFromProps !== null && derivedStateFromProps !== undefined) {
       // Render-phase updates (like this) should not be added to the update queue,
       // So that multiple render passes do not enqueue multiple updates.
       // Instead, just synchronously merge the returned state into the instance.
       newState =
         newState === null || newState === undefined
-          ? derivedStateFromCatch
-          : Object.assign({}, newState, derivedStateFromCatch);
+          ? derivedStateFromProps
+          : Object.assign({}, newState, derivedStateFromProps);
     }
 
     if (