Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix to skip updates when nextState is null or undefined #1785

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,9 @@ class ReactSixteenOneAdapter extends EnzymeAdapter {
componentDidUpdate: {
onSetState: true,
},
setState: {
skipsComponentDidUpdateOnNullish: true,
},
},
};
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -222,6 +222,9 @@ class ReactSixteenTwoAdapter extends EnzymeAdapter {
componentDidUpdate: {
onSetState: true,
},
setState: {
skipsComponentDidUpdateOnNullish: true,
},
},
};
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -241,6 +241,9 @@ class ReactSixteenThreeAdapter extends EnzymeAdapter {
onSetState: true,
},
getSnapshotBeforeUpdate: true,
setState: {
skipsComponentDidUpdateOnNullish: true,
},
},
};
}
Expand Down
3 changes: 3 additions & 0 deletions packages/enzyme-adapter-react-16/src/ReactSixteenAdapter.js
Original file line number Diff line number Diff line change
Expand Up @@ -245,6 +245,9 @@ class ReactSixteenAdapter extends EnzymeAdapter {
onSetState: true,
},
getSnapshotBeforeUpdate: true,
setState: {
skipsComponentDidUpdateOnNullish: true,
},
},
};
}
Expand Down
62 changes: 62 additions & 0 deletions packages/enzyme-test-suite/test/ReactWrapper-spec.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
62 changes: 62 additions & 0 deletions packages/enzyme-test-suite/test/ShallowWrapper-spec.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
21 changes: 18 additions & 3 deletions packages/enzyme/src/ShallowWrapper.js
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,9 @@ function getAdapterLifecycles({ options }) {

return {
...lifecycles,
setState: {
...lifecycles.setState,
},
...(componentDidUpdate && { componentDidUpdate }),
};
}
Expand Down Expand Up @@ -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]);
Expand All @@ -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;
Expand All @@ -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
Expand Down