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

[bug] Problems of using react-addons-perf #6949

Closed
joseph1125 opened this issue Jun 2, 2016 · 39 comments
Closed

[bug] Problems of using react-addons-perf #6949

joseph1125 opened this issue Jun 2, 2016 · 39 comments
Assignees

Comments

@joseph1125
Copy link

When trying to use react-addons-perf@15.1.0 in Google Chrome, it is found that after Perf.start(), error messages displayed
Warning: There is an internal error in the React performance measurement code. Did not expect ctor timer to start while componentDidMount timer is still in progress for another instance.
Warning: There is an internal error in the React performance measurement code. We did not expect componentDidMount timer to stop while no timer is still in progress for another instance. Please report this as a bug in React.

After I typed Perf.stop() and try to use getWasted - this error message displayed
ReactPerf.js:239 Uncaught TypeError: Cannot read property 'updateCount' of undefined(…)

@aweary
Copy link
Contributor

aweary commented Jun 2, 2016

@joseph1125 can you share a JSFiddle reproducing the issue? You can use https://jsfiddle.net/reactjs/69z2wepo/ as a base.

@syranide
Copy link
Contributor

syranide commented Jun 2, 2016

#6842

@gaearon
Copy link
Collaborator

gaearon commented Jun 2, 2016

Yeah, fairly sure this is a duplicate. The fix will be out in the next patch release.

@nathanmarks
Copy link

nathanmarks commented Jul 5, 2016

@gaearon What's the status of this in 15.2.0?

I'm getting results back from Perf.getWasted(measurements) but I'm seeing this warning crop up too:

Warning: There is an internal error in the React performance measurement code. We did not expect componentDidMount timer to stop while no timer is still in progress for another instance. Please report this as a bug in React.

Here is the code: https://gist.github.com/nathanmarks/a284a8c9a8c926bcd79c7a2a93427dbc

And here is a fiddle which reproduces the warning (same component above converted to createClass syntax):
https://jsfiddle.net/69z2wepo/47766/

@gaearon
Copy link
Collaborator

gaearon commented Jul 5, 2016

Interesting. Generally we haven't tested ReactPerf in this scenario much. It is primarily meant to be used from browser console, not from a wrapping component.

We should support this scenario better. If it's not fixed in 15.2.0 it's probably a bug. Can you send a PR that adds similar case as a failing test to ReactPerf-test? The test will fall if there is an expected warning so merely triggering this behavior should be enough.

@gaearon
Copy link
Collaborator

gaearon commented Jul 6, 2016

@nathanmarks Can you please check if your TimeWaster works after my change in #7202? You can clone React, run npm run build and then copy build/modules into your node_modules/react/lib to test the effect.

@nathanmarks
Copy link

nathanmarks commented Jul 6, 2016

@gaearon Looks good on my end, warning is gone. Awesome 👍

@gaearon
Copy link
Collaborator

gaearon commented Jul 11, 2016

I think all of the cases related to this should be solved in 15.2.1.
If not, please let me know with a case reproducing the issue.

@gaearon gaearon closed this as completed Jul 11, 2016
@jdelafon
Copy link

jdelafon commented Jul 13, 2016

I don't know if it is the same issue, but with 15.2.1 I just got

warning.js:44 Warning: There is an internal error in the React performance measurement code. Did not expect componentDidMount timer to start while render timer is still in progress for another instance. 

It happened after I added this minimal code to a working boilerplate kit:

import { Component, PropTypes } from 'react';

class Panel extends Component {
    render() {
        return <div>...</div>;
    }
}

export default Panel;

and added <Panel /> inside the render method of another pre-existing component.

Edit: it goes away when instead I use

import React from 'react';
.... React.Component {

But still the error is confusing.

@gaearon
Copy link
Collaborator

gaearon commented Jul 13, 2016

@jdelafon There’s a ton of code there. I would rather avoid spending 15 minutes setting it up just to discover I can’t reproduce the issue. Would you mind extracting the relevant part that produces the warning as a separate GitHub project or a JSFiddle so I could inspect it?

@faergeek
Copy link

This warning is also shown when something throws in render. I'm pretty sure that it's not intended behavior.

@faergeek
Copy link

Unfortunately I can't reproduce it with minimal case. Probably there's something wrong in my setup :-(

@gaearon
Copy link
Collaborator

gaearon commented Jul 14, 2016

Check if you have duplicate React in the bundle (maybe some component contains a copy in its own node_modules by mistake).

@shobhitg
Copy link

I experienced this issue first thing while trying out gaearon/todos:

There is an internal error in the React performance measurement code. Did not expect componentDidMount timer to start while render timer is still in progress for another instance.

@gelobelo02
Copy link

gelobelo02 commented Jul 18, 2016

Experiencing this one too

 Warning: There is an internal error in the React performance measurement code. Did not expect componentDidMount timer to start while render timer is still in progress for another instance.

@jdelafon
Copy link

It is hard for us to isolate the problem without knowing where this warning comes from. It seems that ReactDebugTool is catching all kinds of errors, especially in render. It happens when there is really an error in my case, but it hides the real message. I don't have react-addons-perf in my package.json.

@gaearon
Copy link
Collaborator

gaearon commented Jul 18, 2016

It is hard for us to isolate the problem without knowing where this warning comes from. It seems that ReactDebugTool is catching all kinds of errors, especially in render.

There are no try-catch statements there. What likely happens is that you might have a Promise polyfill than doesn’t log its unhandled rejections. Therefore errors thrown inside render() get caught by that polyfill, and are never reported. Then React gets into an inconsistent state, and you see that warning (which is a symptom). But the real problem is the error thrown in your render() method, and you should make sure that whatever Promise polyfill you are using isn’t swallowing it.

I experienced this issue first thing while trying out gaearon/todos

Please provide exact steps to reproduce it. I cloned the project, ran npm install and npm start, and just don’t see any warnings.


To reiterate: I’m happy to reopen if this is a real issue but I can’t help without a project reproducing it.

@nathanmarks
Copy link

@gaearon I managed to reproduce it in gaearon/todos.

in src/components/VisibleTodoList.js, create a typo in mapStateToProps where getVisibleTodos() is called:

const mapStateToProps = (state) => {
  return {
    todos: getVisiblesTodos(state.todos, state.visibilityFilter), // typo creates a reference error
  };
};

Result:

warning.js:44 Warning: There is an internal error in the React performance measurement code. Did not expect componentDidMount timer to start while render timer is still in progress for another instance.warning
VisibleTodoList.js:36 Uncaught ReferenceError: getVisiblesTodos is not defined

@nathanmarks
Copy link

nathanmarks commented Jul 18, 2016

Just as an additional note -- I realise that this isn't hiding the message, but I personally haven't been able to reproduce this in a manner which completely hides the error message.

I haven't got this problem in my own application except when using HMR + react hot loader. In my own app, if I create a reference error in render and let it get hot reloaded, I get this error (HMR gives me the reference error). But if I do a complete refresh, I just see an unhandled in promise error in my console.

@nathanmarks
Copy link

Yeah, I'm not sure how to completely mask the error with this. By the way, is the perf tools warning intended to fire under these circumstances?

@gaearon
Copy link
Collaborator

gaearon commented Jul 18, 2016

@gaearon I managed to reproduce it in gaearon/todos.

This looks like correct behavior to me. The error is printed but it gets React into an inconsistent state.
We should probably rephrase to include something This might be caused by an error thrown in a component, in which case you should fix that error instead.

@gaearon
Copy link
Collaborator

gaearon commented Jul 18, 2016

Let’s reopen because we should definitely make this more user-friendly.
I’m open to suggestions here.

@nathanmarks
Copy link

Rephrasing is a good option. It's certainly correct behaviour given what happens, but the current message could be confusing to some.

One thing I haven't been able to reproduce is masking the original error. @jdelafon can you shed any light on how to reproduce this?

@jdelafon
Copy link

Apparently I did not notice the real error after the warning (I reproduced my example above and saw React is not defined after it). So it is fine, and rephrasing the warning should fix it.

@seanrucker
Copy link

Rephrasing seems like a good idea. I started seeing errors like this after upgrading to 15.2.1:

warning.js?8a56:44 Warning: There is an internal error in the React performance measurement code. Did not expect componentDidMount timer to start while render timer is still in progress for another instance.

I see now this is thrown anytime I have an error in one of my components (which happens frequently during development) but thought there was an additional problem due to the wording of the message.

@gaearon
Copy link
Collaborator

gaearon commented Jul 21, 2016

Got it. I think it was my mistake to enable this by default. We should change it to only be active in tests. I’ll get back to this soon.

@michaelsync
Copy link

I have the same error with 15.3.0 ..

Warning: There is an internal error in the React performance measurement code. Did not expect componentDidMount timer to start while render timer

@gaearon
Copy link
Collaborator

gaearon commented Aug 10, 2016

@michaelsync

Thanks for your comment, but this doesn’t help diagnose the issue in any way.
We already know that this warning exists.

Please read the comments above. There are two situations in which you can get this warning:

  • Your application code throws an error, and this warning is just a byproduct. In this case, look for real error close to it. (I agree we should fix this case to not output a warning, it’s on my todo list.)
  • There is an actual bug in React. In which case just saying you see this warning is not helpful. We will need to have an example reproducing it to fix the bug.

Thanks!

@tayiorbeii
Copy link

For anyone searching and finding this thread, I saw this warning when losing to a race condition arising from dispatching an async action in componentWillMount and trying to render before the data was available. Fixed it by adjusting my render logic.

@patrickgordon
Copy link

hey @tayiorbeii could you expand on this? I think I've got the same problem but not sure what I should be looking for...

@tayiorbeii
Copy link

Hey @patrickgordon, it's funny how long ago 9 days seems. I wish would I have been more clear in my previous comment. As I recall, my problem was along these lines:

const user = getUser(foo) || Map()
if (user) {
  // Stuff
}

The issue was I just looking at user being truthy and not having size > 0 (and therefore data from my call).

So in short, not a React issue. Look around your code for late night slip ups like that.

@gaearon
Copy link
Collaborator

gaearon commented Aug 24, 2016

Fixed via #7548. This will be out in the next release.

@gaearon
Copy link
Collaborator

gaearon commented Sep 19, 2016

Fixed in 15.3.2.

@d0ruk
Copy link

d0ruk commented Dec 3, 2016

Got the pls report as a bug request in my console, and here I am.

using 15.4.1

Warning appears when I use <iframe>.

There is a web audio object (as a component) as the children of that frame. Above the frame, there is static DOM.

@aweary
Copy link
Contributor

aweary commented Dec 3, 2016

@Norm- can you share a small test case reproducing the issue? You can use https://jsfiddle.net/reactjs/69z2wepo/ as a starting point

@d0ruk
Copy link

d0ruk commented Dec 3, 2016

@aweary

I probably should've investigated more before I made the comment. It was a mistake on my part. There was actually a TypeError before that React warning.

I use a web audio object inside an iframe. When the component (the only stateful one) holding the frame updates, the iframe component gets a new key, thus unmounts and a fresh Frame component is rendered.

The audio lib I am using (howler) requires that it should be initialized with an array of strings. I passed it null at one time and that's when I got the warning.

I can still reproduce it, if you want, but it would be broken code.

@gaearon
Copy link
Collaborator

gaearon commented Dec 4, 2016

A reproducing case would still be nice because I wouldn't expect this warning even if code throws after this fix.

@samsch
Copy link
Contributor

samsch commented Jun 8, 2017

I seem to have found another instance of this warning. It's triggered by a null reference inside a react-toolbox Dialog (similar to react-portal) child method. I built a test-case. It's not as minimal as I would like, but I wasn't able to recreate the bug in a couple hours in jsfiddle without react-toolbox.

Test case

@gaearon
Copy link
Collaborator

gaearon commented Jun 8, 2017

At this point it’s unlikely we’ll spend time fixing it, as we’re working on a rewrite that doesn’t include this addon. That said if you find an easy fix please send a PR.

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

No branches or pull requests