-
Notifications
You must be signed in to change notification settings - Fork 43
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
Show stack traces #43
Conversation
Copied over the UI and source transformation logic from CRA's react-error-overlay package.
I've just pushed a sizeable update to this branch, which reuses the stack parsing logic and stack trace UI components from However, the branch now has some issues:
The linting stuff can be fixed one way or another. My immediate concern is the Babel config. To get this to work, I updated the config to use I'd be happy to discuss the best way to get this fixed up and merged in ASAP. Please let me know what would be a good way to handle this. |
Thanks @markerikson. I'll test it and publish the extension tomorrow. Regarding the ui, you might give a try to |
@markerikson would you like to move it as a separate monitor and maintain it? See If you don't have time these days to work on that, I could merge it as is, but it won't be enabled by default, will add a message on how to enable it from client part. |
I published When there's no callstack, we have only Also please remove the code which is not meant for production, I see some |
@markerikson I moved the code to a separate package in reduxjs/redux-devtools#418 and added some tweaks. Mentioned you as the author and added credits in the README. Feel free to send a PR there with changes if any. I'll add tomorrow more details about how to check it in the related issue. |
Cool. Yeah, I've been really busy with Redux docs related stuff over the last couple months, so I haven't had time to think about this again. Thanks for poking at it :) |
Quick question: is this going to be enabled by default in the extension and instrumentation, or will most users need to do something to turn it on? |
Yes, it will be present on the extension / remotedev-app, but it should be enabled on the client side (like in the examples), and if necessary limiting the number of frames (not a problem in Chrome since it's limited to 10 on browser side, but could be on Firefox). We might enable by default in future if it won't impact the performance significantly. The problem is that most are including the extension and not using it, so it will affect the perf when not needed. I guess we'll just show a suggestion in that tab to set that option if no trace stack received. Also the editor name and project path should be added in the extension's options. |
When the underlying library is Redux, a "Stack" tab is now added after the "Diff" tab. When selected, it deserializes the
Error
that was captured when the original action was dispatched, formats the stack trace usingstacktrace-js
, and renders the resulting stack frames.This is a bare-bones implementation, and does not actually format the stack frame list beyond stringifying each frame entry. It also isn't filtering the list of stack frames at all, so there's frames from both the instrumentation and React in there.
Ideally, it would be great if we could filter out any frames that aren't part of the application source, and highlight the specific frame that actually triggered the dispatching sequence. But, this is a good first step - the data is being passed around, and we can do more formatting later.
See zalmoxisus/redux-devtools-extension#429 for the original request.