From 2dc1f6404830d8c478911fa64680d009013272aa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=99=B3=E4=B9=99=E5=B1=B1?= Date: Thu, 17 Jan 2019 14:58:46 +0800 Subject: [PATCH 1/4] Fix shallow renderer set instance state after gDSFP before calling sCU --- .../__tests__/ReactComponentLifeCycle-test.js | 34 ++++++++++++++++++ .../src/ReactShallowRenderer.js | 13 ++++--- .../__tests__/ReactShallowRenderer-test.js | 36 +++++++++++++++++++ 3 files changed, 79 insertions(+), 4 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactComponentLifeCycle-test.js b/packages/react-dom/src/__tests__/ReactComponentLifeCycle-test.js index 8879bca9987e3..cb073938bd118 100644 --- a/packages/react-dom/src/__tests__/ReactComponentLifeCycle-test.js +++ b/packages/react-dom/src/__tests__/ReactComponentLifeCycle-test.js @@ -1201,6 +1201,40 @@ describe('ReactComponentLifeCycle', () => { expect(log).toEqual([]); }); + it('should pass old state to shouldComponentUpdate even getDerivedStateFromProps return new state', () => { + const divRef = React.createRef(); + class SimpleComponent extends React.Component { + constructor(props) { + super(props); + this.state = { + value: props.value, + }; + } + + static getDerivedStateFromProps(nextProps, prevState) { + if (nextProps.value === prevState.value) { + return null; + } + return {value: nextProps.value}; + } + + shouldComponentUpdate(nextProps, nextState) { + return nextState.value !== this.state.value; + } + + render() { + return
value: {this.state.value}
; + } + } + + const div = document.createElement('div'); + + ReactDOM.render(, div); + expect(divRef.current.textContent).toBe('value: initial'); + ReactDOM.render(, div); + expect(divRef.current.textContent).toBe('value: updated'); + }); + it('should call getSnapshotBeforeUpdate before mutations are committed', () => { const log = []; diff --git a/packages/react-test-renderer/src/ReactShallowRenderer.js b/packages/react-test-renderer/src/ReactShallowRenderer.js index 9bf373d033caf..a2a6484ecf32a 100644 --- a/packages/react-test-renderer/src/ReactShallowRenderer.js +++ b/packages/react-test-renderer/src/ReactShallowRenderer.js @@ -157,7 +157,10 @@ class ReactShallowRenderer { this._updater, ); - this._updateStateFromStaticLifecycle(element.props); + const updated = this._updateNewStateFromStaticLifecycle(element.props); + if (updated) { + this._instance.state = this._newState; + } if (element.type.hasOwnProperty('contextTypes')) { currentlyValidatingElement = element; @@ -263,7 +266,7 @@ class ReactShallowRenderer { } } } - this._updateStateFromStaticLifecycle(props); + this._updateNewStateFromStaticLifecycle(props); // Read state after cWRP in case it calls setState const state = this._newState || oldState; @@ -310,7 +313,7 @@ class ReactShallowRenderer { // because DOM refs are not available. } - _updateStateFromStaticLifecycle(props) { + _updateNewStateFromStaticLifecycle(props) { const {type} = this._element; if (typeof type.getDerivedStateFromProps === 'function') { @@ -323,9 +326,11 @@ class ReactShallowRenderer { if (partialState != null) { const newState = Object.assign({}, oldState, partialState); - this._instance.state = this._newState = newState; + this._newState = newState; + return true; } } + return false; } } diff --git a/packages/react-test-renderer/src/__tests__/ReactShallowRenderer-test.js b/packages/react-test-renderer/src/__tests__/ReactShallowRenderer-test.js index b6c4259af9f40..13bae5a0622c3 100644 --- a/packages/react-test-renderer/src/__tests__/ReactShallowRenderer-test.js +++ b/packages/react-test-renderer/src/__tests__/ReactShallowRenderer-test.js @@ -942,6 +942,42 @@ describe('ReactShallowRenderer', () => { expect(result).toEqual(
value:1
); }); + it('should pass old state to shouldComponentUpdate even getDerivedStateFromProps return new state', () => { + class SimpleComponent extends React.Component { + constructor(props) { + super(props); + this.state = { + value: props.value, + }; + } + + static getDerivedStateFromProps(nextProps, prevState) { + if (nextProps.value === prevState.value) { + return null; + } + return {value: nextProps.value}; + } + + shouldComponentUpdate(nextProps, nextState) { + return nextState.value !== this.state.value; + } + + render() { + return
{`value:${this.state.value}`}
; + } + } + + const shallowRenderer = createRenderer(); + const initialResult = shallowRenderer.render( + , + ); + expect(initialResult).toEqual(
value:initial
); + const updatedResult = shallowRenderer.render( + , + ); + expect(updatedResult).toEqual(
value:updated
); + }); + it('can setState with an updater function', () => { let instance; From 5f343a3dda1f74bcc49d8282294bac7d2fd914d3 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Fri, 18 Jan 2019 01:13:18 +0000 Subject: [PATCH 2/4] Update ReactShallowRenderer.js --- packages/react-test-renderer/src/ReactShallowRenderer.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/react-test-renderer/src/ReactShallowRenderer.js b/packages/react-test-renderer/src/ReactShallowRenderer.js index 4dea6d5f5e11f..303ae674dc512 100644 --- a/packages/react-test-renderer/src/ReactShallowRenderer.js +++ b/packages/react-test-renderer/src/ReactShallowRenderer.js @@ -703,7 +703,7 @@ class ReactShallowRenderer { // because DOM refs are not available. } - _updateStateFromStaticLifecycle(props: Object) { + _updateNewStateFromStaticLifecycle(props: Object) { if (this._element === null) { return; } From 94b6f8ce9a90da5b70092cecdedf52cb82bfe1b0 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Fri, 18 Jan 2019 01:53:41 +0000 Subject: [PATCH 3/4] Unwind abstraction --- .../__tests__/ReactComponentLifeCycle-test.js | 2 +- .../src/ReactShallowRenderer.js | 51 +++++++++---------- .../__tests__/ReactShallowRenderer-test.js | 2 +- 3 files changed, 25 insertions(+), 30 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactComponentLifeCycle-test.js b/packages/react-dom/src/__tests__/ReactComponentLifeCycle-test.js index cb073938bd118..2bbd3c57c258f 100644 --- a/packages/react-dom/src/__tests__/ReactComponentLifeCycle-test.js +++ b/packages/react-dom/src/__tests__/ReactComponentLifeCycle-test.js @@ -1201,7 +1201,7 @@ describe('ReactComponentLifeCycle', () => { expect(log).toEqual([]); }); - it('should pass old state to shouldComponentUpdate even getDerivedStateFromProps return new state', () => { + it('should pass previous state to shouldComponentUpdate even with getDerivedStateFromProps', () => { const divRef = React.createRef(); class SimpleComponent extends React.Component { constructor(props) { diff --git a/packages/react-test-renderer/src/ReactShallowRenderer.js b/packages/react-test-renderer/src/ReactShallowRenderer.js index d36f4f6148d05..6d4d213932219 100644 --- a/packages/react-test-renderer/src/ReactShallowRenderer.js +++ b/packages/react-test-renderer/src/ReactShallowRenderer.js @@ -529,9 +529,17 @@ class ReactShallowRenderer { this._updater, ); - const updated = this._updateNewStateFromStaticLifecycle(element.props); - if (updated) { - this._instance.state = this._newState; + if (typeof element.type.getDerivedStateFromProps === 'function') { + const oldState = this._instance.state; + const partialState = element.type.getDerivedStateFromProps.call( + null, + element.props, + oldState, + ); + if (partialState != null) { + const newState = Object.assign({}, oldState, partialState); + this._instance.state = newState; + } } if (element.type.hasOwnProperty('contextTypes')) { @@ -656,10 +664,19 @@ class ReactShallowRenderer { } } } - this._updateNewStateFromStaticLifecycle(props); // Read state after cWRP in case it calls setState - const state = this._newState || oldState; + let state = this._newState || oldState; + if (typeof type.getDerivedStateFromProps === 'function') { + const partialState = type.getDerivedStateFromProps.call( + null, + props, + state, + ); + if (partialState != null) { + state = Object.assign({}, state, partialState); + } + } let shouldUpdate = true; if (this._forcedUpdate) { @@ -695,6 +712,7 @@ class ReactShallowRenderer { this._instance.context = context; this._instance.props = props; this._instance.state = state; + this._newState = null; if (shouldUpdate) { this._rendered = this._instance.render(); @@ -702,29 +720,6 @@ class ReactShallowRenderer { // Intentionally do not call componentDidUpdate() // because DOM refs are not available. } - - _updateNewStateFromStaticLifecycle(props: Object) { - if (this._element === null) { - return; - } - const {type} = this._element; - - if (typeof type.getDerivedStateFromProps === 'function') { - const oldState = this._newState || this._instance.state; - const partialState = type.getDerivedStateFromProps.call( - null, - props, - oldState, - ); - - if (partialState != null) { - const newState = Object.assign({}, oldState, partialState); - this._newState = newState; - return true; - } - } - return false; - } } let currentlyValidatingElement = null; diff --git a/packages/react-test-renderer/src/__tests__/ReactShallowRenderer-test.js b/packages/react-test-renderer/src/__tests__/ReactShallowRenderer-test.js index 13bae5a0622c3..d83dada160f90 100644 --- a/packages/react-test-renderer/src/__tests__/ReactShallowRenderer-test.js +++ b/packages/react-test-renderer/src/__tests__/ReactShallowRenderer-test.js @@ -942,7 +942,7 @@ describe('ReactShallowRenderer', () => { expect(result).toEqual(
value:1
); }); - it('should pass old state to shouldComponentUpdate even getDerivedStateFromProps return new state', () => { + it('should pass previous state to shouldComponentUpdate even with getDerivedStateFromProps', () => { class SimpleComponent extends React.Component { constructor(props) { super(props); From 58f1532a930d220765435df6b3a0236a1a12f6f8 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Fri, 18 Jan 2019 01:58:02 +0000 Subject: [PATCH 4/4] Fewer names --- .../react-test-renderer/src/ReactShallowRenderer.js | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/packages/react-test-renderer/src/ReactShallowRenderer.js b/packages/react-test-renderer/src/ReactShallowRenderer.js index 6d4d213932219..778e5943ec9c1 100644 --- a/packages/react-test-renderer/src/ReactShallowRenderer.js +++ b/packages/react-test-renderer/src/ReactShallowRenderer.js @@ -530,15 +530,17 @@ class ReactShallowRenderer { ); if (typeof element.type.getDerivedStateFromProps === 'function') { - const oldState = this._instance.state; const partialState = element.type.getDerivedStateFromProps.call( null, element.props, - oldState, + this._instance.state, ); if (partialState != null) { - const newState = Object.assign({}, oldState, partialState); - this._instance.state = newState; + this._instance.state = Object.assign( + {}, + this._instance.state, + partialState, + ); } }