[FB-only] Show which hooks (indices) changed when profiling

[bvaughn]

I don't think this feature is broadly useful or user friendly, but @bgirard and I chatted about it and he said that it would significantly speed up his performance investigation process. I have enabled it only for Facebook DevTools builds (using the feature flag system I recently added). This is the first feature to actually use the flags, so I also had to tweak a few things to make it work like it should.

Note this doesn't impact either build unless the "Record why each component rendered while profiling." setting is enabled:

When the feature flag and the setting are enabled, selecting an element in a profile will show which hooks have changed:

Since the Profiler can't display the names of the changed hooks (it would be too heavy to compute this at runtime) the numbers will need to be matched up to the selected component:

Fortunately the selection is synced between these two panels which should make matching at least somewhat faster.

[sizebot]

Comparing: 1d1e49c...826a5da

Critical size changes

Includes critical production bundles, as well as any change greater than 2%:

Name	+/-	Base	Current	+/- gzip	Base gzip	Current gzip
oss-stable/react-dom/cjs/react-dom.production.min.js	=	122.85 kB	122.85 kB	=	39.53 kB	39.54 kB
oss-experimental/react-dom/cjs/react-dom.production.min.js	=	129.35 kB	129.35 kB	=	41.60 kB	41.60 kB
facebook-www/ReactDOM-prod.classic.js	=	408.73 kB	408.73 kB	=	75.80 kB	75.80 kB
facebook-www/ReactDOM-prod.modern.js	=	396.99 kB	396.99 kB	=	73.86 kB	73.86 kB
facebook-www/ReactDOMForked-prod.classic.js	=	408.74 kB	408.74 kB	=	75.80 kB	75.80 kB

Significant size changes

Includes any change greater than 0.2%:

(No significant changes)

Generated by 🚫 dangerJS against 826a5da

[bvaughn]

This command is only used by the internal FB build tool to create the Chef-installed extension. So this change will cause that build to use the FB feature flags file.

[bvaughn]

This was an accidental omission from when I added the flags files.

[bvaughn]

I dunno is this overkill? Maybe I should just say e.g. "0,2,5"

[bgirard]

This looks great. Looking forward to see if this speeds up pin-pointing the root cause of a re-render. Faster workflow here will make optimizations worthwhile.

Nice follow-up for people encountering this feature that isn't me or you might be a short explanation how to map the hook # to specific hooks. Maybe a tooltip that says:

The hook number represent the primitive (non-user) hooks as seen by React. You can look it up using the React Inspector.

[eps1lon]

Nice follow-up for people encountering this feature that isn't me or you might be a short explanation how to map the hook # to specific hooks.

And maybe add the hook number directly into the inspector. In a component with custom hooks counting in your head might not be the easiest task.

Fortunately the selection is synced between these two panels which should make matching at least somewhat faster.

I originally thought the highlighting in your screenshots was from devtools but I don't believe that is the case? That would really help though for components with custom hooks.

[bvaughn]

And maybe add the hook number directly into the inspector. In a component with custom hooks counting in your head might not be the easiest task.

This is a neat idea 👍🏼

I originally thought the highlighting in your screenshots was from devtools but I don't believe that is the case? That would really help though for components with custom hooks.

I don't think this would work. Which hooks change are commit-specific. The components tab isn't.

[bvaughn]

And maybe add the hook number directly into the inspector. In a component with custom hooks counting in your head might not be the easiest task.

@bgirard How about something subtle like the numbers beside the primitive hooks in this:

[bgirard]

This looks great. It's going to be even easier to chase this down!

Small suggestions: It's a bit confusing seeing the numbers on either side. Perhaps a small icon like #0, #1 and maybe fade it a tiny bit to focus the attention on the hook value and not the hook number.

[bvaughn]

Thanks for the feedback, @bgirard.

Small suggestions: It's a bit confusing seeing the numbers on either side. Perhaps a small icon like #0, #1 and maybe fade it a tiny bit to focus the attention on the hook value and not the hook number.

Is prefixing the number with a "#" any better?

The number is already faded by the way. We could dim it more but it's maybe a bit hard to read?

I could style them a little differently, more like "badges" but then they take up more horizontal space:

Maybe without the "#" helps a little:

I think I'll try to move forward with this last one.

I think putting the numbers on the right is a little worse b'c it makes them further away from the hook name (with wider screens especially).

[bvaughn]

Going to move forward with this so I can start testing it internally.

[bgirard]

This looks great! I think not having names* won’t be a big issue with this design.

Let’s see how this goes!

[bvaughn]

Great! Thanks for the feedback :)

[markerikson]

Yeah, while there's obviously a lot of demand for "named hooks", this would at least help give an indication of what's changed as a stopgap.

Would it be possible to have the "1, 4, and 5 changed" part be expandable to give more details? At a minimum:

- #1: State ("someValue")
- #4: Reducer ({a: 42})
- #5: State (123)

Even niftier, it'd be neat to see the parent custom hooks path listed, like:

- #4: Selector -> SelectorWithStoreAndSubscription -> Reducer (123)

[bvaughn]

Would it be possible to have the "1, 4, and 5 changed" part be expandable to give more details? At a minimum:

Unfortunately, no. Serializing (and storing) values would add a lot of overhead.

And even getting the "name" of the hook (e.g. State let alone SelectorWithStoreAndSubscription -> Reducer) would be too slow because it would require rendering the component to detect its hooks like we do on inspection. Fine to do one component at a time while inspecting. Not good to do for every component during profiling.

[markerikson]

Awwwww :( too bad. was worth asking, though :)

[bvaughn]

What we have to do to detect hooks and values is pretty heavy:
https://github.com/facebook/react/blob/master/packages/react-debug-tools/src/ReactDebugHooks.js#L590-L615

[KamranAsif]

It would be useful to add index numbers to the hooks listed in a component page. I see that hook #11 changed and I have to manually count 11 hooks for that component.

[bvaughn]

Comments above show that this is already being done, unless I misunderstand you

[KamranAsif]

I had assumed it was counting the top level hooks only. Just turned out it was a coincidence that the top level nth hook was also in the list.

[mrkev]

I don't think this feature is broadly useful, but Benoit has asked for it to help with his performance investigations so I have enabled it only for Facebook DevTools builds.

Can confirm this feature is incredibly useful for performance investigations— I spent a few minutes building this from source and it unblocked me from an issue on which I had spent hours.

As a tangent, I find it interesting that some hooks share indices. The few times I've encountered this, the Context hooks are coalesced alongside the first non-Context hook as number 1. If anyone who reads this comment knows where I can learn more about how this works, I'd appreciate a pointer (:

[gaearon]

I think it’s because we don’t really track context usage as a Hook internally because it’s not stateful at the consuming side. However, it’s a Hook in the API so that we have the option to add some hidden state to it later if needed.

[opsidao]

EDIT: Nevermind, I was able to make it work, sorry for the noise 👼

Sorry for chiming in, could somebody please give me a short hint as to how I could use this in my project?

We're having some issues with unexpected rerenders because "hooks changed" and are having a hard time debugging them so, this could be really useful to debug that.

I tried compiling react (for the first time) with the enableProfilerChangedHookIndices set to true, but that doesn't seem to have any effect. I followed the instructions on the devtools-extension README to create an experimental build but I had multiple issues (the build is not a development build by default, the extension throws errors when executed and actually doesn't show the reasons for the rerenders at all, etc...).

Any help would be very much appreciated.

[mrkev]

Hey @bgirard, are you using this feature at your new job? I definitely am 😅

Can we re-visit the decision of making this FB-only? If this isn't added to the store version because it seems unintuitive, I'd press for putting it behind a toggle-able runtime setting instead of a built-time flag. In my experience, the people who use the profiler in the first place could be considered "advanced users" who would either know what this means, or wouldn't turn away from seeing this in the tools even if they don't. If we don't consider this to be the case, it'd still be nice to have it there for the users who are advanced and want or need this feature.

I'm not sure what my timeline looks like, but on a day without much going on I'd be happy to invest a couple hours to put this outside the flag. I'm ok running off my own built version of the tools for now, but I know you're constantly improving the tools @bvaughn, and it'd be nice to get updates from the store as they come (:

If there's another reason why this should remain FB-only though, do let me know

[bvaughn]

@mrkev I've been think it's probably a good thing to enable the flag for open source as well. You want to post a PR for that?

react/packages/react-devtools-shared/src/config/DevToolsFeatureFlags.extension-oss.js

Line 16 in 4225133

export const enableProfilerChangedHookIndices = false;

[mrkev]

Sure, sending soon 👍

[mrkev]

Sent in #22011— it just turns on the flag for OSS builds. We can discuss there if we want any further changes (a setting, or to clean up the flag too, etc).

[KevinPoole]

Hey @bvaughn , very cool addition to the react dev tools. Wondering how hard it would be to add one small addition:

• Be able to recursively expand / collapse all hooks (or all hooks in a given subtree). Possibly as an option when you right click or something?

In my case, I am finding that many of my hooks are deeply nested within what is displayed in the hooks section of a component (for example, ones resulting from useQuery / useMutation of react-query fame). It would be a huge help to be able to expand all of the hooks more efficiently, or perhaps better yet, for them to default "opened" in such a way that all numbered hooks are at least exposed.

Just food for thought... Thanks!

[bvaughn]

Happy to review a PR if you'd like to put together a proposal? :)

[KevinPoole]

Yeah. fair enough :-) ...

[sepsol]

But how should one lookup a hook index using the search tool?