Repository navigation
Use React-provided stacktrace in crash overlay #1746
Description
The recently merged crash overlay should support whitelisted warnings thrown by React.
Currently, this can be accomplished by enabling the console proxy via:
proxyConsole('error');However, this relies on a synthetically generated stack-trace and not the stack-trace provided by React in the warning. e.g.
var error = void 0;
try {
throw new Error(message);
} catch (e) {
error = e;
}This is problematic, however, because of how React works.
In React, components aren't rendered recursively, so there's never more than one component's render() on the stack
- @gaearon
This results in an unreconstructed tree of components specifying where the problem is stemming from.
The stack-trace from React is actually already parsed, and used to replace the line references on the synthetically generated stack-trace.
The proper behavior, however, would be to not generate a synthetic stack-trace and rely solely on the stack-trace given by React.
We realize this may be a daunting issue, but if you are up to the challenge, we will answer your every question!
Activity
Timer commentedon Mar 7, 2017
/cc @brandonmikeska who expressed interest in helping with the overlay
brandonmikeska commentedon Mar 7, 2017
Hey @Timer I am all for helping, but would need guidance on how to properly tackle this.
Timer commentedon Mar 7, 2017
Sure!
To start working on this feature, first uncomment https://github.com/facebookincubator/create-react-app/blob/ed5c016d6169f0bac7e29625afba1bf7bccac1d9/packages/react-dev-utils/crashOverlay.js#L823.
This will enable the
console.errorproxy, defined here: https://github.com/facebookincubator/create-react-app/blob/ed5c016d6169f0bac7e29625afba1bf7bccac1d9/packages/react-dev-utils/crashOverlay.js#L786-L821.However, our area of focus is to remove reliance on https://github.com/facebookincubator/create-react-app/blob/ed5c016d6169f0bac7e29625afba1bf7bccac1d9/packages/react-dev-utils/crashOverlay.js#L810-L815.
We want to create an error with the stack purely from the already extracted stack: https://github.com/facebookincubator/create-react-app/blob/ed5c016d6169f0bac7e29625afba1bf7bccac1d9/packages/react-dev-utils/crashOverlay.js#L794-L809.
However, due to the way this works the source resolving works, you're going to need to find a way to match these already-mapped files back to the bundled files (because of https://github.com/facebookincubator/create-react-app/blob/ed5c016d6169f0bac7e29625afba1bf7bccac1d9/packages/react-dev-utils/crashOverlay.js#L685-L691).
Now, you don't necessarily need to do the reverse of the sourcemap ... but perhaps propose a different solution. That is what this issue is about -- figuring out how to do this best.
Let me know if this requires changes to
stack-frame-resolver, and we can make it happen!gaearon commentedon Mar 7, 2017
FWIW, the way I imagined it is we have a middleware on the dev server running at
/debug-symbolicateor something like this. The error box can call it with the component stack trace from React warning. The middleware would look up the source lines around the locations sent by error box, and send the code in response. The error box would nicely format the code for each component stack frame and display it.brandonmikeska commentedon Mar 10, 2017
Hmm ok! I have not implemented a middleware before, but I wouldn't mind learning if someone has time for a screen share or documentation links that I can read through to get something started.
n3tr commentedon Mar 10, 2017
@brandonmikeska
Since webpack-dev-server is an Express app, you can start by looking at Express's middleware document: https://expressjs.com/en/guide/using-middleware.html and webpack-dev-server's api document: https://webpack.github.io/docs/webpack-dev-server.html#api
We create a webpack-dev-server (which is express app) inside start script:
https://github.com/facebookincubator/create-react-app/blob/76680ae7ebbabaa91eb11a3e593cadebaf77921a/packages/react-scripts/scripts/start.js#L73
Then apply middleware from:
https://github.com/facebookincubator/create-react-app/blob/76680ae7ebbabaa91eb11a3e593cadebaf77921a/packages/react-scripts/scripts/start.js#L76
https://github.com/facebookincubator/create-react-app/blob/master/packages/react-scripts/scripts/utils/addWebpackMiddleware.js
hope this help :)
Timer commentedon Mar 13, 2017
@brandonmikeska we should probably start with #1322, would you like to tackle that issue? 😄
It'll make mapping files to system files easier.
Timer commentedon Mar 14, 2017
I've updated
stack-frameto support this use-case.This should be relatively trivial now. 😄
Timer commentedon Mar 14, 2017
this is a crude POC
brandonmikeska commentedon Mar 14, 2017
@Timer I can look at #1322 but it seems like you already have a proposed solution. Do you still want me to start there?
Timer commentedon Mar 14, 2017
Yes, please! #1322 needs addressing -- I've explored how to implement the solution but not which path base is best for the solution itself.
If you can figure out the best base path (whether it be appBase or appSrc or appBuild, etc) and send a PR that'd be great. Feel more than free to copy my little snippet.
We'll need something for production, too.
Timer commentedon May 14, 2017
This has been completed across multiple PRs. 😄