From cba495ff8b62a813655029d06c0ab84a3bbf47ca Mon Sep 17 00:00:00 2001 From: Toru Kobayashi Date: Wed, 22 Aug 2018 12:25:05 +0900 Subject: [PATCH 1/2] [Fix] `shallow`: skip updates when nextState is `null` or `undefined` - related: /~https://github.com/facebook/react/pull/12756 Fixes #1783. --- .../test/ReactWrapper-spec.jsx | 62 +++++++++++++++++++ .../test/ShallowWrapper-spec.jsx | 62 +++++++++++++++++++ packages/enzyme/src/ShallowWrapper.js | 17 ++++- 3 files changed, 138 insertions(+), 3 deletions(-) diff --git a/packages/enzyme-test-suite/test/ReactWrapper-spec.jsx b/packages/enzyme-test-suite/test/ReactWrapper-spec.jsx index e426c9e83..306da0771 100644 --- a/packages/enzyme-test-suite/test/ReactWrapper-spec.jsx +++ b/packages/enzyme-test-suite/test/ReactWrapper-spec.jsx @@ -2460,6 +2460,68 @@ describeWithDOM('mount', () => { }); }); + it('prevents the update if nextState is null or undefined', () => { + class Foo extends React.Component { + constructor(props) { + super(props); + this.state = { id: 'foo' }; + } + + componentDidUpdate() {} + + render() { + return ( +
+ ); + } + } + + const wrapper = mount(); + const spy = sinon.spy(wrapper.instance(), 'componentDidUpdate'); + const callback = sinon.spy(); + wrapper.setState(() => ({ id: 'bar' }), callback); + expect(spy).to.have.property('callCount', 1); + expect(callback).to.have.property('callCount', 1); + + wrapper.setState(() => null, callback); + expect(spy).to.have.property('callCount', is('>= 16') ? 1 : 2); + expect(callback).to.have.property('callCount', 2); + + wrapper.setState(() => undefined, callback); + expect(spy).to.have.property('callCount', is('>= 16') ? 1 : 3); + expect(callback).to.have.property('callCount', 3); + }); + + itIf(is('>= 16'), 'prevents an infinite loop if nextState is null or undefined from setState in CDU', () => { + class Foo extends React.Component { + constructor(props) { + super(props); + this.state = { id: 'foo' }; + } + + componentDidUpdate() {} + + render() { + return ( +
+ ); + } + } + + let payload; + const stub = sinon.stub(Foo.prototype, 'componentDidUpdate') + .callsFake(function componentDidUpdate() { this.setState(() => payload); }); + + const wrapper = mount(); + + wrapper.setState(() => ({ id: 'bar' })); + expect(stub).to.have.property('callCount', 1); + + payload = null; + wrapper.setState(() => ({ id: 'bar' })); + expect(stub).to.have.property('callCount', 2); + }); + describe('should not call componentWillReceiveProps after setState is called', () => { it('should not call componentWillReceiveProps upon rerender', () => { class A extends React.Component { diff --git a/packages/enzyme-test-suite/test/ShallowWrapper-spec.jsx b/packages/enzyme-test-suite/test/ShallowWrapper-spec.jsx index 93e9b2093..71b790ae5 100644 --- a/packages/enzyme-test-suite/test/ShallowWrapper-spec.jsx +++ b/packages/enzyme-test-suite/test/ShallowWrapper-spec.jsx @@ -2406,6 +2406,68 @@ describe('shallow', () => { }); }); + it('prevents the update if nextState is null or undefined', () => { + class Foo extends React.Component { + constructor(props) { + super(props); + this.state = { id: 'foo' }; + } + + componentDidUpdate() {} + + render() { + return ( +
+ ); + } + } + + const wrapper = shallow(); + const spy = sinon.spy(wrapper.instance(), 'componentDidUpdate'); + const callback = sinon.spy(); + wrapper.setState(() => ({ id: 'bar' }), callback); + expect(spy).to.have.property('callCount', 1); + expect(callback).to.have.property('callCount', 1); + + wrapper.setState(() => null, callback); + expect(spy).to.have.property('callCount', is('>= 16') ? 1 : 2); + expect(callback).to.have.property('callCount', 2); + + wrapper.setState(() => undefined, callback); + expect(spy).to.have.property('callCount', is('>= 16') ? 1 : 3); + expect(callback).to.have.property('callCount', 3); + }); + + itIf(is('>= 16'), 'prevents an infinite loop if nextState is null or undefined from setState in CDU', () => { + class Foo extends React.Component { + constructor(props) { + super(props); + this.state = { id: 'foo' }; + } + + componentDidUpdate() {} + + render() { + return ( +
+ ); + } + } + + let payload; + const stub = sinon.stub(Foo.prototype, 'componentDidUpdate') + .callsFake(function componentDidUpdate() { this.setState(() => payload); }); + + const wrapper = shallow(); + + wrapper.setState(() => ({ id: 'bar' })); + expect(stub).to.have.property('callCount', 1); + + payload = null; + wrapper.setState(() => ({ id: 'bar' })); + expect(stub).to.have.property('callCount', 2); + }); + describe('should not call componentWillReceiveProps after setState is called', () => { it('should not call componentWillReceiveProps upon rerender', () => { class A extends React.Component { diff --git a/packages/enzyme/src/ShallowWrapper.js b/packages/enzyme/src/ShallowWrapper.js index 264a54597..ff0e073e1 100644 --- a/packages/enzyme/src/ShallowWrapper.js +++ b/packages/enzyme/src/ShallowWrapper.js @@ -436,6 +436,7 @@ class ShallowWrapper { if (arguments.length > 1 && typeof callback !== 'function') { throw new TypeError('ReactWrapper::setState() expects a function as its second argument'); } + this.single('setState', () => { withSetStateAllowed(() => { const adapter = getAdapter(this[OPTIONS]); @@ -446,6 +447,15 @@ class ShallowWrapper { const prevProps = instance.props; const prevState = instance.state; const prevContext = instance.context; + + const statePayload = typeof state === 'function' + ? state.call(instance, prevState, prevProps) + : state; + + // returning null or undefined prevents the update + // /~https://github.com/facebook/react/pull/12756 + const maybeHasUpdate = statePayload != null; + // When shouldComponentUpdate returns false we shouldn't call componentDidUpdate. // so we spy shouldComponentUpdate to get the result. let spy; @@ -462,16 +472,17 @@ class ShallowWrapper { // We don't pass the setState callback here // to guarantee to call the callback after finishing the render if (instance[SET_STATE]) { - instance[SET_STATE](state); + instance[SET_STATE](statePayload); } else { - instance.setState(state); + instance.setState(statePayload); } if (spy) { shouldRender = spy.getLastReturnValue(); spy.restore(); } if ( - shouldRender + maybeHasUpdate + && shouldRender && !this[OPTIONS].disableLifecycleMethods && lifecycles.componentDidUpdate && lifecycles.componentDidUpdate.onSetState From b88b4254530e51f6aede896a33703747fc947b4c Mon Sep 17 00:00:00 2001 From: Jordan Harband Date: Tue, 21 Aug 2018 22:37:59 -0700 Subject: [PATCH 2/2] [enzyme, enzyme-adapter-react-16*] `shallow`: only skip-cDU-on-nullish when adapters opt in. --- .../src/ReactSixteenOneAdapter.js | 3 +++ .../src/ReactSixteenTwoAdapter.js | 3 +++ .../src/ReactSixteenThreeAdapter.js | 3 +++ .../enzyme-adapter-react-16/src/ReactSixteenAdapter.js | 3 +++ packages/enzyme/src/ShallowWrapper.js | 8 ++++++-- 5 files changed, 18 insertions(+), 2 deletions(-) diff --git a/packages/enzyme-adapter-react-16.1/src/ReactSixteenOneAdapter.js b/packages/enzyme-adapter-react-16.1/src/ReactSixteenOneAdapter.js index 54d0d605b..4da8391e7 100644 --- a/packages/enzyme-adapter-react-16.1/src/ReactSixteenOneAdapter.js +++ b/packages/enzyme-adapter-react-16.1/src/ReactSixteenOneAdapter.js @@ -220,6 +220,9 @@ class ReactSixteenOneAdapter extends EnzymeAdapter { componentDidUpdate: { onSetState: true, }, + setState: { + skipsComponentDidUpdateOnNullish: true, + }, }, }; } diff --git a/packages/enzyme-adapter-react-16.2/src/ReactSixteenTwoAdapter.js b/packages/enzyme-adapter-react-16.2/src/ReactSixteenTwoAdapter.js index f5e52fed2..15845bddb 100644 --- a/packages/enzyme-adapter-react-16.2/src/ReactSixteenTwoAdapter.js +++ b/packages/enzyme-adapter-react-16.2/src/ReactSixteenTwoAdapter.js @@ -222,6 +222,9 @@ class ReactSixteenTwoAdapter extends EnzymeAdapter { componentDidUpdate: { onSetState: true, }, + setState: { + skipsComponentDidUpdateOnNullish: true, + }, }, }; } diff --git a/packages/enzyme-adapter-react-16.3/src/ReactSixteenThreeAdapter.js b/packages/enzyme-adapter-react-16.3/src/ReactSixteenThreeAdapter.js index fc0d700e7..a0cdc69ef 100644 --- a/packages/enzyme-adapter-react-16.3/src/ReactSixteenThreeAdapter.js +++ b/packages/enzyme-adapter-react-16.3/src/ReactSixteenThreeAdapter.js @@ -241,6 +241,9 @@ class ReactSixteenThreeAdapter extends EnzymeAdapter { onSetState: true, }, getSnapshotBeforeUpdate: true, + setState: { + skipsComponentDidUpdateOnNullish: true, + }, }, }; } diff --git a/packages/enzyme-adapter-react-16/src/ReactSixteenAdapter.js b/packages/enzyme-adapter-react-16/src/ReactSixteenAdapter.js index c20e9e7aa..43d0f7d07 100644 --- a/packages/enzyme-adapter-react-16/src/ReactSixteenAdapter.js +++ b/packages/enzyme-adapter-react-16/src/ReactSixteenAdapter.js @@ -245,6 +245,9 @@ class ReactSixteenAdapter extends EnzymeAdapter { onSetState: true, }, getSnapshotBeforeUpdate: true, + setState: { + skipsComponentDidUpdateOnNullish: true, + }, }, }; } diff --git a/packages/enzyme/src/ShallowWrapper.js b/packages/enzyme/src/ShallowWrapper.js index ff0e073e1..8160100b9 100644 --- a/packages/enzyme/src/ShallowWrapper.js +++ b/packages/enzyme/src/ShallowWrapper.js @@ -129,6 +129,9 @@ function getAdapterLifecycles({ options }) { return { ...lifecycles, + setState: { + ...lifecycles.setState, + }, ...(componentDidUpdate && { componentDidUpdate }), }; } @@ -452,9 +455,10 @@ class ShallowWrapper { ? state.call(instance, prevState, prevProps) : state; - // returning null or undefined prevents the update + // returning null or undefined prevents the update in React 16+ // /~https://github.com/facebook/react/pull/12756 - const maybeHasUpdate = statePayload != null; + const maybeHasUpdate = !lifecycles.setState.skipsComponentDidUpdateOnNullish + || statePayload != null; // When shouldComponentUpdate returns false we shouldn't call componentDidUpdate. // so we spy shouldComponentUpdate to get the result.