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

Profiler flamegraph could show icon/emoji for the re-render reasons. Making it easier to scan. #18014

Closed
bgirard opened this issue Feb 11, 2020 · 3 comments

Comments

@bgirard
Copy link
Contributor

bgirard commented Feb 11, 2020

Right now when working on a large project/component tree it can be difficult to understand why a large subtree is re-rendering. You click on each component and a reason: 1) Re-rendered because of the parent, 2) Re-rendered because a property changed, 3) Re-rendered because a hook changed. We don't get notified if a context value changed but ideally we would in the future.

If the flamegraph added icons we could perhaps spot patterns and more quickly identify what's going on when a large tree re-renders. For example state is changed (📚) at the top causing components to re-render via props (📋). Here's an example

📚Parent View
📋 TabStrip
📋 Page
📚Content
📋 Story

Here we can quickly spot that the state is changing in two component, and 3 components are changing because of prop. That might focus the investigation into why the state of Parent View / Content are changing which are likely causing the other props to change.

@bgirard bgirard added the Status: Unconfirmed A potential issue that we haven't yet confirmed as a bug label Feb 11, 2020
@bvaughn bvaughn added Component: Developer Tools and removed Status: Unconfirmed A potential issue that we haven't yet confirmed as a bug labels Feb 11, 2020
@M-Izadmehr
Copy link
Contributor

@bvaughn @bgirard,
I went on and added the tooltip summary of each fiber on hover, I feel it helps a lot for the overview, regardless, I still feel that emoji suggestion is also a good one, but the problem was that when I tried to implement it for the really small components in the chart it doesn't add any value. But we can still add it for the larger components in the chart.

@bgirard
Copy link
Contributor Author

bgirard commented Feb 17, 2020

I think the tooltip is a better idea overall. Your PR screenshot looks really nice!

the problem was that when I tried to implement it for the really small components in the chart it doesn't add any value

Me and Brian discussed that. If the component is small you can zoom in. If it's still small then it's probably not one that we care about anyways. We'd still get the visualization for the large components that matter the most.

But with your context menus we probably don't need this at all!

@bvaughn
Copy link
Contributor

bvaughn commented Mar 4, 2020

Now that we have tooltips, I don't think these icons are necessary anymore so I'm going to close this issue out. (I'm not a huge fan of the icon idea anyway, although the underlying need of getting info without zooming in makes sense!)

@bvaughn bvaughn closed this as completed Mar 4, 2020
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