Perf.print*() method not working in 15.1.0: Cannot read property 'forEach' of undefined

[EvHaus]

After upgrading to 15.1.0 I'm having trouble using the React Perf tools. I've done the following:

Added to one of my files:

import Perf from 'react-addons-perf';
window.Perf = Perf;

I then launch my app in Chrome. Open the Dev Tools and run this in the console:

Perf.start();
Perf.stop();
Perf.printWasted();

And I get:

TypeError: Cannot read property 'forEach' of undefined
getWasted   @   7.7.js:149515
printWasted @   7.7.js:149666
(anonymous function)    @   VM508:1

The failure is on this line: flushHistory.forEach(function (flush) {.

The same behaviour occurs for all other print* method such as printInclusive() and printExclusive().
The same behaviour occurs if I put the commands into my code (ie. NOT running in the Chrome Console).

[gaearon]

This probably happens because you’re running a production build of React, but ReactPerf only works in the development build.

We need to make it clear by emitting a warning instead of crashing.

[EvHaus]

Ah. Yes, I am indeed running Perf in production mode as I wanted to avoid PropType validation getting in the way of profiling.

[gaearon]

We’ll allow this in the future, but it isn’t possible at the moment. You can track #6627.

[gaearon]

If someone wants to take this, here’s what I’d do:

1. Make ReactDebugTool.getFlushHistory() return flushHistory regardless of the environment
2. Add warnings and early returns to all methods in ReactPerf when __DEV__ is false

[sashashakun]

@gaearon Hi, I will try to fix this!

[natenorberg]

Dang, just beat me to it

[gaearon]

@sashashakun Thanks! I put myself as assignee so I don’t forget to come back to it, but please work on it and feel free to ask any questions!

[sashashakun]

@gaearon Great! Issue looks not difficult, but I want to do my best, so already started to work and because I don't have deep knowledge about React internals, have some questions:

About this point:

Add warnings and early returns to all methods in ReactPerf when DEV is false

1. I found 12 not deprecated methods in ReactPerf, so if I will simple add thing like

if (!__DEV__) {
  warning('some warning message here');
  return false;
}

at the beginning of each method it will be to much copy-paste, no? May be it will be better to make a separate helper, like a roundFloat in the beginning of ReactPerf.js?
2) What warning message I should use?
3) What need to return, false or may be 0?
4) One of the methods of ReactPerf use ReactDebugTool.getFlushHistory(), which i fixed this way
Make ReactDebugTool.getFlushHistory() return flushHistory regardless of the environment, so should I add __DEV__ checking from your first point to this method also?

[gaearon]

May be it will be better to make a separate helper, like a roundFloat in the beginning of ReactPerf.js?

Sure, sounds good.

2. What warning message I should use?

ReactPerf is not supported in the production builds of React. To collect measurements, please use the development build of React instead.

3. What need to return, false or may be 0?

I think print*() methods can just exit early without a return value (just like they usually do anyway). get*() methods can probably return empty arrays. I suggest using whatever return type they usually have, but without any information.

so should I add DEV checking from your first point to this method also?

Worth adding it to log the warning, but the return value should be an empty array anyway.

Finally: I think we should make sure we don’t warn more than once. If somebody has ReactPerf.start() and ReactPerf.stop() calls around some hot function we don’t want to spam the console in production.

[sashashakun]

Start my work in branch react-perf-warnings-changes:

Sure, sounds good.
Done at ReactPerf.js#L23.

ReactPerf is not supported in the production builds of React. To collect measurements, please use the development build of React instead.
Added at ReactPerf.js#L26.

I suggest using whatever return type they usually have, but without any information.
Added at ReactPerf.js#L23. Value for return passed as param;

Worth adding it to log the warning, but the return value should be an empty array anyway
Added at ReactPerf.js#L34

Finally: I think we should make sure we don’t warn more than once.
Added variable 'alreadyWarned' which set turn off warning after first one and rollback after calling ReactPerf.stop()

Make ReactDebugTool.getFlushHistory() return flushHistory regardless of the environment
Done at ReactDebugTool.js#L210

Also, about steps before open PR:

• 1. Fork the repo and create your branch from master.
• 2. If you've added code that should be tested, add tests! - need to investigate
• 3. If you've changed APIs, update the documentation. - need to investigate
• 4. Ensure the test suite passes (grunt test).
• 5. Make sure your code lints (grunt lint) - we've done our best to make sure these rules match our internal linting guidelines.
• 6. If you haven't already, complete the CLA.

Looking forward for your feedback.

[gaearon]

I suggest let’s open a PR now so I don’t forget to look into it?
Thanks a bunch!

[sophiebits]

Note that warning() is a noop in prod builds.

[gaearon]

Haha, oops, I didn’t realize that 😞 .
Do you have any preferences about how to tackle this?

[sophiebits]

Let's just do a console.error.

[sashashakun]

Ok, will fix this