unlock unlimited code runs for $79, limited time →
NOSSLETTER.techLIVE
back to OSSpedia
testing-strategydata-structuresobservability

Improve the detection of changed hooks

Project: facebook/react | Language: JavaScript | Concepts: testing-strategy, data-structures, observability | Difficulty: mid
Link: https://github.com/facebook/react/pull/35123

The problem

React DevTools tracks which hooks have changed between renders to help developers debug their components. Some hooks, like useSyncExternalStore, useTransition, useActionState, and useFormState, are composite—they internally use multiple primitive hooks, but are exposed as a single logical hook to the user. Previously, DevTools would incorrectly count and compare these internal hooks individually, causing hook change detection and indices to be inaccurate. This led to confusing or incorrect reporting of which user-facing hooks had changed.

Why it's non-trivial

A naive fix might compare the raw internal hook linked lists, which does not map cleanly to user-facing hooks when composite hooks are present. This would cause off-by-one errors in hook indices, and misreport changes for hooks after a composite hook. Correctly mapping internal hook structure to user-facing hooks requires understanding and flattening this structure, so that composite hooks are treated as a single unit.

The approach

1. Switch from comparing raw hook linked lists to using the parsed hook tree:

Instead of comparing the raw memoizedState linked lists, the PR switches to using the output of inspectHooks, which returns a tree structure representing the user-facing hooks (flattening out the internal details of composite hooks):

-          const indices = getChangedHooksIndices(
-            prevFiber.memoizedState,
-            nextFiber.memoizedState,
-          );
+          const prevHooks = inspectHooks(prevFiber);
+          const nextHooks = inspectHooks(nextFiber);
+          const indices = getChangedHooksIndices(prevHooks, nextHooks);

This change ensures that all further logic operates on the user-facing hook structure, not the internal implementation details.

2. Update the hook comparison logic to operate on the hook tree:

The function that detects which hooks have changed is rewritten to traverse the hook tree, recursively comparing each hook and its sub-hooks, and collecting indices of changed hooks:

-  function getChangedHooksIndices(prev: any, next: any): null | Array<number> {
-    if (prev == null || next == null) {
-      return null;
-    }
-
-    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)) {
-        next = next.next;
-        prev = prev.next;
-      }
-
-      next = next.next;
-      prev = prev.next;
-      index++;
-    }
-
-    return indices;
-  }
+  function getChangedHooksIndices(
+    prevHooks: HooksTree | null,
+    nextHooks: HooksTree | null,
+  ): null | Array<number> {
+    if (prevHooks == null || nextHooks == null) {
+      return null;
+    }
+
+    const indices: Array<number> = [];
+    let index = 0;
+
+    function traverse(prevTree: HooksTree, nextTree: HooksTree): void {
+      for (let i = 0; i < prevTree.length; i++) {
+        const prevHook = prevTree[i];
+        const nextHook = nextTree[i];
+
+        if (prevHook.subHooks.length > 0 && nextHook.subHooks.length > 0) {
+          traverse(prevHook.subHooks, nextHook.subHooks);
+          continue;
+        }
+
+        if (didStatefulHookChange(prevHook, nextHook)) {
+          indices.push(index);
+        }
+
+        index++;
+      }
+    }
+    traverse(prevHooks, nextHooks);
+    return indices;
+  }

This code walks the user-facing hook tree, comparing hooks at each index, and recursing into sub-hooks where appropriate. It collects the indices of changed hooks as seen by the user, not the internal implementation.

3. Update tests to match new output and add new scenarios:

The tests are updated so that when no hooks have changed, "hooks": [] is used instead of null, and a new test is added to cover custom and composite hooks:

-        "hooks": null,
+        "hooks": [],

A comprehensive new test ensures that changes to composite hooks are detected and indexed correctly:

+  it('should detect what hooks changed in a render with custom and composite hooks', () => {
+    // ...setup for a component using useState, useSyncExternalStore, useTransition, useActionState, and a custom hook...
+    // ...simulate changes to each hook in turn...
+    // ...assert that the correct hook indices are reported as changed...
+  });

This verifies that the new logic works as intended for a variety of real-world hook usage patterns.

From the review thread

andreisilviudragnea: I think that merging https://github.com/facebook/react/pull/34547 without fixing the index counting error for all composite hooks just does not fix the initial problem at all, because having for example a useTransition hook before any other hook will invalidate the "Hook X changed" index for all hooks that follow useTransition

blazejkustra: @andreisilviudragnea read through the discussion on my previous PR, especially [this comment](https://gi

Takeaways

  • Always compare user-facing abstractions, not internal implementation details, when reporting information to end users.
  • When dealing with tree or nested data structures, use recursive traversal to ensure all relevant nodes (including sub-hooks) are processed.
  • Comprehensive, scenario-based tests are essential when changing logic that maps internal state to user-facing output, especially for complex or composite abstractions.