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

Ignore DOM writes outside the batch in ReactPerf #6516

Merged
merged 1 commit into from
Apr 14, 2016
Merged

Ignore DOM writes outside the batch in ReactPerf #6516

merged 1 commit into from
Apr 14, 2016

Conversation

gaearon
Copy link
Collaborator

@gaearon gaearon commented Apr 14, 2016

ReactDOMInput and a few other classes handle change event by scheduling updateWrapper() in an asap(). It gets executed after the batch, which confuses ReactPerf that expects all DOM writes to happen during a batch.

This causes it to throw when calculating printWasted() if you interacted with an input before taking the measurement: #5548.

Since this implementation of ReactPerf relying on the stack is going away, let's plug the hole temporarily by ignoring DOM writes that occur during postponed updateWrapper(). In any case, they have no relation to actual calculations of wasted renders, as they don't occur due to updateComponent(), but rather due to onChange() special DOM behavior.

This fixes #5548. Thanks to @Danita for her help investigating the issue which allowed me to find the minimal case to repro this.

@gaearon gaearon added this to the 15.0.x milestone Apr 14, 2016
ReactDOMInput and a few other classes handle change event by scheduling updateWrapper() in an asap().
It gets executed after the batch, which confuses ReactPerf that expects all DOM writes to happen during a batch.
Since this implementation of ReactPerf relying on the stack is going away, let's plug the hole temporarily by ignoring DOM writes that occur during postponed updateWrapper(). In any case, they have no relation to actual calculations of wasted renders, as they don't occur due to updateComponent(), but rather due to onChange() special DOM behavior.

This fixes #5548
@sophiebits
Copy link
Contributor

Yeah, this seems okay. Thanks!

@gaearon gaearon merged commit 6a93137 into facebook:master Apr 14, 2016
@gaearon gaearon deleted the ignore-dom-writes-outside-batch branch April 25, 2016 16:47
@zpao zpao modified the milestones: 15.0.2, 15.0.x Apr 28, 2016
zpao pushed a commit that referenced this pull request Apr 28, 2016
Ignore DOM writes outside the batch in ReactPerf
(cherry picked from commit 6a93137)
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.

ReactDefaultPerf.printWasted() doesn't work
4 participants