Merge pull request #1785 from koba04/setState-returning-null-or-undef…

…ined-is-noop Fix to skip updates when nextState is null or undefined
enzymejs · Aug 22, 2018 · 5a4bb4e · 5a4bb4e
2 parents 8bc3635 + b88b425
commit 5a4bb4e
Show file tree

Hide file tree

Showing 7 changed files with 154 additions and 3 deletions.
diff --git 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
@@ -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
@@ -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
@@ -245,6 +245,9 @@ class ReactSixteenAdapter extends EnzymeAdapter {
           onSetState: true,
         },
         getSnapshotBeforeUpdate: true,
+        setState: {
+          skipsComponentDidUpdateOnNullish: true,
+        },
       },
     };
   }

diff --git 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 (
+            <div className={this.state.id} />
+          );
+        }
+      }
+
+      const wrapper = mount(<Foo />);
+      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 (
+            <div className={this.state.id} />
+          );
+        }
+      }
+
+      let payload;
+      const stub = sinon.stub(Foo.prototype, 'componentDidUpdate')
+        .callsFake(function componentDidUpdate() { this.setState(() => payload); });
+
+      const wrapper = mount(<Foo />);
+
+      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
@@ -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 (
+            <div className={this.state.id} />
+          );
+        }
+      }
+
+      const wrapper = shallow(<Foo />);
+      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 (
+            <div className={this.state.id} />
+          );
+        }
+      }
+
+      let payload;
+      const stub = sinon.stub(Foo.prototype, 'componentDidUpdate')
+        .callsFake(function componentDidUpdate() { this.setState(() => payload); });
+
+      const wrapper = shallow(<Foo />);
+
+      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
@@ -129,6 +129,9 @@ function getAdapterLifecycles({ options }) {
 
   return {
     ...lifecycles,
+    setState: {
+      ...lifecycles.setState,
+    },
     ...(componentDidUpdate && { componentDidUpdate }),
   };
 }
@@ -436,6 +439,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 +450,16 @@ 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 in React 16+
+        // /~https://github.com/facebook/react/pull/12756
+        const maybeHasUpdate = !lifecycles.setState.skipsComponentDidUpdateOnNullish
+          || statePayload != null;
+
         // When shouldComponentUpdate returns false we shouldn't call componentDidUpdate.
         // so we spy shouldComponentUpdate to get the result.
         let spy;
@@ -462,16 +476,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