Skip to content

Conversation

gaearon
Copy link
Collaborator

@gaearon gaearon commented Aug 3, 2017

We did this for initial compatibility with Stack, but this behavior seems undesirable because it's inconsistent with componentDidMount.

Past discussion in #9426 (comment) seems to uncover Enzyme depends on it. Can we verify this is the case? I ran Jest internally and did not see issues caused by this change. Since Enzyme needs to adapt to other 16 changes, we might as well make this one.

Copy link
Collaborator

@sophiebits sophiebits left a comment

Choose a reason for hiding this comment

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

I think this is good

Sent from my iPhone haha
Copy link
Contributor

@bvaughn bvaughn left a comment

Choose a reason for hiding this comment

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

I'm okay with this change. Seems safer.

typeof this._instance.componentDidUpdate === 'function'
) {
this._instance.componentDidUpdate(oldProps, oldState);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: Might be nice to leave a placeholder comment referencing why we decided not to call cDU

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants