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

Make ReactPerf.getLastMeasurements() opaque #6286

Merged
merged 1 commit into from
Mar 17, 2016
Merged

Make ReactPerf.getLastMeasurements() opaque #6286

merged 1 commit into from
Mar 17, 2016

Conversation

gaearon
Copy link
Collaborator

@gaearon gaearon commented Mar 17, 2016

This is a temporary change I want to get into 15.0.

We intend to change the measurement format significantly in #6046. In order to introduce the new ReactPerf with a different and more detailed measurement format during 15.x release cycle, I propose to make the measurement structure opaque-ish in 15.0.

I searched the GitHub for usage of ReactPerf.getLastMeasurements() and couldn’t find any popular projects relying on its internal structure except React Native in this single module. I left __unstable_this_format_will_change as a workaround in case RN makes a release with 15.0 before we ship 15.1 with the new ReactPerf, or if anyone else really needs reading those measurements in the raw form.

In any case I will be working on updating React Native to use the new ReactPerf later so I don’t think we should be held back by this. It’s likely that by the time React Native is ready to switch to 15, the new ReactPerf will be ready, but if not, __unstable_this_format_will_change covers their use case.

All existing methods continue to work with the measurements returned from getLastMeasurements() so we aren’t breaking the contract. We’re just signaling that the internal format of measurements is an implementation detail in 15.0 until we iron it out and present the new, more detailed one, in 15.1.

Test Plan

Verify that ReactPerf.printExclusive(), ReactPerf.printInclusive(), ReactPerf.printWasted(), ReactPerf.printDOM() still work both with zero arguments and with a single argument obtained from ReactPerf.getLastMeasurements(). Verify that ReactPerf.getMeasurementsSummaryMap() still works with a single argument.

Reviewers

@sebmarkbage

We intend to change the measurement format significantly in #6046. In order to introduce the new ReactPerf during 15.x release cycle, we are making the measurement structure opaque-ish in 15.0.
@sebmarkbage
Copy link
Collaborator

Do we have reason to believe that there are tools out there relying on this?

@gaearon
Copy link
Collaborator Author

gaearon commented Mar 17, 2016

I found these:

Nothing popular as far as I see. However people do rely on the ability to pass those measurements back to the ReactPerf methods for things like CI (#2219).

@gaearon gaearon self-assigned this Mar 17, 2016
gaearon added a commit that referenced this pull request Mar 17, 2016
Make ReactPerf.getLastMeasurements() opaque
@gaearon gaearon merged commit 67647fd into facebook:master Mar 17, 2016
@gaearon gaearon deleted the opaque-perf-measurements branch March 17, 2016 19:25
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