Skip to content

Fix indices of hooks in devtools when using useSyncExternalStore #34547

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.

Sign up for GitHub

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 4 commits into from
Oct 21, 2025
Merged

Fix indices of hooks in devtools when using useSyncExternalStore #34547

Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
16e77a3
Fix indices of hooks ind evtools when using useSyncExternalStore
blazejkustra Sep 20, 2025
0dd8c60
Clean the code a bit
blazejkustra Sep 20, 2025
bd16801
Move back line with increment
blazejkustra Oct 15, 2025
aa0e125
Remove next.next presence check in traversal logic
blazejkustra Oct 15, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 26 additions & 7 deletions packages/react-devtools-shared/src/backend/fiber/renderer.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1908,6 +1908,20 @@ export function attach(
return false;
}

function isUseSyncExternalStoreHook(hookObject: any): boolean {
const queue = hookObject.queue;
if (!queue) {
return false;
}

const boundHasOwnProperty = hasOwnProperty.bind(queue);
return (
boundHasOwnProperty('value') &&
boundHasOwnProperty('getSnapshot') &&
typeof queue.getSnapshot === 'function'
);
}

function isHookThatCanScheduleUpdate(hookObject: any) {
const queue = hookObject.queue;
if (!queue) {
Expand All @@ -1924,12 +1938,7 @@ export function attach(
return true;
}

// Detect useSyncExternalStore()
return (
boundHasOwnProperty('value') &&
boundHasOwnProperty('getSnapshot') &&
typeof queue.getSnapshot === 'function'
);
return isUseSyncExternalStoreHook(hookObject);
}

function didStatefulHookChange(prev: any, next: any): boolean {
Expand All @@ -1950,13 +1959,23 @@ export function attach(

const indices = [];
let index = 0;

while (next !== null) {
if (didStatefulHookChange(prev, next)) {
indices.push(index);
}

// useSyncExternalStore creates 2 internal hooks, but we only count it as 1 user-facing hook
if (isUseSyncExternalStoreHook(next)) {
if (next.next !== null) {
next = next.next;
prev = prev.next;
}

@hoxyq hoxyq Oct 15, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We probably don't want to check on next.next presence here, it should be implied if the hook object is actually an internal representation of useSyncExternalStore().

I would prefer it to crash instead, if this ever happens, so we spot the gap in our hook parsing logic first.

@blazejkustra blazejkustra Oct 15, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense to me as well!

}

index++;

@hoxyq hoxyq Oct 15, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why did you move this increment there?

@blazejkustra blazejkustra Oct 15, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By mistake, moving it back 👍

next = next.next;
prev = prev.next;
index++;
}

return indices;
Expand Down
Loading