Improve the detection of changed hooks

[blazejkustra]

Summary

cc @hoxyq

Fixes #28584. Follow up to PR: #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>
    </>
  );
}

[react-sizebot]

Comparing: bbe3f4d...f565055

Critical size changes

Includes critical production bundles, as well as any change greater than 2%:

Name	+/-	Base	Current	+/- gzip	Base gzip	Current gzip
oss-stable/react-dom/cjs/react-dom.production.js	=	6.68 kB	6.68 kB	+0.11%	1.83 kB	1.83 kB
oss-stable/react-dom/cjs/react-dom-client.production.js	=	608.16 kB	608.16 kB	=	107.65 kB	107.65 kB
oss-experimental/react-dom/cjs/react-dom.production.js	=	6.69 kB	6.69 kB	+0.11%	1.83 kB	1.83 kB
oss-experimental/react-dom/cjs/react-dom-client.production.js	=	666.18 kB	666.18 kB	=	117.35 kB	117.35 kB
facebook-www/ReactDOM-prod.classic.js	=	693.31 kB	693.31 kB	=	121.97 kB	121.98 kB
facebook-www/ReactDOM-prod.modern.js	=	683.73 kB	683.73 kB	=	120.36 kB	120.36 kB

Significant size changes

Includes any change greater than 0.2%:

Expand to show

Name	+/-	Base	Current	+/- gzip	Base gzip	Current gzip
oss-experimental/react-debug-tools/cjs/react-debug-tools.production.js	+0.25%	29.70 kB	29.78 kB	+0.16%	5.80 kB	5.81 kB
oss-stable-semver/react-debug-tools/cjs/react-debug-tools.production.js	+0.25%	29.70 kB	29.78 kB	+0.16%	5.80 kB	5.81 kB
oss-stable/react-debug-tools/cjs/react-debug-tools.production.js	+0.25%	29.70 kB	29.78 kB	+0.16%	5.80 kB	5.81 kB
oss-experimental/react-debug-tools/cjs/react-debug-tools.development.js	+0.23%	33.29 kB	33.37 kB	+0.14%	5.92 kB	5.93 kB
oss-stable-semver/react-debug-tools/cjs/react-debug-tools.development.js	+0.23%	33.29 kB	33.37 kB	+0.14%	5.92 kB	5.93 kB
oss-stable/react-debug-tools/cjs/react-debug-tools.development.js	+0.23%	33.29 kB	33.37 kB	+0.14%	5.92 kB	5.93 kB

Generated by 🚫 dangerJS against f565055

[blazejkustra]

@hoxyq I may need your help... Two tests are failing and I don't understand what's wrong

• profilingCache-test.js › should properly detect changed hooks
• profilingCache-test.js › should detect context changes or lack of changes with conditional use()

They only fail when I call:

const prevHooks = inspectHooks(prevFiber);
const nextHooks = inspectHooks(nextFiber);

I’ve narrowed it to this:

export function inspectHooks<Props>(
  renderFunction: (props: Props) => React$Node,
  props: Props,
  currentDispatcher?: CurrentDispatcherRef,
): HooksTree {
  if (currentDispatcher == null) {
    currentDispatcher = ReactSharedInternals;
  }

  const previousDispatcher = currentDispatcher.H;
  currentDispatcher.H = DispatcherProxy;

  let readHookLog;
  let ancestorStackError;

  try {
    ancestorStackError = new Error();
    renderFunction(props);
  } catch (error) {
    handleRenderFunctionError(error);
  } finally {
    readHookLog = hookLog;
    hookLog = [];
    // $FlowFixMe[incompatible-use] found when upgrading Flow
    currentDispatcher.H = previousDispatcher;
  }

  const rootStack =
    ancestorStackError === undefined
      ? ([]: ParsedStackFrame[])
      : ErrorStackParser.parse(ancestorStackError);

  return buildTree(rootStack, readHookLog);
}

I suspect the issue comes from how inspectHooks temporarily swaps out the dispatcher. That dispatcher replacement might be overwriting or leaking some internal data, which then causes the change descriptions recorded by the Profiler to become incomplete.