@samitouri / QOS-React / commits / c5d3104fc0

Do not fire getDerivedStateFromProps unless props or state have changed (#12802)

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.

Andrew Clark committed May 14, 2018 at 14:56 UTC c5d3104fc0958027820972f3416ec572ded89f28
2 files changed +53 -21
packages/react-reconciler/src/ReactFiberClassComponent.js
+20 -21
@@ -802,16 +802,6 @@ export default function(
802 );
803 newState = workInProgress.memoizedState;
804 }
805 -
806 - if (typeof getDerivedStateFromProps === 'function') {
807 - applyDerivedStateFromProps(
808 - workInProgress,
809 - getDerivedStateFromProps,
810 - newProps,
811 - );
812 - newState = workInProgress.memoizedState;
813 - }
814 -
805 if (
806 oldProps === newProps &&
807 oldState === newState &&
@@ -829,6 +819,15 @@ export default function(
819 return false;
820 }
821
822 + if (typeof getDerivedStateFromProps === 'function') {
823 + applyDerivedStateFromProps(
824 + workInProgress,
825 + getDerivedStateFromProps,
826 + newProps,
827 + );
828 + newState = workInProgress.memoizedState;
829 + }
830 +
831 const shouldUpdate = checkShouldComponentUpdate(
832 workInProgress,
833 oldProps,
@@ -937,17 +936,6 @@ export default function(
936 newState = workInProgress.memoizedState;
937 }
938
940 - if (typeof getDerivedStateFromProps === 'function') {
941 - if (fireGetDerivedStateFromPropsOnStateUpdates || oldProps !== newProps) {
942 - applyDerivedStateFromProps(
943 - workInProgress,
944 - getDerivedStateFromProps,
945 - newProps,
946 - );
947 - newState = workInProgress.memoizedState;
948 - }
949 - }
950 -
939 if (
940 oldProps === newProps &&
941 oldState === newState &&
@@ -978,6 +966,17 @@ export default function(
966 return false;
967 }
968
969 + if (typeof getDerivedStateFromProps === 'function') {
970 + if (fireGetDerivedStateFromPropsOnStateUpdates || oldProps !== newProps) {
971 + applyDerivedStateFromProps(
972 + workInProgress,
973 + getDerivedStateFromProps,
974 + newProps,
975 + );
976 + newState = workInProgress.memoizedState;
977 + }
978 + }
979 +
980 const shouldUpdate = checkShouldComponentUpdate(
981 workInProgress,
982 oldProps,
packages/react-reconciler/src/__tests__/ReactIncremental-test.internal.js
+33
@@ -1464,6 +1464,39 @@ describe('ReactIncremental', () => {
1464 expect(instance.state).toEqual({foo: 'foo'});
1465 });
1466
1467 + it('does not call getDerivedStateFromProps if neither state nor props have changed', () => {
1468 + class Parent extends React.Component {
1469 + state = {parentRenders: 0};
1470 + static getDerivedStateFromProps(props, prevState) {
1471 + ReactNoop.yield('getDerivedStateFromProps');
1472 + return prevState.parentRenders + 1;
1473 + }
1474 + render() {
1475 + ReactNoop.yield('Parent');
1476 + return <Child parentRenders={this.state.parentRenders} ref={child} />;
1477 + }
1478 + }
1479 +
1480 + class Child extends React.Component {
1481 + render() {
1482 + ReactNoop.yield('Child');
1483 + return this.props.parentRenders;
1484 + }
1485 + }
1486 +
1487 + const child = React.createRef(null);
1488 + ReactNoop.render(<Parent />);
1489 + expect(ReactNoop.flush()).toEqual([
1490 + 'getDerivedStateFromProps',
1491 + 'Parent',
1492 + 'Child',
1493 + ]);
1494 +
1495 + // Schedule an update on the child. The parent should not re-render.
1496 + child.current.setState({});
1497 + expect(ReactNoop.flush()).toEqual(['Child']);
1498 + });
1499 +
1500 it('does not call getDerivedStateFromProps for state-only updates if feature flag is disabled', () => {
1501 jest.resetModules();
1502 ReactFeatureFlags = require('shared/ReactFeatureFlags');