-
Notifications
You must be signed in to change notification settings - Fork 51.2k
Improve the detection of changed hooks #35123
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
Merged
hoxyq
merged 17 commits into
react:main
from
blazejkustra:fix/devtools-hook-indexes-mismatch
Jan 15, 2026
Merged
Changes from 10 commits
Commits
Show all changes
17 commits
Select commit
Hold shift + click to select a range
4e09f54
Improve the detection of changed hooks
blazejkustra 7120a95
Add 'FormState' to the list of editable state names in hook detection
blazejkustra b77efa4
Fix lint
blazejkustra 7371cfc
Fix tests 4/10
blazejkustra de00387
Use dispatcherHookName instead of name
blazejkustra ddf0277
Update hook detection to use 'name' property instead of 'dispatcherHo…
blazejkustra f565055
Refactor hook inspection to simplify detection logic
blazejkustra c0ec480
Merge branch 'main' of github.com:facebook/react into fix/devtools-ho…
blazejkustra 2cae0a1
Merge branch 'main' of github.com:facebook/react into fix/devtools-ho…
blazejkustra 7a68cae
Fix profiling cache tests to preserve real dispatch and setState refe…
blazejkustra ac421e0
Merge branch 'main' of github.com:facebook/react into fix/devtools-ho…
blazejkustra 82f5ede
Removed flattenHooksTree function and replaced it with a recursive tr…
blazejkustra 78545f2
Fix hook traversal logic
blazejkustra 3df7554
Remove potentially unreachable code
blazejkustra 4b7295b
Add test for detecting changes in custom and composite hooks in Profi…
blazejkustra 2ee5680
Remove unnecessary optional chaining
blazejkustra 6a9c7bd
Update optional chaining for subhooks
blazejkustra File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Is there a reason why this specific order is chosen?
This looks like subHooks wills have lower indexes in an array, when flattened. In the UI, the hook numbers (index + 1) usually reflect the tree structure. Basically
indexOf(parent) < indexOf(parent.child)is always true.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.
flattenHooksTreefunction performs a depth-first traversal that only keeps leaf hooks (those withoutsubHooks). This means custom hooks are unwrapped to primitive hooks.For example, given this tree:
The flattened result is:
This order matches React's hooks order, right? So comparing
prevFlattened[i]withnextFlattened[i]identifies which primitive hooks changed between renders.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.
If we were to change the traversal order the indexes wouldn't be correct I believe, wdyt @hoxyq?
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 clarifying. I was confused a bit, but that is not an issue, since we only assign indexes to built-in hooks, which should be leaf nodes by definition.
In your example above, I believe this will be the result of flattening:
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.
Do you think it still makes sense to do flattening? We could probably just do dfs on both hook trees at the same time.
Uh oh!
There was an error while loading. Please reload this page.
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.
I think custom hook would not be there since it is omitted by the
continuestatement: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.
Ahaa, so instead of building
prevFlattenedandnextFlattenedI could do it in place? Good idea, let me try it 👀