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

Pert.start() leads to Uncaught TypeError: Cannot read property 'counts' of undefined #3389

Closed
ryanzec opened this Issue Mar 12, 2015 · 8 comments

Comments

Projects
None yet
6 participants
@ryanzec

ryanzec commented Mar 12, 2015

I wanted to see how each of my pages where performing so I looked to the Perf tool. I added the perf tool to the componentWillUpdate / componentDidUpdate on my main component (I assume that is the right place to put it, did not see any documentation anywhere on how to use Perf within ReactJS code) but that gives me this error:

Uncaught TypeError: Cannot read property 'counts' of undefined

The code looks like this:

https://gist.github.com/ryanzec/806db1aee328a5d847aa

I put console log's in the code and it never gets to the componentDidUpdate method. The error is happening here:

addValue(entry.counts, rootNodeID, 1);

Is this is bug? Am I use Perf incorrectly?

@ryanzec

This comment has been minimized.

Show comment
Hide comment
@ryanzec

ryanzec Mar 13, 2015

Any insight anyone can provide into this?

ryanzec commented Mar 13, 2015

Any insight anyone can provide into this?

@zpao

This comment has been minimized.

Show comment
Hide comment
@zpao

zpao Mar 13, 2015

Member

Taking a look through it, I think you need to call start outside of a render. Many of the measurements are based around update batches so a batch is already started by the time you call this. The internal state of Perf gets messed up (which is why something is undefined).

The original goal of perf is that it would be used to measure the whole app and then you could drill down. It is not the same as something like console.time/End.

Member

zpao commented Mar 13, 2015

Taking a look through it, I think you need to call start outside of a render. Many of the measurements are based around update batches so a batch is already started by the time you call this. The internal state of Perf gets messed up (which is why something is undefined).

The original goal of perf is that it would be used to measure the whole app and then you could drill down. It is not the same as something like console.time/End.

@zpao zpao closed this Mar 13, 2015

@also

This comment has been minimized.

Show comment
Hide comment
@also

also Mar 13, 2015

I don't understand why this is closed. Shouldn't it at least throw a helpful error? Throwing a type error seems very unlike React, especially for something that seems seems perfectly valid given the documentation.

Start/stop the measurement. The React operations in-between are recorded for analyses below. Operations that took an insignificant amount of time are ignored.

This makes it sound like console.profile() and console.profileEnd() which can be called at any time.

also commented Mar 13, 2015

I don't understand why this is closed. Shouldn't it at least throw a helpful error? Throwing a type error seems very unlike React, especially for something that seems seems perfectly valid given the documentation.

Start/stop the measurement. The React operations in-between are recorded for analyses below. Operations that took an insignificant amount of time are ignored.

This makes it sound like console.profile() and console.profileEnd() which can be called at any time.

@slorber

This comment has been minimized.

Show comment
Hide comment
@slorber

slorber Jul 21, 2015

Contributor

I've had this case just now, after playing with React-tappable lib (maybe related??? JedWatson/react-tappable#28)

I don't use Perf.start on the render phase and did not touch any perf code since 3 months.

Uncaught TypeError: Cannot read property 'counts' of undefined appLibs.js:51216
ReactDefaultPerf.measure appLibs.js:51216
ReactPerf.measure.wrapper appLibs.js:55139
ReactCompositeComponentMixin._updateRenderedComponent appLibs.js:48539
ReactCompositeComponentMixin._performComponentUpdate appLibs.js:48519
ReactCompositeComponentMixin.updateComponent appLibs.js:48435
ReactDefaultPerf.measure appLibs.js:51222
ReactPerf.measure.wrapper appLibs.js:55139
ReactCompositeComponentMixin.receiveComponent appLibs.js:48299
ReactReconciler.receiveComponent appLibs.js:56015
ReactCompositeComponentMixin.performUpdateIfNecessary appLibs.js:48317
ReactReconciler.performUpdateIfNecessary appLibs.js:56033
runBatchedUpdates appLibs.js:57855
Mixin.perform appLibs.js:59643
Mixin.perform appLibs.js:59643assign.perform appLibs.js:57799
flushBatchedUpdates appLibs.js:57879
ReactPerf.measure.wrapper appLibs.js:55141
Mixin.closeAll appLibs.js:59716
Mixin.perform appLibs.js:59657
ReactDefaultBatchingStrategy.batchedUpdates appLibs.js:50821
batchedUpdates appLibs.js:57814
ReactEventListener.dispatchEvent
Contributor

slorber commented Jul 21, 2015

I've had this case just now, after playing with React-tappable lib (maybe related??? JedWatson/react-tappable#28)

I don't use Perf.start on the render phase and did not touch any perf code since 3 months.

Uncaught TypeError: Cannot read property 'counts' of undefined appLibs.js:51216
ReactDefaultPerf.measure appLibs.js:51216
ReactPerf.measure.wrapper appLibs.js:55139
ReactCompositeComponentMixin._updateRenderedComponent appLibs.js:48539
ReactCompositeComponentMixin._performComponentUpdate appLibs.js:48519
ReactCompositeComponentMixin.updateComponent appLibs.js:48435
ReactDefaultPerf.measure appLibs.js:51222
ReactPerf.measure.wrapper appLibs.js:55139
ReactCompositeComponentMixin.receiveComponent appLibs.js:48299
ReactReconciler.receiveComponent appLibs.js:56015
ReactCompositeComponentMixin.performUpdateIfNecessary appLibs.js:48317
ReactReconciler.performUpdateIfNecessary appLibs.js:56033
runBatchedUpdates appLibs.js:57855
Mixin.perform appLibs.js:59643
Mixin.perform appLibs.js:59643assign.perform appLibs.js:57799
flushBatchedUpdates appLibs.js:57879
ReactPerf.measure.wrapper appLibs.js:55141
Mixin.closeAll appLibs.js:59716
Mixin.perform appLibs.js:59657
ReactDefaultBatchingStrategy.batchedUpdates appLibs.js:50821
batchedUpdates appLibs.js:57814
ReactEventListener.dispatchEvent
@jimfb

This comment has been minimized.

Show comment
Hide comment
@jimfb

jimfb Jul 21, 2015

Contributor

Yeah, that seems reasonable @also, I'm going to re-open this for the purpose of providing a more useful error message.

Contributor

jimfb commented Jul 21, 2015

Yeah, that seems reasonable @also, I'm going to re-open this for the purpose of providing a more useful error message.

@slorber

This comment has been minimized.

Show comment
Hide comment
@slorber

slorber Jul 23, 2015

Contributor

so @jimfb the problem with Perf I had in React-tappable is related to a bug in React-tappable.

Basically, this bug makes my app code do something like that:

this.setState({someState:"x",function() {
     Perf.start();
     triggerRendering(); 
     Perf.stop();
     showPerf();
})

The onTap listener is fired on the setState callback and produces weird behaviors with Perf

Just showing my bug so that you can provide a good error message :)

Contributor

slorber commented Jul 23, 2015

so @jimfb the problem with Perf I had in React-tappable is related to a bug in React-tappable.

Basically, this bug makes my app code do something like that:

this.setState({someState:"x",function() {
     Perf.start();
     triggerRendering(); 
     Perf.stop();
     showPerf();
})

The onTap listener is fired on the setState callback and produces weird behaviors with Perf

Just showing my bug so that you can provide a good error message :)

@gaearon gaearon referenced this issue Mar 12, 2016

Merged

Add new ReactPerf #6046

21 of 30 tasks complete
@gaearon

This comment has been minimized.

Show comment
Hide comment
@gaearon

gaearon Mar 18, 2016

Member

Removing “good first bug” because this code is being rewritten anyway.
Keeping this open to make sure we fix it in the new code.

Member

gaearon commented Mar 18, 2016

Removing “good first bug” because this code is being rewritten anyway.
Keeping this open to make sure we fix it in the new code.

@gaearon

This comment has been minimized.

Show comment
Hide comment
@gaearon

gaearon Oct 27, 2016

Member

AFAIK we already support starting/stopping outside of top level now.

Member

gaearon commented Oct 27, 2016

AFAIK we already support starting/stopping outside of top level now.

@gaearon gaearon closed this Oct 27, 2016

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