Do not fire getDerivedStateFromProps unless props or state have changed

Fixes an oversight from #12600. getDerivedStateFromProps should fire if either props *or* state have changed, but not if *neither* have changed. This prevents a parent from re-rendering if a deep child receives an update.
facebook · May 14, 2018 · ddbfffe · ddbfffe
1 parent 4b2e65d
commit ddbfffe
Show file tree

Hide file tree

Showing 2 changed files with 53 additions and 21 deletions.
diff --git a/packages/react-reconciler/src/ReactFiberClassComponent.js b/packages/react-reconciler/src/ReactFiberClassComponent.js
@@ -802,16 +802,6 @@ export default function(
       );
       newState = workInProgress.memoizedState;
     }
-
-    if (typeof getDerivedStateFromProps === 'function') {
-      applyDerivedStateFromProps(
-        workInProgress,
-        getDerivedStateFromProps,
-        newProps,
-      );
-      newState = workInProgress.memoizedState;
-    }
-
     if (
       oldProps === newProps &&
       oldState === newState &&
@@ -829,6 +819,15 @@ export default function(
       return false;
     }
 
+    if (typeof getDerivedStateFromProps === 'function') {
+      applyDerivedStateFromProps(
+        workInProgress,
+        getDerivedStateFromProps,
+        newProps,
+      );
+      newState = workInProgress.memoizedState;
+    }
+
     const shouldUpdate = checkShouldComponentUpdate(
       workInProgress,
       oldProps,
@@ -937,17 +936,6 @@ export default function(
       newState = workInProgress.memoizedState;
     }
 
-    if (typeof getDerivedStateFromProps === 'function') {
-      if (fireGetDerivedStateFromPropsOnStateUpdates || oldProps !== newProps) {
-        applyDerivedStateFromProps(
-          workInProgress,
-          getDerivedStateFromProps,
-          newProps,
-        );
-        newState = workInProgress.memoizedState;
-      }
-    }
-
     if (
       oldProps === newProps &&
       oldState === newState &&
@@ -978,6 +966,17 @@ export default function(
       return false;
     }
 
+    if (typeof getDerivedStateFromProps === 'function') {
+      if (fireGetDerivedStateFromPropsOnStateUpdates || oldProps !== newProps) {
+        applyDerivedStateFromProps(
+          workInProgress,
+          getDerivedStateFromProps,
+          newProps,
+        );
+        newState = workInProgress.memoizedState;
+      }
+    }
+
     const shouldUpdate = checkShouldComponentUpdate(
       workInProgress,
       oldProps,

diff --git a/packages/react-reconciler/src/__tests__/ReactIncremental-test.internal.js b/packages/react-reconciler/src/__tests__/ReactIncremental-test.internal.js
@@ -1464,6 +1464,39 @@ describe('ReactIncremental', () => {
     expect(instance.state).toEqual({foo: 'foo'});
   });
 
+  it('does not call getDerivedStateFromProps if neither state nor props have changed', () => {
+    class Parent extends React.Component {
+      state = {parentRenders: 0};
+      static getDerivedStateFromProps(props, prevState) {
+        ReactNoop.yield('getDerivedStateFromProps');
+        return prevState.parentRenders + 1;
+      }
+      render() {
+        ReactNoop.yield('Parent');
+        return <Child parentRenders={this.state.parentRenders} ref={child} />;
+      }
+    }
+
+    class Child extends React.Component {
+      render() {
+        ReactNoop.yield('Child');
+        return this.props.parentRenders;
+      }
+    }
+
+    const child = React.createRef(null);
+    ReactNoop.render(<Parent />);
+    expect(ReactNoop.flush()).toEqual([
+      'getDerivedStateFromProps',
+      'Parent',
+      'Child',
+    ]);
+
+    // Schedule an update on the child. The parent should not re-render.
+    child.current.setState({});
+    expect(ReactNoop.flush()).toEqual(['Child']);
+  });
+
   it('does not call getDerivedStateFromProps for state-only updates if feature flag is disabled', () => {
     jest.resetModules();
     ReactFeatureFlags = require('shared/ReactFeatureFlags');