Improve the detection of changed hooks

packages/react-debug-tools/src/ReactDebugHooks.js

@@ -467,9 +467,11 @@ function useSyncExternalStore<T>(
   // useSyncExternalStore() composes multiple hooks internally.
   // Advance the current hook index the same number of times
   // so that subsequent hooks have the right memoized state.
-  nextHook(); // SyncExternalStore
+  const hook = nextHook(); // SyncExternalStore
   nextHook(); // Effect
-  const value = getSnapshot();
+  // Read from hook.memoizedState to get the value that was used during render,
+  // not the current value from getSnapshot() which may have changed.
+  const value = hook !== null ? hook.memoizedState : getSnapshot();
   hookLog.push({
     displayName: null,
     primitive: 'SyncExternalStore',

packages/react-devtools-shared/src/__tests__/profilerChangeDescriptions-test.js

@@ -90,7 +90,7 @@ describe('Profiler change descriptions', () => {
       {
         "context": true,
         "didHooksChange": false,
-        "hooks": null,
+        "hooks": [],
         "isFirstMount": false,
         "props": [],
         "state": null,
@@ -110,7 +110,7 @@ describe('Profiler change descriptions', () => {
       {
         "context": true,
         "didHooksChange": false,
-        "hooks": null,
+        "hooks": [],
         "isFirstMount": false,
         "props": [],
         "state": null,
@@ -125,7 +125,7 @@ describe('Profiler change descriptions', () => {
       {
         "context": false,
         "didHooksChange": false,
-        "hooks": null,
+        "hooks": [],
         "isFirstMount": false,
         "props": [],
         "state": null,
@@ -140,7 +140,7 @@ describe('Profiler change descriptions', () => {
       {
         "context": true,
         "didHooksChange": false,
-        "hooks": null,
+        "hooks": [],
         "isFirstMount": false,
         "props": [],
         "state": null,

packages/react-devtools-shared/src/backend/fiber/renderer.js

@@ -18,7 +18,7 @@ import type {
   Wakeable,
 } from 'shared/ReactTypes';
 
-import type {HooksTree} from 'react-debug-tools/src/ReactDebugHooks';
+import type {HooksNode, HooksTree} from 'react-debug-tools/src/ReactDebugHooks';
 
 import {
   ComponentFilterDisplayName,
@@ -126,7 +126,6 @@ import {enableStyleXFeatures} from 'react-devtools-feature-flags';
 import {componentInfoToComponentLogsMap} from '../shared/DevToolsServerComponentLogs';
 
 import is from 'shared/objectIs';
-import hasOwnProperty from 'shared/hasOwnProperty';
 
 import {getIODescription} from 'shared/ReactIODescription';
 
@@ -1975,10 +1974,9 @@ export function attach(
             state: null,
           };
         } else {
-          const indices = getChangedHooksIndices(
-            prevFiber.memoizedState,
-            nextFiber.memoizedState,
-          );
+          const prevHooks = inspectHooks(prevFiber);
+          const nextHooks = inspectHooks(nextFiber);
+          const indices = getChangedHooksIndices(prevHooks, nextHooks);
           const data: ChangeDescription = {
             context: getContextChanged(prevFiber, nextFiber),
             didHooksChange: indices !== null && indices.length > 0,
@@ -2027,72 +2025,62 @@ 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 didStatefulHookChange(prev: HooksNode, next: HooksNode): boolean {
+    // Detect the shape of useState() / useReducer() / useTransition() / useSyncExternalStore() / useActionState()
+    const isStatefulHook =
+      prev.isStateEditable === true ||
+      prev.name === 'SyncExternalStore' ||
+      prev.name === 'Transition' ||
+      prev.name === 'ActionState' ||
+      prev.name === 'FormState';
 
-  function isHookThatCanScheduleUpdate(hookObject: any) {
-    const queue = hookObject.queue;
-    if (!queue) {
-      return false;
-    }
-
-    const boundHasOwnProperty = hasOwnProperty.bind(queue);
-
-    // Detect the shape of useState() / useReducer() / useTransition()
-    // using the attributes that are unique to these hooks
-    // but also stable (e.g. not tied to current Lanes implementation)
-    // We don't check for dispatch property, because useTransition doesn't have it
-    if (boundHasOwnProperty('pending')) {
-      return true;
+    // Compare the values to see if they changed
+    if (isStatefulHook) {
+      return prev.value !== next.value;
     }
 
-    return isUseSyncExternalStoreHook(hookObject);
+    return false;
   }
 
-  function didStatefulHookChange(prev: any, next: any): boolean {
-    const prevMemoizedState = prev.memoizedState;
-    const nextMemoizedState = next.memoizedState;
-
-    if (isHookThatCanScheduleUpdate(prev)) {
-      return prevMemoizedState !== nextMemoizedState;
+  function flattenHooksTree(hooksTree: HooksTree): HooksTree {
+    const flattened: HooksTree = [];
+    for (let i = 0; i < hooksTree.length; i++) {
+      const currentHook = hooksTree[i];
+      // If the hook has subHooks, flatten them recursively
+      if (currentHook.subHooks && currentHook.subHooks.length > 0) {
+        flattened.push(...flattenHooksTree(currentHook.subHooks));
+        continue;
+      }
+      // If the hook doesn't have subHooks, add it to the flattened list
+      flattened.push(currentHook);
     }
-
-    return false;
+    return flattened;
   }
 
-  function getChangedHooksIndices(prev: any, next: any): null | Array<number> {
-    if (prev == null || next == null) {
+  function getChangedHooksIndices(
+    prevHooks: HooksTree | null,
+    nextHooks: HooksTree | null,
+  ): null | Array<number> {
+    if (prevHooks == null || nextHooks == null) {
       return null;
     }
 
-    const indices = [];
-    let index = 0;
+    const prevFlattened = flattenHooksTree(prevHooks);
+    const nextFlattened = flattenHooksTree(nextHooks);
 
-    while (next !== null) {
-      if (didStatefulHookChange(prev, next)) {
-        indices.push(index);
-      }
+    const indices: Array<number> = [];
+
+    for (let index = 0; index < prevFlattened.length; index++) {
+      const prevHook = prevFlattened[index];
+      const nextHook = nextFlattened[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;
+      if (prevHook === null || nextHook === null) {
+        continue;
       }
 
-      next = next.next;
-      prev = prev.next;
-      index++;
+      if (didStatefulHookChange(prevHook, nextHook)) {
+        indices.push(index);
+      }
     }
 
     return indices;