From 59a51653e157aa7bcc79db9c45abd82c1e561a49 Mon Sep 17 00:00:00 2001 From: Toru Kobayashi Date: Fri, 3 Nov 2017 20:17:53 +0900 Subject: [PATCH 1/2] Fix to reset the forceUpdate flag after the update --- .../react-test-renderer/src/ReactShallowRenderer.js | 3 +-- .../src/__tests__/ReactShallowRenderer-test.js | 11 +++++++++++ 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/packages/react-test-renderer/src/ReactShallowRenderer.js b/packages/react-test-renderer/src/ReactShallowRenderer.js index fc3baf251f029..8fe1dc264739e 100644 --- a/packages/react-test-renderer/src/ReactShallowRenderer.js +++ b/packages/react-test-renderer/src/ReactShallowRenderer.js @@ -157,8 +157,6 @@ class ReactShallowRenderer { this._instance.context = context; this._instance.props = props; this._instance.state = state; - this._forcedUpdate = false; - return; } } @@ -189,6 +187,7 @@ class Updater { enqueueForceUpdate(publicInstance, callback, callerName) { this._renderer._forcedUpdate = true; this._renderer.render(this._renderer._element, this._renderer._context); + this._renderer._forcedUpdate = false; if (typeof callback === 'function') { callback.call(publicInstance); diff --git a/packages/react-test-renderer/src/__tests__/ReactShallowRenderer-test.js b/packages/react-test-renderer/src/__tests__/ReactShallowRenderer-test.js index 1f51e390040b6..32be6a9589c89 100644 --- a/packages/react-test-renderer/src/__tests__/ReactShallowRenderer-test.js +++ b/packages/react-test-renderer/src/__tests__/ReactShallowRenderer-test.js @@ -125,6 +125,12 @@ describe('ReactShallowRenderer', () => { it('should not run shouldComponentUpdate during forced update', () => { let scuCounter = 0; class SimpleComponent extends React.Component { + constructor(props) { + super(props); + this.state = { + count: 1, + }; + } shouldComponentUpdate() { scuCounter++; } @@ -140,6 +146,11 @@ describe('ReactShallowRenderer', () => { const instance = shallowRenderer.getMountedInstance(); instance.forceUpdate(); expect(scuCounter).toEqual(0); + + // shouldComponentUpdate should be called + instance.setState(state => ({count: state.count + 1})); + expect(scuCounter).toEqual(1); + expect(instance.state.count).toEqual(2); }); it('should rerender when calling forceUpdate', () => { From c390ca96b21640a89a507311f601cbeb09f888d4 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Fri, 3 Nov 2017 14:26:52 +0000 Subject: [PATCH 2/2] Add support for PureComponent and mirror real behavior closer --- .../src/ReactShallowRenderer.js | 46 +++++++++++-------- .../__tests__/ReactShallowRenderer-test.js | 45 ++++++++++++++---- 2 files changed, 65 insertions(+), 26 deletions(-) diff --git a/packages/react-test-renderer/src/ReactShallowRenderer.js b/packages/react-test-renderer/src/ReactShallowRenderer.js index 8fe1dc264739e..8f64bef8e0da6 100644 --- a/packages/react-test-renderer/src/ReactShallowRenderer.js +++ b/packages/react-test-renderer/src/ReactShallowRenderer.js @@ -11,6 +11,7 @@ import describeComponentFrame from 'shared/describeComponentFrame'; import getComponentName from 'shared/getComponentName'; import emptyObject from 'fbjs/lib/emptyObject'; import invariant from 'fbjs/lib/invariant'; +import shallowEqual from 'fbjs/lib/shallowEqual'; import checkPropTypes from 'prop-types/checkPropTypes'; class ReactShallowRenderer { @@ -63,7 +64,7 @@ class ReactShallowRenderer { this._context = context; if (this._instance) { - this._updateClassComponent(element.props, context); + this._updateClassComponent(element.type, element.props, context); } else { if (shouldConstruct(element.type)) { this._instance = new element.type( @@ -133,7 +134,8 @@ class ReactShallowRenderer { // because DOM refs are not available. } - _updateClassComponent(props, context) { + _updateClassComponent(type, props, context) { + const oldState = this._instance.state || emptyObject; const oldProps = this._instance.props; if ( @@ -145,31 +147,40 @@ class ReactShallowRenderer { // Read state after cWRP in case it calls setState // Fallback to previous instance state to support rendering React.cloneElement() + // TODO: the cloneElement() use case is broken and should be removed + // https://github.com/facebook/react/issues/11441 const state = this._newState || this._instance.state || emptyObject; - if ( - typeof this._instance.shouldComponentUpdate === 'function' && - !this._forcedUpdate - ) { - if ( - this._instance.shouldComponentUpdate(props, state, context) === false - ) { - this._instance.context = context; - this._instance.props = props; - this._instance.state = state; - return; - } + let shouldUpdate = true; + if (this._forcedUpdate) { + shouldUpdate = true; + this._forcedUpdate = false; + } else if (typeof this._instance.shouldComponentUpdate === 'function') { + shouldUpdate = !!this._instance.shouldComponentUpdate( + props, + state, + context, + ); + } else if (type && type.prototype && type.prototype.isPureReactComponent) { + // TODO: we can remove the type existence check when we fix this: + // https://github.com/facebook/react/issues/11441 + shouldUpdate = + !shallowEqual(oldProps, props) || !shallowEqual(oldState, state); } - if (typeof this._instance.componentWillUpdate === 'function') { - this._instance.componentWillUpdate(props, state, context); + if (shouldUpdate) { + if (typeof this._instance.componentWillUpdate === 'function') { + this._instance.componentWillUpdate(props, state, context); + } } this._instance.context = context; this._instance.props = props; this._instance.state = state; - this._rendered = this._instance.render(); + if (shouldUpdate) { + this._rendered = this._instance.render(); + } // Intentionally do not call componentDidUpdate() // because DOM refs are not available. } @@ -187,7 +198,6 @@ class Updater { enqueueForceUpdate(publicInstance, callback, callerName) { this._renderer._forcedUpdate = true; this._renderer.render(this._renderer._element, this._renderer._context); - this._renderer._forcedUpdate = false; if (typeof callback === 'function') { callback.call(publicInstance); diff --git a/packages/react-test-renderer/src/__tests__/ReactShallowRenderer-test.js b/packages/react-test-renderer/src/__tests__/ReactShallowRenderer-test.js index 32be6a9589c89..7bd5cf159cff6 100644 --- a/packages/react-test-renderer/src/__tests__/ReactShallowRenderer-test.js +++ b/packages/react-test-renderer/src/__tests__/ReactShallowRenderer-test.js @@ -122,35 +122,64 @@ describe('ReactShallowRenderer', () => { expect(shallowRenderer.getRenderOutput()).toEqual(
2
); }); + it('should enable PureComponent to prevent a re-render', () => { + let renderCounter = 0; + class SimpleComponent extends React.PureComponent { + state = {update: false}; + render() { + renderCounter++; + return
{`${renderCounter}`}
; + } + } + + const shallowRenderer = createRenderer(); + shallowRenderer.render(); + expect(shallowRenderer.getRenderOutput()).toEqual(
1
); + + const instance = shallowRenderer.getMountedInstance(); + instance.setState({update: false}); + expect(shallowRenderer.getRenderOutput()).toEqual(
1
); + + instance.setState({update: true}); + expect(shallowRenderer.getRenderOutput()).toEqual(
2
); + }); + it('should not run shouldComponentUpdate during forced update', () => { let scuCounter = 0; class SimpleComponent extends React.Component { - constructor(props) { - super(props); - this.state = { - count: 1, - }; - } + state = {count: 1}; shouldComponentUpdate() { scuCounter++; + return false; } render() { - return
; + return
{`${this.state.count}`}
; } } const shallowRenderer = createRenderer(); shallowRenderer.render(); expect(scuCounter).toEqual(0); + expect(shallowRenderer.getRenderOutput()).toEqual(
1
); + // Force update the initial state. sCU should not fire. const instance = shallowRenderer.getMountedInstance(); instance.forceUpdate(); expect(scuCounter).toEqual(0); + expect(shallowRenderer.getRenderOutput()).toEqual(
1
); - // shouldComponentUpdate should be called + // Setting state updates the instance, but doesn't re-render + // because sCU returned false. instance.setState(state => ({count: state.count + 1})); expect(scuCounter).toEqual(1); expect(instance.state.count).toEqual(2); + expect(shallowRenderer.getRenderOutput()).toEqual(
1
); + + // A force update updates the render output, but doesn't call sCU. + instance.forceUpdate(); + expect(scuCounter).toEqual(1); + expect(instance.state.count).toEqual(2); + expect(shallowRenderer.getRenderOutput()).toEqual(
2
); }); it('should rerender when calling forceUpdate', () => {