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.

Sign up for GitHub

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

Jump to bottom

Fix to call componentDidUpdate on setState of React v16 #1261

Merged
merged 1 commit into from
Nov 3, 2017
Merged

Fix to call componentDidUpdate on setState of React v16 #1261

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
22 changes: 14 additions & 8 deletions packages/enzyme-test-suite/test/ShallowWrapper-spec.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1028,7 +1028,7 @@ describe('shallow', () => {
expect(wrapper.first('div').text()).to.equal('yolo');
});

it('should call componentWillReceiveProps, shouldComponentUpdate and componentWillUpdate with merged newProps', () => {
it('should call componentWillReceiveProps, shouldComponentUpdate, componentWillUpdate, and componentDidUpdate with merged newProps', () => {
const spy = sinon.spy();

class Foo extends React.Component {
Expand All @@ -1042,6 +1042,9 @@ describe('shallow', () => {
componentWillUpdate(nextProps) {
spy('componentWillUpdate', this.props, nextProps);
}
componentDidUpdate(prevProps) {
spy('componentDidUpdate', prevProps, this.props);
}
render() {
return (
<div />
Expand Down Expand Up @@ -1069,6 +1072,11 @@ describe('shallow', () => {
{ a: 'a', b: 'b' },
{ a: 'a', b: 'c', d: 'e' },
],
[
'componentDidUpdate',
{ a: 'a', b: 'b' },
{ a: 'a', b: 'c', d: 'e' },
],
]);
});

Expand Down Expand Up @@ -3098,7 +3106,7 @@ describe('shallow', () => {
]);
});

// componentDidUpdate does not seem to get called in react 16 beta.
// componentDidUpdate is not called in react 16
itIf(REACT16, 'calls expected methods for setState', () => {
wrapper.setState({ bar: 'bar' });
expect(spy.args).to.deep.equal([
Expand Down Expand Up @@ -3519,15 +3527,13 @@ describe('shallow', () => {
[
'render',
],
];
if (!REACT16) {
expected.push([
[
'componentDidUpdate',
{ foo: 'props' }, { foo: 'props' },
{ foo: 'bar' }, { foo: 'baz' },
{ foo: 'context' },
]);
}
REACT16 ? undefined : { foo: 'context' },
],
];
expect(spy.args).to.deep.equal(expected);
});

Expand Down
6 changes: 3 additions & 3 deletions packages/enzyme/src/ShallowWrapper.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -235,7 +235,7 @@ class ShallowWrapper {

/**
* A method is for re-render with new props and context.
* This calls componentDidUpdate method if lifecycleExperimental is enabled.
* This calls componentDidUpdate method if disableLifecycleMethods is not enabled.
*
* NOTE: can only be called on a wrapper instance that is also the root instance.
*
Expand Down Expand Up @@ -373,7 +373,7 @@ class ShallowWrapper {
// so we replace shouldComponentUpdate to know the result and restore it later.
let originalShouldComponentUpdate;
if (
this[OPTIONS].lifecycleExperimental &&
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hm, i wonder if this is causing a number of other bugs related to lifecycle methods?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't find issues causing by this in enzyme repository, but there might be in outside.

!this[OPTIONS].disableLifecycleMethods &&
adapter.options.enableComponentDidUpdateOnSetState &&
instance &&
typeof instance.shouldComponentUpdate === 'function'
Expand All @@ -388,7 +388,7 @@ class ShallowWrapper {
instance.setState(state, callback);
if (
shouldRender &&
this[OPTIONS].lifecycleExperimental &&
!this[OPTIONS].disableLifecycleMethods &&
adapter.options.enableComponentDidUpdateOnSetState &&
instance &&
typeof instance.componentDidUpdate === 'function'
Expand Down