Skip to content

Commit

Permalink
Do not fire getDerivedStateFromProps unless props or state have changed
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
acdlite committed May 14, 2018
1 parent 4b2e65d commit e853d98
Show file tree
Hide file tree
Showing 2 changed files with 52 additions and 21 deletions.
41 changes: 20 additions & 21 deletions packages/react-reconciler/src/ReactFiberClassComponent.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 &&
Expand All @@ -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,
Expand Down Expand Up @@ -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 &&
Expand Down Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1464,6 +1464,38 @@ 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',
]);

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');
Expand Down

0 comments on commit e853d98

Please sign in to comment.