-
Notifications
You must be signed in to change notification settings - Fork 46.9k
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: devtools Profiler causes unexpected errors #19911
Comments
Thanks for the report. I'll look at it sometime soon. :) |
@bvaughn Shall I work on a pull request for this issue? |
@ashr81 If you'd like! |
@bvaughn Yes, I like to contribute. |
I'm going to look into this this afternoon! :) |
I'm not sure I understand yet what Profiling or Edit I can reproduce this on Code Sandbox itself, just not locally. It's possible that this Code Sandbox is pinned to using Edit 2 I have a bit more context now. Still looking though. |
You should see the bug when you open codesandbox in seperate tab in browser and start profiling. |
Profiling using same |
I see the problem here. Logging a warning during render causes DevTools (if configured to "append component stacks" to warnings) to do its own shallow render of a component. The problem is that the DevTools overrides the dispatcher before shallow rendering to prevent this from happening: react/packages/react-devtools-shared/src/backend/DevToolsComponentStackFrame.js Lines 88 to 95 in e614e69
That being said, looking in the Chrome debugger– the source I'm seeing does not do that: function describeNativeComponentFrame(fn, construct, currentDispatcherRef) {
// If something asked for a stack inside a fake render, it should get ignored.
if (!fn || reentry) {
return '';
}
if (false) {}
let control;
const previousPrepareStackTrace = Error.prepareStackTrace; // $FlowFixMe It does accept undefined.
Error.prepareStackTrace = undefined;
reentry = true;
let previousDispatcher;
// ... This reason for this is that I'm kind of clowny and didn't take into account the fact that the DevTools extension itself would be running in production mode, even if the application code (that logs warnings) is in DEV mode. |
Feel like testing this (local) build of DevTools and seeing if it fixes the problem for you? |
It's working fine now. tested it using the generated link on pull request you raised. |
Thank you for confirming, and for the repro case. I'll try go get an update (with the fix) pushed to the browser stores shortly. |
We noticed that our app would behave differently during profiling runs and trigger errors. I'm not totally sure what the underlying issue is but I was able to put together a example app to reproduce. As far as I can tell it has to do with how devtools is overriding
console.warn
andconsole.error
. In that casedescribeNativeComponentFrame()
will call a function component with no args. This works fine as the error is caught indescribeNativeComponentFrame()
but in it looks like auseEffect()
that accesses thoseprops
is still triggered and it does not expectprops
to be undefined.I realize that having
props
in the dependencies array of theuseEffect
doesn't really make sense but I still think it probably shouldn't error.React version: 16.13.1
React devtools version: 4.8.2
Steps To Reproduce
<h1>Hello World</h1>
Uncaught TypeError: Cannot read property 'foo' of undefined
Link to code example:
https://codesandbox.io/s/cool-sun-wdrry
The current behavior
The expected behavior
The app should work as it while not profile. It should render a
<h1>Hello World</h1>
The text was updated successfully, but these errors were encountered: