diff --git a/packages/react-debug-tools/src/ReactDebugHooks.js b/packages/react-debug-tools/src/ReactDebugHooks.js index db9495a97dd..b4eb6c2b596 100644 --- a/packages/react-debug-tools/src/ReactDebugHooks.js +++ b/packages/react-debug-tools/src/ReactDebugHooks.js @@ -467,9 +467,11 @@ function useSyncExternalStore( // 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', diff --git a/packages/react-devtools-shared/src/__tests__/profilerChangeDescriptions-test.js b/packages/react-devtools-shared/src/__tests__/profilerChangeDescriptions-test.js index 87d8132e50e..4710f5cd1b2 100644 --- a/packages/react-devtools-shared/src/__tests__/profilerChangeDescriptions-test.js +++ b/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, diff --git a/packages/react-devtools-shared/src/__tests__/profilingCache-test.js b/packages/react-devtools-shared/src/__tests__/profilingCache-test.js index 428b22d466d..c17a59c2925 100644 --- a/packages/react-devtools-shared/src/__tests__/profilingCache-test.js +++ b/packages/react-devtools-shared/src/__tests__/profilingCache-test.js @@ -378,6 +378,12 @@ describe('ProfilingCache', () => { ), ); + // Save references to the real dispatch/setState functions. + // inspectHooks() re-runs the component with a mock dispatcher, + // which would overwrite these variables with mock functions that do nothing. + const realDispatch = dispatch; + const realSetState = setState; + // Second render has no changed hooks, only changed props. utils.act(() => render( @@ -388,10 +394,10 @@ describe('ProfilingCache', () => { ); // Third render has a changed reducer hook. - utils.act(() => dispatch({type: 'invert'})); + utils.act(() => realDispatch({type: 'invert'})); // Fourth render has a changed state hook. - utils.act(() => setState('def')); + utils.act(() => realSetState('def')); // Fifth render has a changed context value, but no changed hook. utils.act(() => @@ -521,6 +527,238 @@ describe('ProfilingCache', () => { } }); + // @reactVersion >= 19.0 + it('should detect what hooks changed in a render with custom and composite hooks', () => { + let snapshot = 0; + let syncExternalStoreCallback; + + function subscribe(callback) { + syncExternalStoreCallback = callback; + return () => {}; + } + + function getSnapshot() { + return snapshot; + } + + // Custom hook wrapping multiple primitive hooks + function useCustomHook() { + const [value, setValue] = React.useState('custom'); + React.useEffect(() => {}, [value]); + return [value, setValue]; + } + + let setState = null; + let startTransition = null; + let actionStateDispatch = null; + let setCustomValue = null; + let setFinalState = null; + + const Component = () => { + // Hook 0: useState + const [state, _setState] = React.useState('initial'); + setState = _setState; + + // Hook 1: useSyncExternalStore (composite hook - internally uses multiple hooks) + const storeValue = React.useSyncExternalStore( + subscribe, + getSnapshot, + getSnapshot, + ); + + // Hook 2: useTransition (composite hook - internally uses multiple hooks) + const [isPending, _startTransition] = React.useTransition(); + startTransition = _startTransition; + + // Hook 3: useActionState (composite hook - internally uses multiple hooks) + const [actionState, _actionStateDispatch] = React.useActionState( + (_prev, action) => action, + 'action-initial', + ); + actionStateDispatch = _actionStateDispatch; + + // Hook 4: useState inside custom hook (flattened) + // Hook 5: useEffect inside custom hook (not stateful, won't show in changes) + const [customValue, _setCustomValue] = useCustomHook(); + setCustomValue = _setCustomValue; + + // Hook 6: direct useState at the end + const [finalState, _setFinalState] = React.useState('final'); + setFinalState = _setFinalState; + + return `${state}-${storeValue}-${isPending}-${actionState}-${customValue}-${finalState}`; + }; + + utils.act(() => store.profilerStore.startProfiling()); + utils.act(() => render()); + + // Save references before inspectHooks() overwrites them + const realSetState = setState; + const realStartTransition = startTransition; + const realActionStateDispatch = actionStateDispatch; + const realSetCustomValue = setCustomValue; + const realSetFinalState = setFinalState; + + // 2nd render: change useState (hook 0) + utils.act(() => realSetState('changed')); + + // 3rd render: change useSyncExternalStore (hook 1) + utils.act(() => { + snapshot = 1; + syncExternalStoreCallback(); + }); + + // 4th render: trigger useTransition (hook 2) + // Note: useTransition triggers two renders - one when isPending becomes true, + // and another when isPending becomes false after the transition completes + utils.act(() => { + realStartTransition(() => {}); + }); + + // 6th render: change useActionState (hook 3) + utils.act(() => realActionStateDispatch('action-changed')); + + // 7th render: change custom hook's useState (hook 4) + utils.act(() => realSetCustomValue('custom-changed')); + + // 8th render: change final useState (hook 6) + utils.act(() => realSetFinalState('final-changed')); + + utils.act(() => store.profilerStore.stopProfiling()); + + const rootID = store.roots[0]; + + const changeDescriptions = store.profilerStore + .getDataForRoot(rootID) + .commitData.map(commitData => commitData.changeDescriptions); + expect(changeDescriptions).toHaveLength(8); + + // 1st render: Initial mount + expect(changeDescriptions[0]).toMatchInlineSnapshot(` + Map { + 2 => { + "context": null, + "didHooksChange": false, + "isFirstMount": true, + "props": null, + "state": null, + }, + } + `); + + // 2nd render: Changed hook 0 (useState) + expect(changeDescriptions[1]).toMatchInlineSnapshot(` + Map { + 2 => { + "context": false, + "didHooksChange": true, + "hooks": [ + 0, + ], + "isFirstMount": false, + "props": [], + "state": null, + }, + } + `); + + // 3rd render: Changed hook 1 (useSyncExternalStore) + expect(changeDescriptions[2]).toMatchInlineSnapshot(` + Map { + 2 => { + "context": false, + "didHooksChange": true, + "hooks": [ + 1, + ], + "isFirstMount": false, + "props": [], + "state": null, + }, + } + `); + + // 4th render: Changed hook 2 (useTransition - isPending becomes true) + expect(changeDescriptions[3]).toMatchInlineSnapshot(` + Map { + 2 => { + "context": false, + "didHooksChange": true, + "hooks": [ + 2, + ], + "isFirstMount": false, + "props": [], + "state": null, + }, + } + `); + + // 5th render: Changed hook 2 (useTransition - isPending becomes false) + expect(changeDescriptions[4]).toMatchInlineSnapshot(` + Map { + 2 => { + "context": false, + "didHooksChange": true, + "hooks": [ + 2, + ], + "isFirstMount": false, + "props": [], + "state": null, + }, + } + `); + + // 6th render: Changed hook 3 (useActionState) + expect(changeDescriptions[5]).toMatchInlineSnapshot(` + Map { + 2 => { + "context": false, + "didHooksChange": true, + "hooks": [ + 3, + ], + "isFirstMount": false, + "props": [], + "state": null, + }, + } + `); + + // 7th render: Changed hook 4 (useState inside useCustomHook) + expect(changeDescriptions[6]).toMatchInlineSnapshot(` + Map { + 2 => { + "context": false, + "didHooksChange": true, + "hooks": [ + 4, + ], + "isFirstMount": false, + "props": [], + "state": null, + }, + } + `); + + // 8th render: Changed hook 6 (final useState) + expect(changeDescriptions[7]).toMatchInlineSnapshot(` + Map { + 2 => { + "context": false, + "didHooksChange": true, + "hooks": [ + 6, + ], + "isFirstMount": false, + "props": [], + "state": null, + }, + } + `); + }); + // @reactVersion >= 19.0 it('should detect context changes or lack of changes with conditional use()', () => { const ContextA = React.createContext(0); @@ -553,6 +791,11 @@ describe('ProfilingCache', () => { ), ); + // Save reference to the real setState function before profiling starts. + // inspectHooks() re-runs the component with a mock dispatcher, + // which would overwrite setState with a mock function that does nothing. + const realSetState = setState; + utils.act(() => store.profilerStore.startProfiling()); // First render changes Context. @@ -567,7 +810,7 @@ describe('ProfilingCache', () => { ); // Second render has no changed Context, only changed state. - utils.act(() => setState('def')); + utils.act(() => realSetState('def')); utils.act(() => store.profilerStore.stopProfiling()); diff --git a/packages/react-devtools-shared/src/backend/fiber/renderer.js b/packages/react-devtools-shared/src/backend/fiber/renderer.js index 6848e327f67..892a3c6fc77 100644 --- a/packages/react-devtools-shared/src/backend/fiber/renderer.js +++ b/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, @@ -127,7 +127,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'; @@ -1976,10 +1975,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, @@ -2028,74 +2026,53 @@ 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) { - 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; - } - - return isUseSyncExternalStoreHook(hookObject); - } - - function didStatefulHookChange(prev: any, next: any): boolean { - const prevMemoizedState = prev.memoizedState; - const nextMemoizedState = next.memoizedState; + 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'; - if (isHookThatCanScheduleUpdate(prev)) { - return prevMemoizedState !== nextMemoizedState; + // Compare the values to see if they changed + if (isStatefulHook) { + return prev.value !== next.value; } return false; } - function getChangedHooksIndices(prev: any, next: any): null | Array { - if (prev == null || next == null) { + function getChangedHooksIndices( + prevHooks: HooksTree | null, + nextHooks: HooksTree | null, + ): null | Array { + if (prevHooks == null || nextHooks == null) { return null; } - const indices = []; + const indices: Array = []; let index = 0; - while (next !== null) { - if (didStatefulHookChange(prev, next)) { - indices.push(index); - } + 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++; + } } + traverse(prevHooks, nextHooks); return indices; }