diff --git a/packages/react-dom/src/__tests__/ReactComponentLifeCycle-test.js b/packages/react-dom/src/__tests__/ReactComponentLifeCycle-test.js index aa81546bd211b..ef0ee8ac13bfc 100644 --- a/packages/react-dom/src/__tests__/ReactComponentLifeCycle-test.js +++ b/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', () => { + const divRef = React.createRef(); + 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 ( +
{`remote:${ + this.state.remote + }, local:${this.state.local}`}
+ ); + } + } + + class Parent extends React.Component { + state = {value: 0}; + handleChange = value => { + this.setState({value}); + }; + render() { + return ; + } + } + + ReactTestUtils.renderIntoDocument(); + expect(divRef.current.textContent).toBe('remote:0, local:0'); + + // Trigger setState() calls + childInstance.updateState(); + expect(divRef.current.textContent).toBe('remote:1, local:1'); + + // Trigger batched setState() calls + ReactTestUtils.Simulate.click(divRef.current); + expect(divRef.current.textContent).toBe('remote:2, local:2'); + }); }); diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.js b/packages/react-reconciler/src/ReactFiberBeginWork.js index 1b2b799fe13ba..4a310ce2897c6 100644 --- a/packages/react-reconciler/src/ReactFiberBeginWork.js +++ b/packages/react-reconciler/src/ReactFiberBeginWork.js @@ -612,6 +612,7 @@ export default function( workInProgress, value, props, + workInProgress.memoizedState, ); if (partialState !== null && partialState !== undefined) { diff --git a/packages/react-reconciler/src/ReactFiberClassComponent.js b/packages/react-reconciler/src/ReactFiberClassComponent.js index 7f0514f6e413a..092ff7b149c69 100644 --- a/packages/react-reconciler/src/ReactFiberClassComponent.js +++ b/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,15 +696,6 @@ 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. @@ -743,6 +732,18 @@ export default function( newState = oldState; } + let derivedStateFromProps; + if (oldProps !== newProps) { + // 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. @@ -867,15 +868,6 @@ 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. @@ -912,6 +904,18 @@ export default function( newState = oldState; } + let derivedStateFromProps; + if (oldProps !== newProps) { + // 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.