Improve the detection of changed hooks by everettbu · Pull Request #191 · greptileai/react-mirror

everettbu · 2025-12-12T23:45:50Z

Mirror of facebook/react#35123
Original author: blazejkustra

Summary

cc @hoxyq

Fixes react/react#28584. Follow up to PR: react/react#34547

This PR updates getChangedHooksIndices to account for the fact that useSyncExternalStore, useTransition, useActionState, useFormState internally mounts more than one hook while DevTools should treat it as a single user-facing hook.

Approach idea came from this comment 😄

Before:

QuickTime.movie.2.mov

After:

QuickTime.movie.mov

How did you test this change?

I used this component to reproduce this issue locally (I followed instructions in packages/react-devtools/CONTRIBUTING.md).

Details

import * as React from 'react';

function useDeepNestedHook() {
  React.useState(0); // 1
  return React.useState(1); // 2
}

function useNestedHook() {
  const deepState = useDeepNestedHook();
  React.useState(2); // 3
  React.useState(3); // 4

  return deepState;
}

// Create a simple store for useSyncExternalStore
function createStore(initialValue) {
  let value = initialValue;
  const listeners = new Set();
  return {
    getSnapshot: () => value,
    subscribe: listener => {
      listeners.add(listener);
      return () => {
        listeners.delete(listener);
      };
    },
    update: newValue => {
      value = newValue;
      listeners.forEach(listener => listener());
    },
  };
}

const syncExternalStore = createStore(0);

export default function InspectableElements(): React.Node {
  const [nestedState, setNestedState] = useNestedHook();

  // 5
  const syncExternalValue = React.useSyncExternalStore(
    syncExternalStore.subscribe,
    syncExternalStore.getSnapshot,
  );

  // 6
  const [isPending, startTransition] = React.useTransition();

  // 7
  const [formState, formAction, formPending] = React.useActionState(
    async (prevState, formData) => {
      return {count: (prevState?.count || 0) + 1};
    },
    {count: 0},
  );

  const handleTransition = () => {
    startTransition(() => {
      setState(Math.random());
    });
  };

  // 8
  const [state, setState] = React.useState('test');

  return (
    <>
      <div
        style={{
          padding: '20px',
          display: 'flex',
          flexDirection: 'column',
          gap: '10px',
        }}>
        <div
          onClick={() => setNestedState(Math.random())}
          style={{backgroundColor: 'red', padding: '10px', cursor: 'pointer'}}>
          State: {nestedState}
        </div>

        <button onClick={handleTransition} style={{padding: '10px'}}>
          Trigger Transition {isPending ? '(pending...)' : ''}
        </button>

        <div style={{display: 'flex', gap: '10px', alignItems: 'center'}}>
          <button
            onClick={() => syncExternalStore.update(syncExternalValue + 1)}
            style={{padding: '10px'}}>
            Trigger useSyncExternalStore
          </button>
          <span>Value: {syncExternalValue}</span>
        </div>

        <form
          action={formAction}
          style={{display: 'flex', gap: '10px', alignItems: 'center'}}>
          <button
            type="submit"
            style={{padding: '10px'}}
            disabled={formPending}>
            Trigger useFormState {formPending ? '(pending...)' : ''}
          </button>
          <span>Count: {formState.count}</span>
        </form>

        <div
          onClick={() => setState(Math.random())}
          style={{backgroundColor: 'red', padding: '10px', cursor: 'pointer'}}>
          State: {state}
        </div>
      </div>
    </>
  );
}

…okName'

…ok-indexes-mismatch

greptile-apps · 2025-12-12T23:50:50Z

Greptile Summary

This PR refactors hook change detection to better handle composite hooks like useSyncExternalStore, useTransition, and useActionState by switching from low-level fiber inspection to using inspectHooks() which returns a properly structured HooksTree.

Key changes:

Replaced fiber-based hook traversal with HooksTree-based comparison using recursive traverse() function
Fixed useSyncExternalStore to read memoized state instead of calling getSnapshot() which may have changed since render
Added comprehensive test coverage for composite hooks detection
Changed hooks field from null to empty array [] when no hooks changed

Critical issues found:

The traverse() function has a critical runtime bug when hook trees have different lengths between renders - it will crash when accessing properties on undefined and will miss changes when the next tree is longer than the previous tree

Confidence Score: 1/5

This PR has critical runtime bugs that will cause crashes in production
The traverse() function in renderer.js has three related critical bugs that will cause runtime errors when hook trees have different lengths. This can happen if React's internal hook implementation changes or if there are edge cases not covered by tests. The code will crash when trying to access properties on undefined hooks.
packages/react-devtools-shared/src/backend/fiber/renderer.js requires immediate attention - the traverse function needs guards for undefined values

Important Files Changed

Filename	Overview
packages/react-devtools-shared/src/backend/fiber/renderer.js	Critical bug in `traverse` function: crashes when hook trees have different lengths, and misses changes when `nextTree` is longer than `prevTree`
packages/react-debug-tools/src/ReactDebugHooks.js	Fixed `useSyncExternalStore` to read from hook memoizedState instead of calling `getSnapshot()` which may have changed
packages/react-devtools-shared/src/tests/profilingCache-test.js	Added test coverage for composite hooks detection and fixed test by saving function references before profiling

greptile-apps

_{3 files reviewed, no comments}

_{Edit Code Review Agent Settings | Greptile}

…ok-indexes-mismatch

…rences

greptile-apps

_{4 files reviewed, 1 comment}

_{Edit Code Review Agent Settings | Greptile}

greptile-apps · 2025-12-25T23:16:59Z

-      if (isUseSyncExternalStoreHook(next)) {
-        next = next.next;
-        prev = prev.next;
+      if (prevHook === null || nextHook === null) {


logic: Check should handle undefined values. If nextFlattened.length < prevFlattened.length, accessing nextFlattened[index] returns undefined, not null, so this check won't catch it.

Suggested change

if (prevHook === null || nextHook === null) {

if (prevHook == null || nextHook == null) {

Prompt To Fix With AI

This is a comment left during a code review. Path: packages/react-devtools-shared/src/backend/fiber/renderer.js Line: 2078:2078 Comment: **logic:** Check should handle `undefined` values. If `nextFlattened.length < prevFlattened.length`, accessing `nextFlattened[index]` returns `undefined`, not `null`, so this check won't catch it. ```suggestion if (prevHook == null || nextHook == null) { ``` How can I resolve this? If you propose a fix, please make it concise.

…ok-indexes-mismatch

greptile-apps

_{4 files reviewed, 1 comment}

_{Edit Code Review Agent Settings | Greptile}

greptile-apps · 2026-01-14T19:13:54Z

+      if (prevHook === null || nextHook === null) {
+        continue;
      }


logic: check doesn't handle undefined - when nextFlattened.length < prevFlattened.length, nextFlattened[index] returns undefined, not null

Suggested change

if (prevHook === null || nextHook === null) {

continue;

}

if (prevHook == null || nextHook == null) {

continue;

}

Prompt To Fix With AI

This is a comment left during a code review. Path: packages/react-devtools-shared/src/backend/fiber/renderer.js Line: 2078:2080 Comment: **logic:** check doesn't handle `undefined` - when `nextFlattened.length < prevFlattened.length`, `nextFlattened[index]` returns `undefined`, not `null` ```suggestion if (prevHook == null || nextHook == null) { continue; } ``` How can I resolve this? If you propose a fix, please make it concise.

…averse function to directly calculate changed hooks

greptile-apps

_{4 files reviewed, 2 comments}

_{Edit Code Review Agent Settings | Greptile}

greptile-apps · 2026-01-14T21:13:31Z

-        indices.push(index);
-      }
+    function traverse(prevTree: HooksTree, nextTree: HooksTree): void {
+      for (let i = 0; i < prevTree.length; i++) {


logic: loop should use the longer array's length to handle cases where trees differ in size

Suggested change

for (let i = 0; i < prevTree.length; i++) {

for (let i = 0; i < Math.max(prevTree.length, nextTree.length); i++) {

Prompt To Fix With AI

This is a comment left during a code review. Path: packages/react-devtools-shared/src/backend/fiber/renderer.js Line: 2058:2058 Comment: **logic:** loop should use the longer array's length to handle cases where trees differ in size ```suggestion for (let i = 0; i < Math.max(prevTree.length, nextTree.length); i++) { ``` How can I resolve this? If you propose a fix, please make it concise.

greptile-apps · 2026-01-14T21:13:32Z

+        const prevHook = prevTree[i];
+        const nextHook = nextTree[i];

-      // useSyncExternalStore creates 2 internal hooks, but we only count it as 1 user-facing hook
-      if (isUseSyncExternalStoreHook(next)) {
-        next = next.next;
-        prev = prev.next;
-      }
+        if (prevHook?.subHooks?.length > 0 && nextHook?.subHooks?.length > 0) {
+          traverse(prevHook.subHooks, nextHook.subHooks);
+          continue;
+        }

-      next = next.next;
-      prev = prev.next;
-      index++;
+        if (didStatefulHookChange(prevHook, nextHook)) {


logic: add guard for when hooks are undefined due to array length mismatch

Suggested change

const prevHook = prevTree[i];

const nextHook = nextTree[i];

// useSyncExternalStore creates 2 internal hooks, but we only count it as 1 user-facing hook

if (isUseSyncExternalStoreHook(next)) {

next = next.next;

prev = prev.next;

}

if (prevHook?.subHooks?.length > 0 && nextHook?.subHooks?.length > 0) {

traverse(prevHook.subHooks, nextHook.subHooks);

continue;

}

next = next.next;

prev = prev.next;

index++;

if (didStatefulHookChange(prevHook, nextHook)) {

const prevHook = prevTree[i];

const nextHook = nextTree[i];

if (!prevHook || !nextHook) {

continue;

}

if (prevHook.subHooks.length > 0 && nextHook.subHooks.length > 0) {

traverse(prevHook.subHooks, nextHook.subHooks);

continue;

}

if (didStatefulHookChange(prevHook, nextHook)) {

Prompt To Fix With AI

This is a comment left during a code review. Path: packages/react-devtools-shared/src/backend/fiber/renderer.js Line: 2059:2067 Comment: **logic:** add guard for when hooks are `undefined` due to array length mismatch ```suggestion const prevHook = prevTree[i]; const nextHook = nextTree[i]; if (!prevHook || !nextHook) { continue; } if (prevHook.subHooks.length > 0 && nextHook.subHooks.length > 0) { traverse(prevHook.subHooks, nextHook.subHooks); continue; } if (didStatefulHookChange(prevHook, nextHook)) { ``` How can I resolve this? If you propose a fix, please make it concise.

…lingCache

greptile-apps

_{4 files reviewed, 1 comment}

_{Edit Code Review Agent Settings | Greptile}

greptile-apps · 2026-01-14T22:12:45Z

+    function traverse(prevTree: HooksTree, nextTree: HooksTree): void {
+      for (let i = 0; i < prevTree.length; i++) {
+        const prevHook = prevTree[i];
+        const nextHook = nextTree[i];

-      // useSyncExternalStore creates 2 internal hooks, but we only count it as 1 user-facing hook
-      if (isUseSyncExternalStoreHook(next)) {
-        next = next.next;
-        prev = prev.next;
-      }
+        if (prevHook?.subHooks?.length > 0 && nextHook?.subHooks?.length > 0) {
+          traverse(prevHook.subHooks, nextHook.subHooks);
+          continue;
+        }

-      next = next.next;
-      prev = prev.next;
-      index++;
+        if (didStatefulHookChange(prevHook, nextHook)) {
+          indices.push(index);
+        }
+
+        index++;
+      }


logic: runtime error when nextTree has fewer hooks than prevTree - accessing properties on undefined

when i >= nextTree.length, nextHook becomes undefined, and didStatefulHookChange(prevHook, nextHook) will throw trying to access next.isStateEditable or next.name

also misses changes when nextTree.length > prevTree.length since loop only iterates to prevTree.length

Suggested change

function traverse(prevTree: HooksTree, nextTree: HooksTree): void {

for (let i = 0; i < prevTree.length; i++) {

const prevHook = prevTree[i];

const nextHook = nextTree[i];

// useSyncExternalStore creates 2 internal hooks, but we only count it as 1 user-facing hook

if (isUseSyncExternalStoreHook(next)) {

next = next.next;

prev = prev.next;

}

if (prevHook?.subHooks?.length > 0 && nextHook?.subHooks?.length > 0) {

traverse(prevHook.subHooks, nextHook.subHooks);

continue;

}

next = next.next;

prev = prev.next;

index++;

if (didStatefulHookChange(prevHook, nextHook)) {

indices.push(index);

}

index++;

}

function traverse(prevTree: HooksTree, nextTree: HooksTree): void {

const maxLength = Math.max(prevTree.length, nextTree.length);

for (let i = 0; i < maxLength; i++) {

const prevHook = prevTree[i];

const nextHook = nextTree[i];

if (!prevHook || !nextHook) {

// Array length mismatch - treat as a change

indices.push(index);

index++;

continue;

}

if (prevHook.subHooks?.length > 0 && nextHook.subHooks?.length > 0) {

traverse(prevHook.subHooks, nextHook.subHooks);

continue;

}

if (didStatefulHookChange(prevHook, nextHook)) {

indices.push(index);

}

index++;

}

}

Prompt To Fix With AI

This is a comment left during a code review. Path: packages/react-devtools-shared/src/backend/fiber/renderer.js Line: 2057:2072 Comment: **logic:** runtime error when `nextTree` has fewer hooks than `prevTree` - accessing properties on `undefined` when `i >= nextTree.length`, `nextHook` becomes `undefined`, and `didStatefulHookChange(prevHook, nextHook)` will throw trying to access `next.isStateEditable` or `next.name` also misses changes when `nextTree.length > prevTree.length` since loop only iterates to `prevTree.length` ```suggestion function traverse(prevTree: HooksTree, nextTree: HooksTree): void { const maxLength = Math.max(prevTree.length, nextTree.length); for (let i = 0; i < maxLength; i++) { const prevHook = prevTree[i]; const nextHook = nextTree[i]; if (!prevHook || !nextHook) { // Array length mismatch - treat as a change indices.push(index); index++; continue; } if (prevHook.subHooks?.length > 0 && nextHook.subHooks?.length > 0) { traverse(prevHook.subHooks, nextHook.subHooks); continue; } if (didStatefulHookChange(prevHook, nextHook)) { indices.push(index); } index++; } } ``` How can I resolve this? If you propose a fix, please make it concise.

Co-authored-by: Ruslan Lesiutin <28902667+hoxyq@users.noreply.github.com>

greptile-apps

_{4 files reviewed, 3 comments}

_{Edit Code Review Agent Settings | Greptile}

greptile-apps · 2026-01-14T23:14:42Z

+      for (let i = 0; i < prevTree.length; i++) {
+        const prevHook = prevTree[i];
+        const nextHook = nextTree[i];


logic: loop must use Math.max(prevTree.length, nextTree.length) to handle trees of different sizes

when i >= nextTree.length, nextHook is undefined, causing runtime error in didStatefulHookChange when accessing next.isStateEditable or next.name

also misses changes when nextTree.length > prevTree.length since loop only iterates to prevTree.length

Suggested change

for (let i = 0; i < prevTree.length; i++) {

const prevHook = prevTree[i];

const nextHook = nextTree[i];

for (let i = 0; i < Math.max(prevTree.length, nextTree.length); i++) {

const prevHook = prevTree[i];

const nextHook = nextTree[i];

Are hook trees expected to always have the same length between renders, or can components conditionally call different numbers of hooks?

Prompt To Fix With AI

This is a comment left during a code review. Path: packages/react-devtools-shared/src/backend/fiber/renderer.js Line: 2058:2060 Comment: **logic:** loop must use `Math.max(prevTree.length, nextTree.length)` to handle trees of different sizes when `i >= nextTree.length`, `nextHook` is `undefined`, causing runtime error in `didStatefulHookChange` when accessing `next.isStateEditable` or `next.name` also misses changes when `nextTree.length > prevTree.length` since loop only iterates to `prevTree.length` ```suggestion for (let i = 0; i < Math.max(prevTree.length, nextTree.length); i++) { const prevHook = prevTree[i]; const nextHook = nextTree[i]; ``` Are hook trees expected to always have the same length between renders, or can components conditionally call different numbers of hooks? How can I resolve this? If you propose a fix, please make it concise.

greptile-apps · 2026-01-14T23:14:42Z

+        if (prevHook.subHooks?.length > 0 && nextHook.subHooks?.length > 0) {
+          traverse(prevHook.subHooks, nextHook.subHooks);
+          continue;
+        }


logic: doesn't handle when prevHook or nextHook is undefined due to mismatched tree lengths

accessing .subHooks on undefined will throw

Suggested change

if (prevHook.subHooks?.length > 0 && nextHook.subHooks?.length > 0) {

traverse(prevHook.subHooks, nextHook.subHooks);

continue;

}

if (prevHook?.subHooks?.length > 0 && nextHook?.subHooks?.length > 0) {

traverse(prevHook.subHooks, nextHook.subHooks);

continue;

}

Prompt To Fix With AI

This is a comment left during a code review. Path: packages/react-devtools-shared/src/backend/fiber/renderer.js Line: 2062:2065 Comment: **logic:** doesn't handle when `prevHook` or `nextHook` is `undefined` due to mismatched tree lengths accessing `.subHooks` on `undefined` will throw ```suggestion if (prevHook?.subHooks?.length > 0 && nextHook?.subHooks?.length > 0) { traverse(prevHook.subHooks, nextHook.subHooks); continue; } ``` How can I resolve this? If you propose a fix, please make it concise.

greptile-apps · 2026-01-14T23:14:43Z

+        if (didStatefulHookChange(prevHook, nextHook)) {
+          indices.push(index);
+        }


logic: didStatefulHookChange will crash if either hook is undefined

needs guard to handle mismatched tree lengths

Suggested change

if (didStatefulHookChange(prevHook, nextHook)) {

indices.push(index);

}

if (prevHook && nextHook && didStatefulHookChange(prevHook, nextHook)) {

indices.push(index);

}

Prompt To Fix With AI

This is a comment left during a code review. Path: packages/react-devtools-shared/src/backend/fiber/renderer.js Line: 2067:2069 Comment: **logic:** `didStatefulHookChange` will crash if either hook is `undefined` needs guard to handle mismatched tree lengths ```suggestion if (prevHook && nextHook && didStatefulHookChange(prevHook, nextHook)) { indices.push(index); } ``` How can I resolve this? If you propose a fix, please make it concise.

everettbu · 2026-01-15T11:14:25Z

Upstream PR was closed or merged. Code is synced via branch mirror.

blazejkustra added 8 commits November 13, 2025 01:13

Improve the detection of changed hooks

4e09f54

Add 'FormState' to the list of editable state names in hook detection

7120a95

Fix lint

b77efa4

Fix tests 4/10

7371cfc

Use dispatcherHookName instead of name

de00387

Update hook detection to use 'name' property instead of 'dispatcherHo…

ddf0277

…okName'

Refactor hook inspection to simplify detection logic

f565055

Merge branch 'main' of github.com:facebook/react into fix/devtools-ho…

c0ec480

…ok-indexes-mismatch

everettbu added the CLA Signed label Dec 12, 2025

greptile-apps Bot reviewed Dec 12, 2025

View reviewed changes

blazejkustra added 2 commits December 25, 2025 23:52

Merge branch 'main' of github.com:facebook/react into fix/devtools-ho…

2cae0a1

…ok-indexes-mismatch

Fix profiling cache tests to preserve real dispatch and setState refe…

7a68cae

…rences

greptile-apps Bot reviewed Dec 25, 2025

View reviewed changes

Merge branch 'main' of github.com:facebook/react into fix/devtools-ho…

ac421e0

…ok-indexes-mismatch

greptile-apps Bot reviewed Jan 14, 2026

View reviewed changes

blazejkustra added 3 commits January 14, 2026 21:27

Removed flattenHooksTree function and replaced it with a recursive tr…

82f5ede

…averse function to directly calculate changed hooks

Fix hook traversal logic

78545f2

Remove potentially unreachable code

3df7554

greptile-apps Bot reviewed Jan 14, 2026

View reviewed changes

Add test for detecting changes in custom and composite hooks in Profi…

4b7295b

…lingCache

greptile-apps Bot reviewed Jan 14, 2026

View reviewed changes

Remove unnecessary optional chaining

2ee5680

Co-authored-by: Ruslan Lesiutin <28902667+hoxyq@users.noreply.github.com>

greptile-apps Bot reviewed Jan 14, 2026

View reviewed changes

everettbu closed this Jan 15, 2026

everettbu deleted the fix/devtools-hook-indexes-mismatch branch January 15, 2026 11:14

	if (prevHook === null \|\| nextHook === null) {
	if (prevHook == null \|\| nextHook == null) {

	for (let i = 0; i < prevTree.length; i++) {
	for (let i = 0; i < Math.max(prevTree.length, nextTree.length); i++) {

Conversation

everettbu commented Dec 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

How did you test this change?

Uh oh!

greptile-apps Bot commented Dec 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Greptile Summary

Confidence Score: 1/5

Important Files Changed

Uh oh!

greptile-apps Bot left a comment

Choose a reason for hiding this comment

Uh oh!

greptile-apps Bot left a comment

Choose a reason for hiding this comment

Uh oh!

greptile-apps Bot Dec 25, 2025

Choose a reason for hiding this comment

Uh oh!

greptile-apps Bot left a comment

Choose a reason for hiding this comment

Uh oh!

greptile-apps Bot Jan 14, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps Bot left a comment

Choose a reason for hiding this comment

Uh oh!

greptile-apps Bot Jan 14, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps Bot Jan 14, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps Bot left a comment

Choose a reason for hiding this comment

Uh oh!

greptile-apps Bot Jan 14, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps Bot left a comment

Choose a reason for hiding this comment

Uh oh!

greptile-apps Bot Jan 14, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps Bot Jan 14, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps Bot Jan 14, 2026

Choose a reason for hiding this comment

Uh oh!

everettbu commented Jan 15, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

everettbu commented Dec 12, 2025 •

edited

Loading

greptile-apps Bot commented Dec 12, 2025 •

edited

Loading