-
-
Notifications
You must be signed in to change notification settings - Fork 2k
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
feat(store-devtools): add redux dev tool trace support (#3517) #3665
Conversation
✅ Deploy Preview for ngrx-io canceled.Built without sensitive environment variables
|
This comment was marked as resolved.
This comment was marked as resolved.
@brandonroberts @timdeschryver I created PR related to #3517, could you help me review it. And this is my first PR, if you have any questions or comments, please let me know. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi, thanks for creating this PR.
I do have two questions.
- I added the
trace: true
config to our example app (npm run example:start
), but the traces weren't coming through. I will take a closer look at this later, but could you reverify if it works on your end please? (StoreDevtoolsModule.instrument({
- What's the reason why the tracelimit is sometimes set to 20?
Thanks for review.
@timdeschryver add GIF below, in my example, I set the trace limit to 50, a small number may not be able show the full stack trace.. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the elaboration, I left a few comments.
Co-authored-by: Tim Deschryver <[email protected]>
Co-authored-by: Tim Deschryver <[email protected]>
Co-authored-by: Tim Deschryver <[email protected]>
Co-authored-by: Tim Deschryver <[email protected]>
Co-authored-by: Tim Deschryver <[email protected]>
These comments make sense to me. And apply the suggested change. Thanks. |
@timdeschryver anything I can do for getting one more reviewer? |
Thanks @zizifn! |
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
Add redux trace option support into dev tool, related to (#3517)
Below is the redux doc for
trace
option.https:/reduxjs/redux-devtools/blob/main/extension/docs/API/Arguments.md#trace
What is the current behavior?
No trace shown in the redux dev tools extension.
Closes #3517 #1868
What is the new behavior?
Trace shown in the redux dev tools extension if enable.
Does this PR introduce a breaking change?
Other information