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 componentWillUnmount() not counted by ReactPerf #6858

Merged
merged 3 commits into from
May 24, 2016
Merged

Fix componentWillUnmount() not counted by ReactPerf #6858

merged 3 commits into from
May 24, 2016

Conversation

gaearon
Copy link
Collaborator

@gaearon gaearon commented May 24, 2016

This fixes componentWillUnmount() not being measured because ReactDebugTool doesn’t realize there is a flush going on. Also adding tests to check that all lifecycle methods are instrumented.

@gaearon
Copy link
Collaborator Author

gaearon commented May 24, 2016

@spicyj

@ghost
Copy link

ghost commented May 24, 2016

@gaearon updated the pull request.

@sophiebits
Copy link
Collaborator

👍

I assume we already have tests to make sure renders due to setState are instrumented? Because that's a different code path than the top-level renders. Thanks for including a top-level update though.

@gaearon
Copy link
Collaborator Author

gaearon commented May 24, 2016

We currently don’t, sorry 😄 . I’ll add a setState path here as well before merging.

@gaearon gaearon merged commit 9ba5668 into facebook:master May 24, 2016
@ghost
Copy link

ghost commented May 24, 2016

@gaearon updated the pull request.

@zpao zpao added this to the 15-next milestone Jun 1, 2016
zpao pushed a commit to zpao/react that referenced this pull request Jun 8, 2016
* Fix componentWillUnmount() not counted by ReactPerf

* Test that functional component render() time shows up in ReactPerf

* Test for setState() code path updates being included

(cherry picked from commit 9ba5668)
zpao pushed a commit that referenced this pull request Jun 14, 2016
* Fix componentWillUnmount() not counted by ReactPerf

* Test that functional component render() time shows up in ReactPerf

* Test for setState() code path updates being included

(cherry picked from commit 9ba5668)
@zpao zpao modified the milestones: 15-next, 15.2.0 Jun 14, 2016
@gaearon gaearon deleted the perf-lifecycle-fix branch July 1, 2016 17:30
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.

3 participants