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

Enhance React DevTools "Why did this render?" for values nested in prop objects #16622

Closed
evbo opened this issue Aug 30, 2019 · 5 comments
Closed

Comments

@evbo
Copy link

evbo commented Aug 30, 2019

Do you want to request a feature or report a bug?
feature

What is the current behavior?
as demonstrated here, "Why did this render?" does a great job reporting what prop changed, but it does not yet report which nested value changed for props that are comprised of nested objects.

What is the expected behavior?
The "why did this render?" shows a collapsible tree with the "leaf" value that changed inside the prop object displayed.

A couple use cases this would benefit:
In some cases, it is most convenient creating props that are nested objects. For instance, maybe you need to pass an object to a library, and you'd like to avoid storing the individual object items as separate prop variables such that you don't need to redefine them together as a dict later on, but changes are due to a single element in the dict that you'd like visibility on in react devTools.

In rarer cases, it is unavoidable having props that aren't nested objects. For instance, how could I preserve the .prototype key of my props object without react stripping it? If I wrap my props inside an object, that key can be preserved. But now all my props are considered 1 prop to the profiler and I have no visibility on which prop changed. With this change, I could expand the tree and drill down to which individual values changed.

Which versions of React, and which browser / OS are affected by this issue? Did this work in previous versions of React?

New feature never before released in React DevTools. I am using 16.8.6

@bvaughn
Copy link
Contributor

bvaughn commented Aug 30, 2019

I understand why this feature would be useful, but I think deeply comparing all props/state/hooks/context values, for every components that renders in every commit, would add too much overhead during a performance sensitive time. (The more profiler negatively impacts performance, the less useful it is for the extreme performance cases- the ones that need profiling most.)

If you have a specific component that would benefit from a deep comparison, you might consider adding a custom one (in DEV bundle only) following an approach like maicki/why-did-you-update or welldone-software/why-did-you-render. (I have not used either library and I'm not endorsing the monkey patching of React components, just mentioning them as general references.)

Let's keep this conversation going if you have follow up questions! I'm going to close the GitHub issue though because I'm fairly confident this is not something we'll want to add to Profiler.

@evbo
Copy link
Author

evbo commented Aug 30, 2019

fair enough. And I presume something like a checkbox that by default disables this functionality (similar to how "why it rendered" is disabled by default) wouldn't be a simple enough work around to dodge performance issues?

I just see these problems as separable: what is slow and why it rendered could be answered in two separate recordings. Not ideal, but at least workable. I understand though if that doesn't align with the vision of this tool.

@bvaughn
Copy link
Contributor

bvaughn commented Aug 30, 2019

Making such a feature opt-in could avoid most of my performance concerns, yes- but then again, it's more complex to support configurable things and DevTools essentially has only a single, part-time maintainer.

I just see these problems as separable: what is slow and why it rendered could be answered in two separate recordings.

Agreed.

If we had a team of people working on DevTools, it might be worth looking into adding things like this into the UI given our current constraints. Fortunately, I think that if you the separate performance details of the profiler from the "why" then it's something that can be implemented in user space (like the libraries I linked above do). 😄

@sedghi
Copy link

sedghi commented Oct 1, 2021

@bvaughn I'm just wondering about your opinion on this after two years, is this feature on the roadmap by any chance?

@bvaughn
Copy link
Contributor

bvaughn commented Oct 2, 2021

No change in thoughts about this issue since my last comment. (It's not on any roadmap.)

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

3 participants