From f65ac7bd4aac61db1ec25af5b03b72d03779a890 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= <sebastian@calyptus.eu>
Date: Mon, 26 Aug 2024 11:53:17 -0400
Subject: [PATCH 01/10] [DevTools] Make function inspection instant (#30786)

I noticed that there is a delay due to the inspection being split into
one part that gets the attribute and another eval that does the
inspection. This is a bit hacky and uses temporary global names that are
leaky. The timeout was presumably to ensure that the first step had
fully propagated but it's slow. As we've learned, it can be throttled,
and it isn't a guarantee either way.

Instead, we can just consolidate these into a single operation that
by-passes the bridge and goes straight to the renderer interface from
the eval.

I did the same for the viewElementSource helper even though that's not
currently in use since #28471 but I think we probably should return to
that technique when it's available since it's more reliable than the
throw - at least in Chrome. I'm not sure about the status of React
Native here. In Firefox, inspecting a function with source maps doesn't
seem to work. It doesn't jump to original code.
---
 .../src/main/index.js                         | 16 +----
 .../src/main/sourceSelection.js               | 59 +++++++++++++++++++
 .../src/backend/agent.js                      | 20 -------
 .../src/backend/fiber/renderer.js             | 34 +++++------
 .../src/backend/legacy/renderer.js            | 19 +++---
 .../src/backend/types.js                      |  6 +-
 6 files changed, 90 insertions(+), 64 deletions(-)
 create mode 100644 packages/react-devtools-extensions/src/main/sourceSelection.js

diff --git a/packages/react-devtools-extensions/src/main/index.js b/packages/react-devtools-extensions/src/main/index.js
index d0bc285b11287..3a51b996e2049 100644
--- a/packages/react-devtools-extensions/src/main/index.js
+++ b/packages/react-devtools-extensions/src/main/index.js
@@ -21,6 +21,8 @@ import {
   setBrowserSelectionFromReact,
   setReactSelectionFromBrowser,
 } from './elementSelection';
+import {viewAttributeSource} from './sourceSelection';
+
 import {startReactPolling} from './reactPolling';
 import cloneStyleTags from './cloneStyleTags';
 import fetchFileWithCaching from './fetchFileWithCaching';
@@ -113,19 +115,7 @@ function createBridgeAndStore() {
   const viewAttributeSourceFunction = (id, path) => {
     const rendererID = store.getRendererIDForElement(id);
     if (rendererID != null) {
-      // Ask the renderer interface to find the specified attribute,
-      // and store it as a global variable on the window.
-      bridge.send('viewAttributeSource', {id, path, rendererID});
-
-      setTimeout(() => {
-        // Ask Chrome to display the location of the attribute,
-        // assuming the renderer found a match.
-        chrome.devtools.inspectedWindow.eval(`
-                if (window.$attribute != null) {
-                  inspect(window.$attribute);
-                }
-              `);
-      }, 100);
+      viewAttributeSource(rendererID, id, path);
     }
   };
 
diff --git a/packages/react-devtools-extensions/src/main/sourceSelection.js b/packages/react-devtools-extensions/src/main/sourceSelection.js
new file mode 100644
index 0000000000000..0534a921af05e
--- /dev/null
+++ b/packages/react-devtools-extensions/src/main/sourceSelection.js
@@ -0,0 +1,59 @@
+/* global chrome */
+
+export function viewAttributeSource(rendererID, elementID, path) {
+  chrome.devtools.inspectedWindow.eval(
+    '{' + // The outer block is important because it means we can declare local variables.
+      'const renderer = window.__REACT_DEVTOOLS_GLOBAL_HOOK__.rendererInterfaces.get(' +
+      JSON.stringify(rendererID) +
+      ');' +
+      'if (renderer) {' +
+      '  const value = renderer.getElementAttributeByPath(' +
+      JSON.stringify(elementID) +
+      ',' +
+      JSON.stringify(path) +
+      ');' +
+      '  if (value) {' +
+      '    inspect(value);' +
+      '    true;' +
+      '  } else {' +
+      '    false;' +
+      '  }' +
+      '} else {' +
+      '  false;' +
+      '}' +
+      '}',
+    (didInspect, evalError) => {
+      if (evalError) {
+        console.error(evalError);
+      }
+    },
+  );
+}
+
+export function viewElementSource(rendererID, elementID) {
+  chrome.devtools.inspectedWindow.eval(
+    '{' + // The outer block is important because it means we can declare local variables.
+      'const renderer = window.__REACT_DEVTOOLS_GLOBAL_HOOK__.rendererInterfaces.get(' +
+      JSON.stringify(rendererID) +
+      ');' +
+      'if (renderer) {' +
+      '  const value = renderer.getElementSourceFunctionById(' +
+      JSON.stringify(elementID) +
+      ');' +
+      '  if (value) {' +
+      '    inspect(value);' +
+      '    true;' +
+      '  } else {' +
+      '    false;' +
+      '  }' +
+      '} else {' +
+      '  false;' +
+      '}' +
+      '}',
+    (didInspect, evalError) => {
+      if (evalError) {
+        console.error(evalError);
+      }
+    },
+  );
+}
diff --git a/packages/react-devtools-shared/src/backend/agent.js b/packages/react-devtools-shared/src/backend/agent.js
index e81af56ebdf76..a71b259441e98 100644
--- a/packages/react-devtools-shared/src/backend/agent.js
+++ b/packages/react-devtools-shared/src/backend/agent.js
@@ -220,8 +220,6 @@ export default class Agent extends EventEmitter<{
       this.updateConsolePatchSettings,
     );
     bridge.addListener('updateComponentFilters', this.updateComponentFilters);
-    bridge.addListener('viewAttributeSource', this.viewAttributeSource);
-    bridge.addListener('viewElementSource', this.viewElementSource);
 
     // Temporarily support older standalone front-ends sending commands to newer embedded backends.
     // We do this because React Native embeds the React DevTools backend,
@@ -816,24 +814,6 @@ export default class Agent extends EventEmitter<{
       }
     };
 
-  viewAttributeSource: CopyElementParams => void = ({id, path, rendererID}) => {
-    const renderer = this._rendererInterfaces[rendererID];
-    if (renderer == null) {
-      console.warn(`Invalid renderer id "${rendererID}" for element "${id}"`);
-    } else {
-      renderer.prepareViewAttributeSource(id, path);
-    }
-  };
-
-  viewElementSource: ElementAndRendererID => void = ({id, rendererID}) => {
-    const renderer = this._rendererInterfaces[rendererID];
-    if (renderer == null) {
-      console.warn(`Invalid renderer id "${rendererID}" for element "${id}"`);
-    } else {
-      renderer.prepareViewElementSource(id);
-    }
-  };
-
   onTraceUpdates: (nodes: Set<HostInstance>) => void = nodes => {
     this.emit('traceUpdates', nodes);
   };
diff --git a/packages/react-devtools-shared/src/backend/fiber/renderer.js b/packages/react-devtools-shared/src/backend/fiber/renderer.js
index 62eacdf5dbe9a..57eafa176f550 100644
--- a/packages/react-devtools-shared/src/backend/fiber/renderer.js
+++ b/packages/react-devtools-shared/src/backend/fiber/renderer.js
@@ -3874,27 +3874,28 @@ export function attach(
 
   // END copied code
 
-  function prepareViewAttributeSource(
+  function getElementAttributeByPath(
     id: number,
     path: Array<string | number>,
-  ): void {
+  ): mixed {
     if (isMostRecentlyInspectedElement(id)) {
-      window.$attribute = getInObject(
+      return getInObject(
         ((mostRecentlyInspectedElement: any): InspectedElement),
         path,
       );
     }
+    return undefined;
   }
 
-  function prepareViewElementSource(id: number): void {
+  function getElementSourceFunctionById(id: number): null | Function {
     const devtoolsInstance = idToDevToolsInstanceMap.get(id);
     if (devtoolsInstance === undefined) {
       console.warn(`Could not find DevToolsInstance with id "${id}"`);
-      return;
+      return null;
     }
     if (devtoolsInstance.kind !== FIBER_INSTANCE) {
       // TODO: Handle VirtualInstance.
-      return;
+      return null;
     }
     const fiber = devtoolsInstance.data;
 
@@ -3906,21 +3907,16 @@ export function attach(
       case IncompleteFunctionComponent:
       case IndeterminateComponent:
       case FunctionComponent:
-        global.$type = type;
-        break;
+        return type;
       case ForwardRef:
-        global.$type = type.render;
-        break;
+        return type.render;
       case MemoComponent:
       case SimpleMemoComponent:
-        global.$type =
-          elementType != null && elementType.type != null
-            ? elementType.type
-            : type;
-        break;
+        return elementType != null && elementType.type != null
+          ? elementType.type
+          : type;
       default:
-        global.$type = null;
-        break;
+        return null;
     }
   }
 
@@ -5727,8 +5723,8 @@ export function attach(
     inspectElement,
     logElementToConsole,
     patchConsoleForStrictMode,
-    prepareViewAttributeSource,
-    prepareViewElementSource,
+    getElementAttributeByPath,
+    getElementSourceFunctionById,
     overrideError,
     overrideSuspense,
     overrideValueAtPath,
diff --git a/packages/react-devtools-shared/src/backend/legacy/renderer.js b/packages/react-devtools-shared/src/backend/legacy/renderer.js
index 1955465607d2c..f8aa548a0573a 100644
--- a/packages/react-devtools-shared/src/backend/legacy/renderer.js
+++ b/packages/react-devtools-shared/src/backend/legacy/renderer.js
@@ -907,30 +907,31 @@ export function attach(
     }
   }
 
-  function prepareViewAttributeSource(
+  function getElementAttributeByPath(
     id: number,
     path: Array<string | number>,
-  ): void {
+  ): mixed {
     const inspectedElement = inspectElementRaw(id);
     if (inspectedElement !== null) {
-      window.$attribute = getInObject(inspectedElement, path);
+      return getInObject(inspectedElement, path);
     }
+    return undefined;
   }
 
-  function prepareViewElementSource(id: number): void {
+  function getElementSourceFunctionById(id: number): null | Function {
     const internalInstance = idToInternalInstanceMap.get(id);
     if (internalInstance == null) {
       console.warn(`Could not find instance with id "${id}"`);
-      return;
+      return null;
     }
 
     const element = internalInstance._currentElement;
     if (element == null) {
       console.warn(`Could not find element with id "${id}"`);
-      return;
+      return null;
     }
 
-    global.$type = element.type;
+    return element.type;
   }
 
   function deletePath(
@@ -1141,8 +1142,8 @@ export function attach(
     overrideValueAtPath,
     renamePath,
     patchConsoleForStrictMode,
-    prepareViewAttributeSource,
-    prepareViewElementSource,
+    getElementAttributeByPath,
+    getElementSourceFunctionById,
     renderer,
     setTraceUpdatesEnabled,
     setTrackedPath,
diff --git a/packages/react-devtools-shared/src/backend/types.js b/packages/react-devtools-shared/src/backend/types.js
index 2bd13a3a1294d..87b0f2048b9db 100644
--- a/packages/react-devtools-shared/src/backend/types.js
+++ b/packages/react-devtools-shared/src/backend/types.js
@@ -394,11 +394,11 @@ export type RendererInterface = {
     value: any,
   ) => void,
   patchConsoleForStrictMode: () => void,
-  prepareViewAttributeSource: (
+  getElementAttributeByPath: (
     id: number,
     path: Array<string | number>,
-  ) => void,
-  prepareViewElementSource: (id: number) => void,
+  ) => mixed,
+  getElementSourceFunctionById: (id: number) => null | Function,
   renamePath: (
     type: Type,
     id: number,

From dcae56f8b72f625d8affe5729ca9991b31a492ac Mon Sep 17 00:00:00 2001
From: Lauren Tan <poteto@users.noreply.github.com>
Date: Mon, 26 Aug 2024 11:57:12 -0400
Subject: [PATCH 02/10] [ez] Remove trailing space from babel-refresh header

ghstack-source-id: d78c53462b3be7b93733cdd0a7def96d7112087e
Pull Request resolved: https://github.com/facebook/react/pull/30806
---
 .github/workflows/runtime_commit_artifacts.yml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/.github/workflows/runtime_commit_artifacts.yml b/.github/workflows/runtime_commit_artifacts.yml
index 073c289843fe1..f47eb3ff360e9 100644
--- a/.github/workflows/runtime_commit_artifacts.yml
+++ b/.github/workflows/runtime_commit_artifacts.yml
@@ -87,7 +87,7 @@ jobs:
             build/oss-experimental/react-refresh/cjs/react-refresh-babel.development.js
       - name: Insert @headers into eslint plugin and react-refresh
         run: |
-          sed -i -e 's/ LICENSE file in the root directory of this source tree./ LICENSE file in the root directory of this source tree.\n * \n * @noformat\n * @nolint\n * @lightSyntaxTransform\n * @preventMunge\n * @oncall react_core/' \
+          sed -i -e 's/ LICENSE file in the root directory of this source tree./ LICENSE file in the root directory of this source tree.\n *\n * @noformat\n * @nolint\n * @lightSyntaxTransform\n * @preventMunge\n * @oncall react_core/' \
             build/oss-experimental/eslint-plugin-react-hooks/cjs/eslint-plugin-react-hooks.development.js \
             build/oss-experimental/react-refresh/cjs/react-refresh-babel.development.js
       - name: Move relevant files for React in www into compiled

From e44685e4f196f9e19c3729ab2b3772a40428ac1e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= <sebastian@calyptus.eu>
Date: Mon, 26 Aug 2024 20:50:43 -0400
Subject: [PATCH 03/10] [DevTools] Use Owner Stacks to Implement View Source of
 a Server Component (#30798)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

We don't have the source location of Server Components on the client
because we don't want to eagerly do the throw trick for all Server
Components just in case. Unfortunately Node.js doesn't expose V8's API
to get a source location of a function.

We do have the owner stacks of the JSX that created it though and at
some point we'll also show that location in DevTools.

However, the realization is that if a Server Component is the owner of
any child. The owner stack of that child will have the owner component's
source location as its bottom stack frame.

The technique I'm implementing here is to track whenever a child mounts
we already have its owner. We track the first discovered owned child's
stack on the owner. Then when we ask for a Source location of the owner
do we parse that stack and extract the location of the bottom frame.
This doesn't give us a location necessarily in the top of the function
but somewhere in the function.

In this case the first owned child is the Container:

<img width="1107" alt="Screenshot 2024-08-22 at 10 24 42 PM"
src="https://github.com/user-attachments/assets/95f32850-24a5-4151-8ce6-b7b89db68aee">
<img width="648" alt="Screenshot 2024-08-22 at 10 24 20 PM"
src="https://github.com/user-attachments/assets/4bcba033-866f-4684-9beb-de09d189deff">

We can even use this technique for Fibers too. Currently I use this as a
fallback in case the error technique didn't work. This covers a case
where nothing errors but you still render a child. This case is actually
quite common:

```
function Foo() {
  return <Bar />;
}
```

However, for Fibers we could really just use the `inspect(function)`
technique which works for all cases. At least in Chrome.

Unfortunately, this technique doesn't work if a Component doesn't create
any new JSX but just renders its children. It also currently doesn't
work if the child is filtered since I only look up the owner if an
instance is not filtered. This means that the container in the fixture
can't view source by default since the host component is filtered:

```
export default function Container({children}) {
  return <div>{children}</div>;
}
```

<img width="1107" alt="Screenshot 2024-08-22 at 10 24 35 PM"
src="https://github.com/user-attachments/assets/c3f8f9c5-5add-4d35-9290-3a5079e82adc">
---
 .../fiber/DevToolsFiberComponentStack.js      |  17 +++
 .../src/backend/fiber/renderer.js             | 123 +++++++++++++-----
 2 files changed, 104 insertions(+), 36 deletions(-)

diff --git a/packages/react-devtools-shared/src/backend/fiber/DevToolsFiberComponentStack.js b/packages/react-devtools-shared/src/backend/fiber/DevToolsFiberComponentStack.js
index 5a1fc4ee5337d..17475cab2860f 100644
--- a/packages/react-devtools-shared/src/backend/fiber/DevToolsFiberComponentStack.js
+++ b/packages/react-devtools-shared/src/backend/fiber/DevToolsFiberComponentStack.js
@@ -108,6 +108,23 @@ export function getStackByFiberInDevAndProd(
   }
 }
 
+export function getSourceLocationByFiber(
+  workTagMap: WorkTagMap,
+  fiber: Fiber,
+  currentDispatcherRef: CurrentDispatcherRef,
+): null | string {
+  // This is like getStackByFiberInDevAndProd but just the first stack frame.
+  try {
+    const info = describeFiber(workTagMap, fiber, currentDispatcherRef);
+    if (info !== '') {
+      return info.slice(1); // skip the leading newline
+    }
+  } catch (x) {
+    console.error(x);
+  }
+  return null;
+}
+
 export function supportsConsoleTasks(fiber: Fiber): boolean {
   // If this Fiber supports native console.createTask then we are already running
   // inside a native async stack trace if it's active - meaning the DevTools is open.
diff --git a/packages/react-devtools-shared/src/backend/fiber/renderer.js b/packages/react-devtools-shared/src/backend/fiber/renderer.js
index 57eafa176f550..ac722a019cd4f 100644
--- a/packages/react-devtools-shared/src/backend/fiber/renderer.js
+++ b/packages/react-devtools-shared/src/backend/fiber/renderer.js
@@ -101,6 +101,14 @@ import {
 import {enableStyleXFeatures} from 'react-devtools-feature-flags';
 import is from 'shared/objectIs';
 import hasOwnProperty from 'shared/hasOwnProperty';
+
+// $FlowFixMe[method-unbinding]
+const toString = Object.prototype.toString;
+
+function isError(object: mixed) {
+  return toString.call(object) === '[object Error]';
+}
+
 import {getStyleXData} from '../StyleX/utils';
 import {createProfilingHooks} from '../profilingHooks';
 
@@ -131,7 +139,8 @@ import type {
   Plugins,
 } from 'react-devtools-shared/src/frontend/types';
 import type {Source} from 'react-devtools-shared/src/shared/types';
-import {getStackByFiberInDevAndProd} from './DevToolsFiberComponentStack';
+import {getSourceLocationByFiber} from './DevToolsFiberComponentStack';
+import {formatOwnerStack} from '../shared/DevToolsOwnerStack';
 
 // Kinds
 const FIBER_INSTANCE = 0;
@@ -152,7 +161,7 @@ type FiberInstance = {
   previousSibling: null | DevToolsInstance, // filtered next sibling, including virtual
   nextSibling: null | DevToolsInstance, // filtered next sibling, including virtual
   flags: number, // Force Error/Suspense
-  componentStack: null | string,
+  source: null | string | Error | Source, // source location of this component function, or owned child stack
   errors: null | Map<string, number>, // error messages and count
   warnings: null | Map<string, number>, // warning messages and count
   data: Fiber, // one of a Fiber pair
@@ -167,7 +176,7 @@ function createFiberInstance(fiber: Fiber): FiberInstance {
     previousSibling: null,
     nextSibling: null,
     flags: 0,
-    componentStack: null,
+    source: null,
     errors: null,
     warnings: null,
     data: fiber,
@@ -187,7 +196,7 @@ type VirtualInstance = {
   previousSibling: null | DevToolsInstance, // filtered next sibling, including virtual
   nextSibling: null | DevToolsInstance, // filtered next sibling, including virtual
   flags: number,
-  componentStack: null | string,
+  source: null | string | Error | Source, // source location of this server component, or owned child stack
   // Errors and Warnings happen per ReactComponentInfo which can appear in
   // multiple places but we track them per stateful VirtualInstance so
   // that old errors/warnings don't disappear when the instance is refreshed.
@@ -209,7 +218,7 @@ function createVirtualInstance(
     previousSibling: null,
     nextSibling: null,
     flags: 0,
-    componentStack: null,
+    source: null,
     errors: null,
     warnings: null,
     data: debugEntry,
@@ -2154,6 +2163,16 @@ export function attach(
         parentInstance,
         debugOwner,
       );
+      if (
+        ownerInstance !== null &&
+        debugOwner === fiber._debugOwner &&
+        fiber._debugStack != null &&
+        ownerInstance.source === null
+      ) {
+        // The new Fiber is directly owned by the ownerInstance. Therefore somewhere on
+        // the debugStack will be a stack frame inside the ownerInstance's source.
+        ownerInstance.source = fiber._debugStack;
+      }
       const ownerID = ownerInstance === null ? 0 : ownerInstance.id;
       const parentID = parentInstance ? parentInstance.id : 0;
 
@@ -2228,6 +2247,16 @@ export function attach(
     // away so maybe it's not so bad.
     const debugOwner = getUnfilteredOwner(componentInfo);
     const ownerInstance = findNearestOwnerInstance(parentInstance, debugOwner);
+    if (
+      ownerInstance !== null &&
+      debugOwner === componentInfo.owner &&
+      componentInfo.debugStack != null &&
+      ownerInstance.source === null
+    ) {
+      // The new Fiber is directly owned by the ownerInstance. Therefore somewhere on
+      // the debugStack will be a stack frame inside the ownerInstance's source.
+      ownerInstance.source = componentInfo.debugStack;
+    }
     const ownerID = ownerInstance === null ? 0 : ownerInstance.id;
     const parentID = parentInstance ? parentInstance.id : 0;
 
@@ -4324,7 +4353,7 @@ export function attach(
 
     let source = null;
     if (canViewSource) {
-      source = getSourceForFiber(fiber);
+      source = getSourceForFiberInstance(fiberInstance);
     }
 
     return {
@@ -4398,7 +4427,8 @@ export function attach(
   function inspectVirtualInstanceRaw(
     virtualInstance: VirtualInstance,
   ): InspectedElement | null {
-    const canViewSource = false;
+    const canViewSource = true;
+    const source = getSourceForInstance(virtualInstance);
 
     const componentInfo = virtualInstance.data;
     const key =
@@ -4438,9 +4468,6 @@ export function attach(
       stylex: null,
     };
 
-    // TODO: Support getting the source location from the owner stack.
-    const source = null;
-
     return {
       id: virtualInstance.id,
 
@@ -5664,39 +5691,63 @@ export function attach(
     return idToDevToolsInstanceMap.has(id);
   }
 
-  function getComponentStackForFiber(fiber: Fiber): string | null {
-    // TODO: This should really just take an DevToolsInstance directly.
-    let fiberInstance = fiberToFiberInstanceMap.get(fiber);
-    if (fiberInstance === undefined && fiber.alternate !== null) {
-      fiberInstance = fiberToFiberInstanceMap.get(fiber.alternate);
-    }
-    if (fiberInstance === undefined) {
-      // We're no longer tracking this instance.
-      return null;
-    }
-    if (fiberInstance.componentStack !== null) {
-      // Cached entry.
-      return fiberInstance.componentStack;
+  function getSourceForFiberInstance(
+    fiberInstance: FiberInstance,
+  ): Source | null {
+    const unresolvedSource = fiberInstance.source;
+    if (
+      unresolvedSource !== null &&
+      typeof unresolvedSource === 'object' &&
+      !isError(unresolvedSource)
+    ) {
+      // $FlowFixMe: isError should have refined it.
+      return unresolvedSource;
     }
     const dispatcherRef = getDispatcherRef(renderer);
-    if (dispatcherRef == null) {
+    const stackFrame =
+      dispatcherRef == null
+        ? null
+        : getSourceLocationByFiber(
+            ReactTypeOfWork,
+            fiberInstance.data,
+            dispatcherRef,
+          );
+    if (stackFrame === null) {
+      // If we don't find a source location by throwing, try to get one
+      // from an owned child if possible. This is the same branch as
+      // for virtual instances.
+      return getSourceForInstance(fiberInstance);
+    }
+    const source = parseSourceFromComponentStack(stackFrame);
+    fiberInstance.source = source;
+    return source;
+  }
+
+  function getSourceForInstance(instance: DevToolsInstance): Source | null {
+    let unresolvedSource = instance.source;
+    if (unresolvedSource === null) {
+      // We don't have any source yet. We can try again later in case an owned child mounts later.
+      // TODO: We won't have any information here if the child is filtered.
       return null;
     }
 
-    return (fiberInstance.componentStack = getStackByFiberInDevAndProd(
-      ReactTypeOfWork,
-      fiber,
-      dispatcherRef,
-    ));
-  }
-
-  function getSourceForFiber(fiber: Fiber): Source | null {
-    const componentStack = getComponentStackForFiber(fiber);
-    if (componentStack == null) {
-      return null;
+    // If we have the debug stack (the creation stack of the JSX) for any owned child of this
+    // component, then at the bottom of that stack will be a stack frame that is somewhere within
+    // the component's function body. Typically it would be the callsite of the JSX unless there's
+    // any intermediate utility functions. This won't point to the top of the component function
+    // but it's at least somewhere within it.
+    if (isError(unresolvedSource)) {
+      unresolvedSource = formatOwnerStack((unresolvedSource: any));
+    }
+    if (typeof unresolvedSource === 'string') {
+      const idx = unresolvedSource.lastIndexOf('\n');
+      const lastLine =
+        idx === -1 ? unresolvedSource : unresolvedSource.slice(idx + 1);
+      return (instance.source = parseSourceFromComponentStack(lastLine));
     }
 
-    return parseSourceFromComponentStack(componentStack);
+    // $FlowFixMe: refined.
+    return unresolvedSource;
   }
 
   return {

From 246d7bfeb0c90ecccd9531929b60a79d628a4c78 Mon Sep 17 00:00:00 2001
From: Benoit Girard <bgirard@fb.com>
Date: Mon, 26 Aug 2024 22:06:02 -0400
Subject: [PATCH 04/10] Enable suspenseCallback on React Native (#29210)

## Summary

suspenseCallback feature has proved to be useful for FB Web. Let's look
at enabling the feature for the React Native build.


## How did you test this change?

Will sync the react changes with a React Native build and will verify
that performance logging is correctly notified of suspense promises
during the suspense callback.
---
 .../src/__tests__/ReactSuspenseCallback-test.js        | 10 +++++-----
 packages/shared/forks/ReactFeatureFlags.native-fb.js   |  2 +-
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/packages/react-reconciler/src/__tests__/ReactSuspenseCallback-test.js b/packages/react-reconciler/src/__tests__/ReactSuspenseCallback-test.js
index eae080d35c0ac..6ddc94905a52a 100644
--- a/packages/react-reconciler/src/__tests__/ReactSuspenseCallback-test.js
+++ b/packages/react-reconciler/src/__tests__/ReactSuspenseCallback-test.js
@@ -46,7 +46,7 @@ describe('ReactSuspense', () => {
 
   // Warning don't fire in production, so this test passes in prod even if
   // the suspenseCallback feature is not enabled
-  // @gate www || !__DEV__
+  // @gate enableSuspenseCallback || !__DEV__
   it('check type', async () => {
     const {PromiseComp} = createThenable();
 
@@ -71,7 +71,7 @@ describe('ReactSuspense', () => {
     await expect(async () => await waitForAll([])).toErrorDev([]);
   });
 
-  // @gate www
+  // @gate enableSuspenseCallback
   it('1 then 0 suspense callback', async () => {
     const {promise, resolve, PromiseComp} = createThenable();
 
@@ -98,7 +98,7 @@ describe('ReactSuspense', () => {
     expect(ops).toEqual([]);
   });
 
-  // @gate www
+  // @gate enableSuspenseCallback
   it('2 then 1 then 0 suspense callback', async () => {
     const {
       promise: promise1,
@@ -145,7 +145,7 @@ describe('ReactSuspense', () => {
     expect(ops).toEqual([]);
   });
 
-  // @gate www
+  // @gate enableSuspenseCallback
   it('nested suspense promises are reported only for their tier', async () => {
     const {promise, PromiseComp} = createThenable();
 
@@ -177,7 +177,7 @@ describe('ReactSuspense', () => {
     expect(ops2).toEqual([new Set([promise])]);
   });
 
-  // @gate www
+  // @gate enableSuspenseCallback
   it('competing suspense promises', async () => {
     const {
       promise: promise1,
diff --git a/packages/shared/forks/ReactFeatureFlags.native-fb.js b/packages/shared/forks/ReactFeatureFlags.native-fb.js
index 6ba5a79ff0f39..61138ddbee6c1 100644
--- a/packages/shared/forks/ReactFeatureFlags.native-fb.js
+++ b/packages/shared/forks/ReactFeatureFlags.native-fb.js
@@ -80,7 +80,7 @@ export const enableScopeAPI = false;
 export const enableServerComponentLogs = true;
 export const enableSuspenseAvoidThisFallback = false;
 export const enableSuspenseAvoidThisFallbackFizz = false;
-export const enableSuspenseCallback = false;
+export const enableSuspenseCallback = true;
 export const enableTaint = true;
 export const enableTransitionTracing = false;
 export const enableTrustedTypesIntegration = false;

From 1a8f92a8699e79966e65841fcb9110bba4c3df7f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= <sebastian@calyptus.eu>
Date: Tue, 27 Aug 2024 12:05:10 -0400
Subject: [PATCH 05/10] [DevTools] Track Tree Base Duration of Virtual
 Instances (#30817)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

These don't have their own time since they don't take up any time to
render but they show up in the tree for context. However they never
render themselves. Their base tree time is the base time of their
children. This way they take up the same space as their combined
children in the Profiler tree. (Instead of leaving a blank line which
they did before this PR.)

The frontend doesn't track the difference between a virtual instance and
a Fiber that didn't render this update. This might be a bit confusing as
to why it didn't render. I add the word "client" to make it a bit
clearer and works for both. We should probably have different verbiage
here based on it is a Server Component or something else.

<img width="1103" alt="Screenshot 2024-08-26 at 5 00 47 PM"
src="https://github.com/user-attachments/assets/87b811d4-7024-466a-845d-542493ed3ca2">

I also took the opportunity to remove idToTreeBaseDurationMap and
idToRootMap maps. Cloning the Map isn't really all that super fast
anyway and it means we have to maintain the map continuously as we
render. Instead, we can track it on the instances and then walk the
instances to create a snapshot when starting to profile. This isn't as
fast but really fast too and requires less bookkeeping while rendering
instead which is more sensitive than that one snapshot in the beginning.
---
 .../src/backend/fiber/renderer.js             | 131 ++++++++++--------
 .../views/Profiler/HoveredFiberInfo.js        |   2 +-
 .../Profiler/SidebarSelectedFiberInfo.js      |   2 +-
 3 files changed, 77 insertions(+), 58 deletions(-)

diff --git a/packages/react-devtools-shared/src/backend/fiber/renderer.js b/packages/react-devtools-shared/src/backend/fiber/renderer.js
index ac722a019cd4f..bb5b1b559d5f3 100644
--- a/packages/react-devtools-shared/src/backend/fiber/renderer.js
+++ b/packages/react-devtools-shared/src/backend/fiber/renderer.js
@@ -164,6 +164,7 @@ type FiberInstance = {
   source: null | string | Error | Source, // source location of this component function, or owned child stack
   errors: null | Map<string, number>, // error messages and count
   warnings: null | Map<string, number>, // warning messages and count
+  treeBaseDuration: number, // the profiled time of the last render of this subtree
   data: Fiber, // one of a Fiber pair
 };
 
@@ -179,6 +180,7 @@ function createFiberInstance(fiber: Fiber): FiberInstance {
     source: null,
     errors: null,
     warnings: null,
+    treeBaseDuration: 0,
     data: fiber,
   };
 }
@@ -202,6 +204,7 @@ type VirtualInstance = {
   // that old errors/warnings don't disappear when the instance is refreshed.
   errors: null | Map<string, number>, // error messages and count
   warnings: null | Map<string, number>, // warning messages and count
+  treeBaseDuration: number, // the profiled time of the last render of this subtree
   // The latest info for this instance. This can be updated over time and the
   // same info can appear in more than once ServerComponentInstance.
   data: ReactComponentInfo,
@@ -221,6 +224,7 @@ function createVirtualInstance(
     source: null,
     errors: null,
     warnings: null,
+    treeBaseDuration: 0,
     data: debugEntry,
   };
 }
@@ -1355,16 +1359,6 @@ export function attach(
     }
   }
 
-  // When profiling is supported, we store the latest tree base durations for each Fiber.
-  // This is so that we can quickly capture a snapshot of those values if profiling starts.
-  // If we didn't store these values, we'd have to crawl the tree when profiling started,
-  // and use a slow path to find each of the current Fibers.
-  const idToTreeBaseDurationMap: Map<number, number> = new Map();
-
-  // When profiling is supported, we store the latest tree base durations for each Fiber.
-  // This map enables us to filter these times by root when sending them to the frontend.
-  const idToRootMap: Map<number, number> = new Map();
-
   // When a mount or update is in progress, this value tracks the root that is being operated on.
   let currentRootID: number = -1;
 
@@ -2211,9 +2205,7 @@ export function attach(
     }
 
     if (isProfilingSupported) {
-      idToRootMap.set(id, currentRootID);
-
-      recordProfilingDurations(fiber);
+      recordProfilingDurations(fiberInstance);
     }
     return fiberInstance;
   }
@@ -2226,8 +2218,6 @@ export function attach(
 
     idToDevToolsInstanceMap.set(id, instance);
 
-    const isProfilingSupported = false; // TODO: Support Tree Base Duration Based on Children.
-
     const componentInfo = instance.data;
 
     const key =
@@ -2274,12 +2264,6 @@ export function attach(
     pushOperation(ownerID);
     pushOperation(displayNameStringID);
     pushOperation(keyStringID);
-
-    if (isProfilingSupported) {
-      idToRootMap.set(id, currentRootID);
-      // TODO: Include tree base duration of children somehow.
-      // recordProfilingDurations(...);
-    }
   }
 
   function recordUnmount(fiberInstance: FiberInstance): void {
@@ -2314,12 +2298,6 @@ export function attach(
     }
 
     untrackFiber(fiberInstance);
-
-    const isProfilingSupported = fiber.hasOwnProperty('treeBaseDuration');
-    if (isProfilingSupported) {
-      idToRootMap.delete(id);
-      idToTreeBaseDurationMap.delete(id);
-    }
   }
 
   // Running state of the remaining children from the previous version of this parent that
@@ -2430,6 +2408,8 @@ export function attach(
         traceNearestHostComponentUpdate,
         virtualLevel + 1,
       );
+      // Must be called after all children have been appended.
+      recordVirtualProfilingDurations(virtualInstance);
     } finally {
       reconcilingParent = stashedParent;
       previouslyReconciledSibling = stashedPrevious;
@@ -2445,12 +2425,6 @@ export function attach(
 
     const id = instance.id;
     pendingRealUnmountedIDs.push(id);
-
-    const isProfilingSupported = false; // TODO: Profiling support.
-    if (isProfilingSupported) {
-      idToRootMap.delete(id);
-      idToTreeBaseDurationMap.delete(id);
-    }
   }
 
   function mountVirtualChildrenRecursively(
@@ -2684,11 +2658,12 @@ export function attach(
     removeChild(instance);
   }
 
-  function recordProfilingDurations(fiber: Fiber) {
-    const id = getFiberIDThrows(fiber);
+  function recordProfilingDurations(fiberInstance: FiberInstance) {
+    const id = fiberInstance.id;
+    const fiber = fiberInstance.data;
     const {actualDuration, treeBaseDuration} = fiber;
 
-    idToTreeBaseDurationMap.set(id, treeBaseDuration || 0);
+    fiberInstance.treeBaseDuration = treeBaseDuration || 0;
 
     if (isProfiling) {
       const {alternate} = fiber;
@@ -2751,6 +2726,38 @@ export function attach(
     }
   }
 
+  function recordVirtualProfilingDurations(virtualInstance: VirtualInstance) {
+    const id = virtualInstance.id;
+
+    let treeBaseDuration = 0;
+    // Add up the base duration of the child instances. The virtual base duration
+    // will be the same as children's duration since we don't take up any render
+    // time in the virtual instance.
+    for (
+      let child = virtualInstance.firstChild;
+      child !== null;
+      child = child.nextSibling
+    ) {
+      treeBaseDuration += child.treeBaseDuration;
+    }
+
+    if (isProfiling) {
+      const previousTreeBaseDuration = virtualInstance.treeBaseDuration;
+      if (treeBaseDuration !== previousTreeBaseDuration) {
+        // Tree base duration updates are included in the operations typed array.
+        // So we have to convert them from milliseconds to microseconds so we can send them as ints.
+        const convertedTreeBaseDuration = Math.floor(
+          (treeBaseDuration || 0) * 1000,
+        );
+        pushOperation(TREE_OPERATION_UPDATE_TREE_BASE_DURATION);
+        pushOperation(id);
+        pushOperation(convertedTreeBaseDuration);
+      }
+    }
+
+    virtualInstance.treeBaseDuration = treeBaseDuration;
+  }
+
   function recordResetChildren(parentInstance: DevToolsInstance) {
     if (__DEBUG__) {
       if (
@@ -2818,6 +2825,8 @@ export function attach(
       ) {
         recordResetChildren(virtualInstance);
       }
+      // Must be called after all children have been appended.
+      recordVirtualProfilingDurations(virtualInstance);
     } finally {
       unmountRemainingChildren();
       reconcilingParent = stashedParent;
@@ -3265,11 +3274,11 @@ export function attach(
         }
       }
 
-      if (shouldIncludeInTree) {
+      if (fiberInstance !== null) {
         const isProfilingSupported =
           nextFiber.hasOwnProperty('treeBaseDuration');
         if (isProfilingSupported) {
-          recordProfilingDurations(nextFiber);
+          recordProfilingDurations(fiberInstance);
         }
       }
       if (shouldResetChildren) {
@@ -5121,8 +5130,8 @@ export function attach(
   let currentCommitProfilingMetadata: CommitProfilingData | null = null;
   let displayNamesByRootID: DisplayNamesByRootID | null = null;
   let idToContextsMap: Map<number, any> | null = null;
-  let initialTreeBaseDurationsMap: Map<number, number> | null = null;
-  let initialIDToRootMap: Map<number, number> | null = null;
+  let initialTreeBaseDurationsMap: Map<number, Array<[number, number]>> | null =
+    null;
   let isProfiling: boolean = false;
   let profilingStartTime: number = 0;
   let recordChangeDescriptions: boolean = false;
@@ -5141,24 +5150,15 @@ export function attach(
     rootToCommitProfilingMetadataMap.forEach(
       (commitProfilingMetadata, rootID) => {
         const commitData: Array<CommitDataBackend> = [];
-        const initialTreeBaseDurations: Array<[number, number]> = [];
 
         const displayName =
           (displayNamesByRootID !== null && displayNamesByRootID.get(rootID)) ||
           'Unknown';
 
-        if (initialTreeBaseDurationsMap != null) {
-          initialTreeBaseDurationsMap.forEach((treeBaseDuration, id) => {
-            if (
-              initialIDToRootMap != null &&
-              initialIDToRootMap.get(id) === rootID
-            ) {
-              // We don't need to convert milliseconds to microseconds in this case,
-              // because the profiling summary is JSON serialized.
-              initialTreeBaseDurations.push([id, treeBaseDuration]);
-            }
-          });
-        }
+        const initialTreeBaseDurations: Array<[number, number]> =
+          (initialTreeBaseDurationsMap !== null &&
+            initialTreeBaseDurationsMap.get(rootID)) ||
+          [];
 
         commitProfilingMetadata.forEach((commitProfilingData, commitIndex) => {
           const {
@@ -5245,6 +5245,22 @@ export function attach(
     };
   }
 
+  function snapshotTreeBaseDurations(
+    instance: DevToolsInstance,
+    target: Array<[number, number]>,
+  ) {
+    // We don't need to convert milliseconds to microseconds in this case,
+    // because the profiling summary is JSON serialized.
+    target.push([instance.id, instance.treeBaseDuration]);
+    for (
+      let child = instance.firstChild;
+      child !== null;
+      child = child.nextSibling
+    ) {
+      snapshotTreeBaseDurations(child, target);
+    }
+  }
+
   function startProfiling(shouldRecordChangeDescriptions: boolean) {
     if (isProfiling) {
       return;
@@ -5257,16 +5273,19 @@ export function attach(
     // since either of these may change during the profiling session
     // (e.g. when a fiber is re-rendered or when a fiber gets removed).
     displayNamesByRootID = new Map();
-    initialTreeBaseDurationsMap = new Map(idToTreeBaseDurationMap);
-    initialIDToRootMap = new Map(idToRootMap);
+    initialTreeBaseDurationsMap = new Map();
     idToContextsMap = new Map();
 
     hook.getFiberRoots(rendererID).forEach(root => {
-      const rootID = getFiberIDThrows(root.current);
+      const rootInstance = getFiberInstanceThrows(root.current);
+      const rootID = rootInstance.id;
       ((displayNamesByRootID: any): DisplayNamesByRootID).set(
         rootID,
         getDisplayNameForRoot(root.current),
       );
+      const initialTreeBaseDurations: Array<[number, number]> = [];
+      snapshotTreeBaseDurations(rootInstance, initialTreeBaseDurations);
+      (initialTreeBaseDurationsMap: any).set(rootID, initialTreeBaseDurations);
 
       if (shouldRecordChangeDescriptions) {
         // Record all contexts at the time profiling is started.
diff --git a/packages/react-devtools-shared/src/devtools/views/Profiler/HoveredFiberInfo.js b/packages/react-devtools-shared/src/devtools/views/Profiler/HoveredFiberInfo.js
index 51f82038d3489..f1156a05aee14 100644
--- a/packages/react-devtools-shared/src/devtools/views/Profiler/HoveredFiberInfo.js
+++ b/packages/react-devtools-shared/src/devtools/views/Profiler/HoveredFiberInfo.js
@@ -93,7 +93,7 @@ export default function HoveredFiberInfo({fiberData}: Props): React.Node {
         )}
 
         <div className={styles.Content}>
-          {renderDurationInfo || <div>Did not render.</div>}
+          {renderDurationInfo || <div>Did not client render.</div>}
 
           <WhatChanged fiberID={id} />
         </div>
diff --git a/packages/react-devtools-shared/src/devtools/views/Profiler/SidebarSelectedFiberInfo.js b/packages/react-devtools-shared/src/devtools/views/Profiler/SidebarSelectedFiberInfo.js
index 2eca7b0fdad3d..d785cfebdacde 100644
--- a/packages/react-devtools-shared/src/devtools/views/Profiler/SidebarSelectedFiberInfo.js
+++ b/packages/react-devtools-shared/src/devtools/views/Profiler/SidebarSelectedFiberInfo.js
@@ -142,7 +142,7 @@ export default function SidebarSelectedFiberInfo(): React.Node {
           </div>
         )}
         {listItems.length === 0 && (
-          <div>Did not render during this profiling session.</div>
+          <div>Did not render on the client during this profiling session.</div>
         )}
       </div>
     </Fragment>

From 9690b9ad749c30eab1900c99e7c25a7ed7e1d9b7 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= <sebastian@calyptus.eu>
Date: Tue, 27 Aug 2024 12:05:24 -0400
Subject: [PATCH 06/10] [DevTools] Remove
 findCurrentFiberUsingSlowPathByFiberInstance (#30818)

We always track the last committed Fiber on `FiberInstance.data`.


https://github.com/facebook/react/blob/dcae56f8b72f625d8affe5729ca9991b31a492ac/packages/react-devtools-shared/src/backend/fiber/renderer.js#L3068

So we can now remove this complex slow path to get the current fiber.
---
 .../src/backend/fiber/renderer.js             | 221 +-----------------
 1 file changed, 7 insertions(+), 214 deletions(-)

diff --git a/packages/react-devtools-shared/src/backend/fiber/renderer.js b/packages/react-devtools-shared/src/backend/fiber/renderer.js
index bb5b1b559d5f3..9e5a35686b34f 100644
--- a/packages/react-devtools-shared/src/backend/fiber/renderer.js
+++ b/packages/react-devtools-shared/src/backend/fiber/renderer.js
@@ -3550,7 +3550,7 @@ export function attach(
     fiberInstance: FiberInstance,
   ): $ReadOnlyArray<HostInstance> {
     const hostInstances = [];
-    const fiber = findCurrentFiberUsingSlowPathByFiberInstance(fiberInstance);
+    const fiber = fiberInstance.data;
     if (!fiber) {
       return hostInstances;
     }
@@ -3601,8 +3601,7 @@ export function attach(
         // TODO: Handle VirtualInstance.
         return null;
       }
-      const fiber =
-        findCurrentFiberUsingSlowPathByFiberInstance(devtoolsInstance);
+      const fiber = devtoolsInstance.data;
       if (fiber === null) {
         return null;
       }
@@ -3710,208 +3709,6 @@ export function attach(
     return null;
   }
 
-  // This function is copied from React and should be kept in sync:
-  // https://github.com/facebook/react/blob/main/packages/react-reconciler/src/ReactFiberTreeReflection.js
-  function assertIsMounted(fiber: Fiber) {
-    if (getNearestMountedFiber(fiber) !== fiber) {
-      throw new Error('Unable to find node on an unmounted component.');
-    }
-  }
-
-  // This function is copied from React and should be kept in sync:
-  // https://github.com/facebook/react/blob/main/packages/react-reconciler/src/ReactFiberTreeReflection.js
-  function getNearestMountedFiber(fiber: Fiber): null | Fiber {
-    let node = fiber;
-    let nearestMounted: null | Fiber = fiber;
-    if (!fiber.alternate) {
-      // If there is no alternate, this might be a new tree that isn't inserted
-      // yet. If it is, then it will have a pending insertion effect on it.
-      let nextNode: Fiber = node;
-      do {
-        node = nextNode;
-        // TODO: This function, and these flags, are a leaked implementation
-        // detail. Once we start releasing DevTools in lockstep with React, we
-        // should import a function from the reconciler instead.
-        const Placement = 0b000000000000000000000000010;
-        const Hydrating = 0b000000000000001000000000000;
-        if ((node.flags & (Placement | Hydrating)) !== 0) {
-          // This is an insertion or in-progress hydration. The nearest possible
-          // mounted fiber is the parent but we need to continue to figure out
-          // if that one is still mounted.
-          nearestMounted = node.return;
-        }
-        // $FlowFixMe[incompatible-type] we bail out when we get a null
-        nextNode = node.return;
-      } while (nextNode);
-    } else {
-      while (node.return) {
-        node = node.return;
-      }
-    }
-    if (node.tag === HostRoot) {
-      // TODO: Check if this was a nested HostRoot when used with
-      // renderContainerIntoSubtree.
-      return nearestMounted;
-    }
-    // If we didn't hit the root, that means that we're in an disconnected tree
-    // that has been unmounted.
-    return null;
-  }
-
-  // This function is copied from React and should be kept in sync:
-  // https://github.com/facebook/react/blob/main/packages/react-reconciler/src/ReactFiberTreeReflection.js
-  // It would be nice if we updated React to inject this function directly (vs just indirectly via findDOMNode).
-  // BEGIN copied code
-  function findCurrentFiberUsingSlowPathByFiberInstance(
-    fiberInstance: FiberInstance,
-  ): Fiber | null {
-    const fiber = fiberInstance.data;
-    const alternate = fiber.alternate;
-    if (!alternate) {
-      // If there is no alternate, then we only need to check if it is mounted.
-      const nearestMounted = getNearestMountedFiber(fiber);
-
-      if (nearestMounted === null) {
-        throw new Error('Unable to find node on an unmounted component.');
-      }
-
-      if (nearestMounted !== fiber) {
-        return null;
-      }
-      return fiber;
-    }
-    // If we have two possible branches, we'll walk backwards up to the root
-    // to see what path the root points to. On the way we may hit one of the
-    // special cases and we'll deal with them.
-    let a: Fiber = fiber;
-    let b: Fiber = alternate;
-    while (true) {
-      const parentA = a.return;
-      if (parentA === null) {
-        // We're at the root.
-        break;
-      }
-      const parentB = parentA.alternate;
-      if (parentB === null) {
-        // There is no alternate. This is an unusual case. Currently, it only
-        // happens when a Suspense component is hidden. An extra fragment fiber
-        // is inserted in between the Suspense fiber and its children. Skip
-        // over this extra fragment fiber and proceed to the next parent.
-        const nextParent = parentA.return;
-        if (nextParent !== null) {
-          a = b = nextParent;
-          continue;
-        }
-        // If there's no parent, we're at the root.
-        break;
-      }
-
-      // If both copies of the parent fiber point to the same child, we can
-      // assume that the child is current. This happens when we bailout on low
-      // priority: the bailed out fiber's child reuses the current child.
-      if (parentA.child === parentB.child) {
-        let child = parentA.child;
-        while (child) {
-          if (child === a) {
-            // We've determined that A is the current branch.
-            assertIsMounted(parentA);
-            return fiber;
-          }
-          if (child === b) {
-            // We've determined that B is the current branch.
-            assertIsMounted(parentA);
-            return alternate;
-          }
-          child = child.sibling;
-        }
-
-        // We should never have an alternate for any mounting node. So the only
-        // way this could possibly happen is if this was unmounted, if at all.
-        throw new Error('Unable to find node on an unmounted component.');
-      }
-
-      if (a.return !== b.return) {
-        // The return pointer of A and the return pointer of B point to different
-        // fibers. We assume that return pointers never criss-cross, so A must
-        // belong to the child set of A.return, and B must belong to the child
-        // set of B.return.
-        a = parentA;
-        b = parentB;
-      } else {
-        // The return pointers point to the same fiber. We'll have to use the
-        // default, slow path: scan the child sets of each parent alternate to see
-        // which child belongs to which set.
-        //
-        // Search parent A's child set
-        let didFindChild = false;
-        let child = parentA.child;
-        while (child) {
-          if (child === a) {
-            didFindChild = true;
-            a = parentA;
-            b = parentB;
-            break;
-          }
-          if (child === b) {
-            didFindChild = true;
-            b = parentA;
-            a = parentB;
-            break;
-          }
-          child = child.sibling;
-        }
-        if (!didFindChild) {
-          // Search parent B's child set
-          child = parentB.child;
-          while (child) {
-            if (child === a) {
-              didFindChild = true;
-              a = parentB;
-              b = parentA;
-              break;
-            }
-            if (child === b) {
-              didFindChild = true;
-              b = parentB;
-              a = parentA;
-              break;
-            }
-            child = child.sibling;
-          }
-
-          if (!didFindChild) {
-            throw new Error(
-              'Child was not found in either parent set. This indicates a bug ' +
-                'in React related to the return pointer. Please file an issue.',
-            );
-          }
-        }
-      }
-
-      if (a.alternate !== b) {
-        throw new Error(
-          "Return fibers should always be each others' alternates. " +
-            'This error is likely caused by a bug in React. Please file an issue.',
-        );
-      }
-    }
-
-    // If the root is not a host container, we're in a disconnected tree. I.e.
-    // unmounted.
-    if (a.tag !== HostRoot) {
-      throw new Error('Unable to find node on an unmounted component.');
-    }
-
-    if (a.stateNode.current === a) {
-      // We've determined that A is the current branch.
-      return fiber;
-    }
-    // Otherwise B has to be current branch.
-    return alternate;
-  }
-
-  // END copied code
-
   function getElementAttributeByPath(
     id: number,
     path: Array<string | number>,
@@ -4096,8 +3893,7 @@ export function attach(
       return {instance, style};
     }
 
-    const fiber =
-      findCurrentFiberUsingSlowPathByFiberInstance(devtoolsInstance);
+    const fiber = devtoolsInstance.data;
     if (fiber !== null) {
       instance = fiber.stateNode;
 
@@ -4153,7 +3949,7 @@ export function attach(
   function inspectFiberInstanceRaw(
     fiberInstance: FiberInstance,
   ): InspectedElement | null {
-    const fiber = findCurrentFiberUsingSlowPathByFiberInstance(fiberInstance);
+    const fiber = fiberInstance.data;
     if (fiber == null) {
       return null;
     }
@@ -4913,8 +4709,7 @@ export function attach(
       // TODO: Handle VirtualInstance.
       return;
     }
-    const fiber =
-      findCurrentFiberUsingSlowPathByFiberInstance(devtoolsInstance);
+    const fiber = devtoolsInstance.data;
     if (fiber !== null) {
       const instance = fiber.stateNode;
 
@@ -4979,8 +4774,7 @@ export function attach(
       // TODO: Handle VirtualInstance.
       return;
     }
-    const fiber =
-      findCurrentFiberUsingSlowPathByFiberInstance(devtoolsInstance);
+    const fiber = devtoolsInstance.data;
     if (fiber !== null) {
       const instance = fiber.stateNode;
 
@@ -5055,8 +4849,7 @@ export function attach(
       // TODO: Handle VirtualInstance.
       return;
     }
-    const fiber =
-      findCurrentFiberUsingSlowPathByFiberInstance(devtoolsInstance);
+    const fiber = devtoolsInstance.data;
     if (fiber !== null) {
       const instance = fiber.stateNode;
 

From f90a6bcc4c988f7524ce2be675b3257a530a51e9 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= <sebastian@calyptus.eu>
Date: Tue, 27 Aug 2024 12:05:47 -0400
Subject: [PATCH 07/10] [DevTools] Reconcile Fibers Against Previous Children
 Instances (#30822)

This loops over the remainingReconcilingChildren to find existing
FiberInstances that match the updated Fiber. This is the same thing we
already do for virtual instances. This avoids the need for a
`fiberToFiberInstanceMap`.

This loop is fast but there is a downside when the children set is very
large and gets reordered with keys since we might have to loop over the
set multiple times to get to the instances in the bottom. If that
becomes a problem we can optimize it the same way ReactChildFiber does
which is to create a temporary Map only when the children don't line up
properly. That way everything except the first pass can use the Map but
there's no need to create it eagerly.

Now that we have the loop we don't need the previousSibling field so we
can save some memory there.
---
 .../src/backend/fiber/renderer.js             | 166 ++++++++++--------
 1 file changed, 89 insertions(+), 77 deletions(-)

diff --git a/packages/react-devtools-shared/src/backend/fiber/renderer.js b/packages/react-devtools-shared/src/backend/fiber/renderer.js
index 9e5a35686b34f..729c8fce5a7a5 100644
--- a/packages/react-devtools-shared/src/backend/fiber/renderer.js
+++ b/packages/react-devtools-shared/src/backend/fiber/renderer.js
@@ -158,7 +158,6 @@ type FiberInstance = {
   id: number,
   parent: null | DevToolsInstance, // filtered parent, including virtual
   firstChild: null | DevToolsInstance, // filtered first child, including virtual
-  previousSibling: null | DevToolsInstance, // filtered next sibling, including virtual
   nextSibling: null | DevToolsInstance, // filtered next sibling, including virtual
   flags: number, // Force Error/Suspense
   source: null | string | Error | Source, // source location of this component function, or owned child stack
@@ -174,7 +173,6 @@ function createFiberInstance(fiber: Fiber): FiberInstance {
     id: getUID(),
     parent: null,
     firstChild: null,
-    previousSibling: null,
     nextSibling: null,
     flags: 0,
     source: null,
@@ -195,7 +193,6 @@ type VirtualInstance = {
   id: number,
   parent: null | DevToolsInstance, // filtered parent, including virtual
   firstChild: null | DevToolsInstance, // filtered first child, including virtual
-  previousSibling: null | DevToolsInstance, // filtered next sibling, including virtual
   nextSibling: null | DevToolsInstance, // filtered next sibling, including virtual
   flags: number,
   source: null | string | Error | Source, // source location of this server component, or owned child stack
@@ -218,7 +215,6 @@ function createVirtualInstance(
     id: getUID(),
     parent: null,
     firstChild: null,
-    previousSibling: null,
     nextSibling: null,
     flags: 0,
     source: null,
@@ -1088,8 +1084,6 @@ export function attach(
         '  '.repeat(indent) + '- ' + instance.id + ' (' + name + ')',
         'parent',
         instance.parent === null ? ' ' : instance.parent.id,
-        'prev',
-        instance.previousSibling === null ? ' ' : instance.previousSibling.id,
         'next',
         instance.nextSibling === null ? ' ' : instance.nextSibling.id,
       );
@@ -2321,21 +2315,25 @@ export function attach(
     if (previouslyReconciledSibling === null) {
       previouslyReconciledSibling = instance;
       parentInstance.firstChild = instance;
-      instance.previousSibling = null;
     } else {
       previouslyReconciledSibling.nextSibling = instance;
-      instance.previousSibling = previouslyReconciledSibling;
       previouslyReconciledSibling = instance;
     }
     instance.nextSibling = null;
   }
 
-  function moveChild(instance: DevToolsInstance): void {
-    removeChild(instance);
+  function moveChild(
+    instance: DevToolsInstance,
+    previousSibling: null | DevToolsInstance,
+  ): void {
+    removeChild(instance, previousSibling);
     insertChild(instance);
   }
 
-  function removeChild(instance: DevToolsInstance): void {
+  function removeChild(
+    instance: DevToolsInstance,
+    previousSibling: null | DevToolsInstance,
+  ): void {
     if (instance.parent === null) {
       if (remainingReconcilingChildren === instance) {
         throw new Error(
@@ -2343,8 +2341,6 @@ export function attach(
         );
       } else if (instance.nextSibling !== null) {
         throw new Error('A deleted instance should not have next siblings');
-      } else if (instance.previousSibling !== null) {
-        throw new Error('A deleted instance should not have previous siblings');
       }
       // Already deleted.
       return;
@@ -2360,7 +2356,7 @@ export function attach(
     }
     // Remove an existing child from its current position, which we assume is in the
     // remainingReconcilingChildren set.
-    if (instance.previousSibling === null) {
+    if (previousSibling === null) {
       // We're first in the remaining set. Remove us.
       if (remainingReconcilingChildren !== instance) {
         throw new Error(
@@ -2369,13 +2365,9 @@ export function attach(
       }
       remainingReconcilingChildren = instance.nextSibling;
     } else {
-      instance.previousSibling.nextSibling = instance.nextSibling;
-    }
-    if (instance.nextSibling !== null) {
-      instance.nextSibling.previousSibling = instance.previousSibling;
+      previousSibling.nextSibling = instance.nextSibling;
     }
     instance.nextSibling = null;
-    instance.previousSibling = null;
     instance.parent = null;
   }
 
@@ -2655,7 +2647,7 @@ export function attach(
     } else {
       recordVirtualUnmount(instance);
     }
-    removeChild(instance);
+    removeChild(instance, null);
   }
 
   function recordProfilingDurations(fiberInstance: FiberInstance) {
@@ -2889,8 +2881,7 @@ export function attach(
                   );
                 }
               }
-              // TODO: Find the best matching existing child based on the key if defined.
-
+              let previousSiblingOfBestMatch = null;
               let bestMatch = remainingReconcilingChildren;
               if (componentInfo.key != null) {
                 // If there is a key try to find a matching key in the set.
@@ -2902,6 +2893,7 @@ export function attach(
                   ) {
                     break;
                   }
+                  previousSiblingOfBestMatch = bestMatch;
                   bestMatch = bestMatch.nextSibling;
                 }
               }
@@ -2916,7 +2908,7 @@ export function attach(
                 // with the same name, then we claim it and reuse it for this update.
                 // Update it with the latest entry.
                 bestMatch.data = componentInfo;
-                moveChild(bestMatch);
+                moveChild(bestMatch, previousSiblingOfBestMatch);
                 previousVirtualInstance = bestMatch;
                 previousVirtualInstanceWasMount = false;
               } else {
@@ -2965,42 +2957,93 @@ export function attach(
           }
           previousVirtualInstance = null;
         }
+
         // We've reached the end of the virtual levels, but not beyond,
         // and now continue with the regular fiber.
+
+        // Do a fast pass over the remaining children to find the previous instance.
+        // TODO: This doesn't have the best O(n) for a large set of children that are
+        // reordered. Consider using a temporary map if it's not the very next one.
+        let prevChild;
         if (prevChildAtSameIndex === nextChild) {
           // This set is unchanged. We're just going through it to place all the
           // children again.
+          prevChild = nextChild;
+        } else {
+          // We don't actually need to rely on the alternate here. We could also
+          // reconcile against stateNode, key or whatever. Doesn't have to be same
+          // Fiber pair.
+          prevChild = nextChild.alternate;
+        }
+        let previousSiblingOfExistingInstance = null;
+        let existingInstance = null;
+        if (prevChild !== null) {
+          existingInstance = remainingReconcilingChildren;
+          while (existingInstance !== null) {
+            if (existingInstance.data === prevChild) {
+              break;
+            }
+            previousSiblingOfExistingInstance = existingInstance;
+            existingInstance = existingInstance.nextSibling;
+          }
+        }
+        if (existingInstance !== null) {
+          // Common case. Match in the same parent.
+          const fiberInstance: FiberInstance = (existingInstance: any); // Only matches if it's a Fiber.
+
+          // We keep track if the order of the children matches the previous order.
+          // They are always different referentially, but if the instances line up
+          // conceptually we'll want to know that.
+          if (prevChild !== prevChildAtSameIndex) {
+            shouldResetChildren = true;
+          }
+
+          // Register the new alternate in case it's not already in.
+          fiberToFiberInstanceMap.set(nextChild, fiberInstance);
+
+          // Update the Fiber so we that we always keep the current Fiber on the data.
+          fiberInstance.data = nextChild;
+          moveChild(fiberInstance, previousSiblingOfExistingInstance);
+
           if (
             updateFiberRecursively(
+              fiberInstance,
               nextChild,
-              nextChild,
+              (prevChild: any),
               traceNearestHostComponentUpdate,
             )
           ) {
-            throw new Error('Updating the same fiber should not cause reorder');
+            // If a nested tree child order changed but it can't handle its own
+            // child order invalidation (e.g. because it's filtered out like host nodes),
+            // propagate the need to reset child order upwards to this Fiber.
+            shouldResetChildren = true;
           }
-        } else if (nextChild.alternate) {
-          const prevChild = nextChild.alternate;
+        } else if (prevChild !== null && shouldFilterFiber(nextChild)) {
+          // If this Fiber should be filtered, we need to still update its children.
+          // This relies on an alternate since we don't have an Instance with the previous
+          // child on it. Ideally, the reconciliation wouldn't need previous Fibers that
+          // are filtered from the tree.
           if (
             updateFiberRecursively(
+              null,
               nextChild,
               prevChild,
               traceNearestHostComponentUpdate,
             )
           ) {
-            // If a nested tree child order changed but it can't handle its own
-            // child order invalidation (e.g. because it's filtered out like host nodes),
-            // propagate the need to reset child order upwards to this Fiber.
-            shouldResetChildren = true;
-          }
-          // However we also keep track if the order of the children matches
-          // the previous order. They are always different referentially, but
-          // if the instances line up conceptually we'll want to know that.
-          if (prevChild !== prevChildAtSameIndex) {
             shouldResetChildren = true;
           }
         } else {
+          // It's possible for a FiberInstance to be reparented when virtual parents
+          // get their sequence split or change structure with the same render result.
+          // In this case we unmount the and remount the FiberInstances.
+          // This might cause us to lose the selection but it's an edge case.
+
+          // We let the previous instance remain in the "remaining queue" it is
+          // in to be deleted at the end since it'll have no match.
+
           mountFiberRecursively(nextChild, traceNearestHostComponentUpdate);
+          // Need to mark the parent set to remount the new instance.
           shouldResetChildren = true;
         }
       }
@@ -3059,6 +3102,7 @@ export function attach(
 
   // Returns whether closest unfiltered fiber parent needs to reset its child list.
   function updateFiberRecursively(
+    fiberInstance: null | FiberInstance, // null if this should be filtered
     nextFiber: Fiber,
     prevFiber: Fiber,
     traceNearestHostComponentUpdate: boolean,
@@ -3092,34 +3136,10 @@ export function attach(
       }
     }
 
-    let fiberInstance: null | FiberInstance = null;
-    const shouldIncludeInTree = !shouldFilterFiber(nextFiber);
-    if (shouldIncludeInTree) {
-      const entry = fiberToFiberInstanceMap.get(prevFiber);
-      if (entry !== undefined && entry.parent === reconcilingParent) {
-        // Common case. Match in the same parent.
-        fiberInstance = entry;
-        // Register the new alternate in case it's not already in.
-        fiberToFiberInstanceMap.set(nextFiber, fiberInstance);
-
-        // Update the Fiber so we that we always keep the current Fiber on the data.
-        fiberInstance.data = nextFiber;
-        moveChild(fiberInstance);
-      } else {
-        // It's possible for a FiberInstance to be reparented when virtual parents
-        // get their sequence split or change structure with the same render result.
-        // In this case we unmount the and remount the FiberInstances.
-        // This might cause us to lose the selection but it's an edge case.
-
-        // We let the previous instance remain in the "remaining queue" it is
-        // in to be deleted at the end since it'll have no match.
-
-        mountFiberRecursively(nextFiber, traceNearestHostComponentUpdate);
-
-        // Need to mark the parent set to remount the new instance.
-        return true;
-      }
-
+    const stashedParent = reconcilingParent;
+    const stashedPrevious = previouslyReconciledSibling;
+    const stashedRemaining = remainingReconcilingChildren;
+    if (fiberInstance !== null) {
       if (
         mostRecentlyInspectedElement !== null &&
         mostRecentlyInspectedElement.id === fiberInstance.id &&
@@ -3129,12 +3149,6 @@ export function attach(
         // If it is inspected again, it may need to be re-run to obtain updated hooks values.
         hasElementUpdatedSinceLastInspected = true;
       }
-    }
-
-    const stashedParent = reconcilingParent;
-    const stashedPrevious = previouslyReconciledSibling;
-    const stashedRemaining = remainingReconcilingChildren;
-    if (fiberInstance !== null) {
       // Push a new DevTools instance parent while reconciling this subtree.
       reconcilingParent = fiberInstance;
       previouslyReconciledSibling = null;
@@ -3189,7 +3203,7 @@ export function attach(
         if (
           nextFallbackChildSet != null &&
           prevFallbackChildSet != null &&
-          updateFiberRecursively(
+          updateChildrenRecursively(
             nextFallbackChildSet,
             prevFallbackChildSet,
             traceNearestHostComponentUpdate,
@@ -3284,10 +3298,8 @@ export function attach(
       if (shouldResetChildren) {
         // We need to crawl the subtree for closest non-filtered Fibers
         // so that we can display them in a flat children set.
-        if (shouldIncludeInTree) {
-          if (reconcilingParent !== null) {
-            recordResetChildren(reconcilingParent);
-          }
+        if (fiberInstance !== null) {
+          recordResetChildren(fiberInstance);
           // We've handled the child order change for this Fiber.
           // Since it's included, there's no need to invalidate parent child order.
           return false;
@@ -3299,7 +3311,7 @@ export function attach(
         return false;
       }
     } finally {
-      if (shouldIncludeInTree) {
+      if (fiberInstance !== null) {
         unmountRemainingChildren();
         reconcilingParent = stashedParent;
         previouslyReconciledSibling = stashedPrevious;
@@ -3489,7 +3501,7 @@ export function attach(
         mountFiberRecursively(current, false);
       } else if (wasMounted && isMounted) {
         // Update an existing root.
-        updateFiberRecursively(current, alternate, false);
+        updateFiberRecursively(rootInstance, current, alternate, false);
       } else if (wasMounted && !isMounted) {
         // Unmount an existing root.
         removeRootPseudoKey(currentRootID);

From 96aca5f4f3d7fbe0c13350f90031d8ec4c060ccb Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= <sebastian@calyptus.eu>
Date: Tue, 27 Aug 2024 13:10:37 -0400
Subject: [PATCH 08/10] Spawn new task if we hit stack overflow (#30419)

If we see the "Maximum call stack size exceeded" error we know we've hit
stack overflow. We can recover from this by spawning a new task and
trying again. Effectively a zero-cost trampoline in the normal case. The
new task will have a clean stack. If you have a lot of siblings at the
same depth that hits the limit you can end up hitting this once for each
sibling but within that new sibling you're unlikely to hit this again.
So it's not too expensive.

If it errors again in the retryTask pass, the other error handling takes
over which causes this to be able to still not infinitely stall. E.g.
when the component itself throws an error like this.

It's still better to increase the stack limit for performance if you
have a really deep tree but it doesn't really hurt to be able to recover
since it's zero cost when it doesn't happen.

We could do the same thing for Flight. Those trees don't tend to be as
deep but could happen.
---
 .../src/__tests__/ReactDOMFizzServer-test.js  | 61 ++++++++++++
 packages/react-server/src/ReactFizzServer.js  | 92 +++++++++++++++----
 2 files changed, 137 insertions(+), 16 deletions(-)

diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js
index 5a763ffe949ab..a109011b35e98 100644
--- a/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js
+++ b/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js
@@ -8677,4 +8677,65 @@ describe('ReactDOMFizzServer', () => {
       '\n    in Bar (at **)' + '\n    in Foo (at **)',
     );
   });
+
+  it('can recover from very deep trees to avoid stack overflow', async () => {
+    function Recursive({n}) {
+      if (n > 0) {
+        return <Recursive n={n - 1} />;
+      }
+      return <span>hi</span>;
+    }
+
+    // Recursively render a component tree deep enough to trigger stack overflow.
+    // Don't make this too short to not hit the limit but also not too deep to slow
+    // down the test.
+    await act(() => {
+      const {pipe} = renderToPipeableStream(
+        <div>
+          <Recursive n={1000} />
+        </div>,
+      );
+      pipe(writable);
+    });
+
+    expect(getVisibleChildren(container)).toEqual(
+      <div>
+        <span>hi</span>
+      </div>,
+    );
+  });
+
+  it('handles stack overflows inside components themselves', async () => {
+    function StackOverflow() {
+      // This component is recursive inside itself and is therefore an error.
+      // Assuming no tail-call optimizations.
+      function recursive(n, a0, a1, a2, a3) {
+        if (n > 0) {
+          return recursive(n - 1, a0, a1, a2, a3) + a0 + a1 + a2 + a3;
+        }
+        return a0;
+      }
+      return recursive(10000, 'should', 'not', 'resolve', 'this');
+    }
+
+    let caughtError;
+
+    await expect(async () => {
+      await act(() => {
+        const {pipe} = renderToPipeableStream(
+          <div>
+            <StackOverflow />
+          </div>,
+          {
+            onError(error, errorInfo) {
+              caughtError = error;
+            },
+          },
+        );
+        pipe(writable);
+      });
+    }).rejects.toThrow('Maximum call stack size exceeded');
+
+    expect(caughtError.message).toBe('Maximum call stack size exceeded');
+  });
 });
diff --git a/packages/react-server/src/ReactFizzServer.js b/packages/react-server/src/ReactFizzServer.js
index 7ccbd65d16b74..aeffa6b813b8f 100644
--- a/packages/react-server/src/ReactFizzServer.js
+++ b/packages/react-server/src/ReactFizzServer.js
@@ -3320,9 +3320,8 @@ function spawnNewSuspendedReplayTask(
   request: Request,
   task: ReplayTask,
   thenableState: ThenableState | null,
-  x: Wakeable,
-): void {
-  const newTask = createReplayTask(
+): ReplayTask {
+  return createReplayTask(
     request,
     thenableState,
     task.replay,
@@ -3340,17 +3339,13 @@ function spawnNewSuspendedReplayTask(
     !disableLegacyContext ? task.legacyContext : emptyContextObject,
     __DEV__ && enableOwnerStacks ? task.debugTask : null,
   );
-
-  const ping = newTask.ping;
-  x.then(ping, ping);
 }
 
 function spawnNewSuspendedRenderTask(
   request: Request,
   task: RenderTask,
   thenableState: ThenableState | null,
-  x: Wakeable,
-): void {
+): RenderTask {
   // Something suspended, we'll need to create a new segment and resolve it later.
   const segment = task.blockedSegment;
   const insertionIndex = segment.chunks.length;
@@ -3367,7 +3362,7 @@ function spawnNewSuspendedRenderTask(
   segment.children.push(newSegment);
   // Reset lastPushedText for current Segment since the new Segment "consumed" it
   segment.lastPushedText = false;
-  const newTask = createRenderTask(
+  return createRenderTask(
     request,
     thenableState,
     task.node,
@@ -3385,9 +3380,6 @@ function spawnNewSuspendedRenderTask(
     !disableLegacyContext ? task.legacyContext : emptyContextObject,
     __DEV__ && enableOwnerStacks ? task.debugTask : null,
   );
-
-  const ping = newTask.ping;
-  x.then(ping, ping);
 }
 
 // This is a non-destructive form of rendering a node. If it suspends it spawns
@@ -3436,14 +3428,48 @@ function renderNode(
         if (typeof x.then === 'function') {
           const wakeable: Wakeable = (x: any);
           const thenableState = getThenableStateAfterSuspending();
-          spawnNewSuspendedReplayTask(
+          const newTask = spawnNewSuspendedReplayTask(
+            request,
+            // $FlowFixMe: Refined.
+            task,
+            thenableState,
+          );
+          const ping = newTask.ping;
+          wakeable.then(ping, ping);
+
+          // Restore the context. We assume that this will be restored by the inner
+          // functions in case nothing throws so we don't use "finally" here.
+          task.formatContext = previousFormatContext;
+          if (!disableLegacyContext) {
+            task.legacyContext = previousLegacyContext;
+          }
+          task.context = previousContext;
+          task.keyPath = previousKeyPath;
+          task.treeContext = previousTreeContext;
+          task.componentStack = previousComponentStack;
+          if (__DEV__ && enableOwnerStacks) {
+            task.debugTask = previousDebugTask;
+          }
+          // Restore all active ReactContexts to what they were before.
+          switchContext(previousContext);
+          return;
+        }
+        if (x.message === 'Maximum call stack size exceeded') {
+          // This was a stack overflow. We do a lot of recursion in React by default for
+          // performance but it can lead to stack overflows in extremely deep trees.
+          // We do have the ability to create a trampoile if this happens which makes
+          // this kind of zero-cost.
+          const thenableState = getThenableStateAfterSuspending();
+          const newTask = spawnNewSuspendedReplayTask(
             request,
             // $FlowFixMe: Refined.
             task,
             thenableState,
-            wakeable,
           );
 
+          // Immediately schedule the task for retrying.
+          request.pingedTasks.push(newTask);
+
           // Restore the context. We assume that this will be restored by the inner
           // functions in case nothing throws so we don't use "finally" here.
           task.formatContext = previousFormatContext;
@@ -3493,13 +3519,14 @@ function renderNode(
         if (typeof x.then === 'function') {
           const wakeable: Wakeable = (x: any);
           const thenableState = getThenableStateAfterSuspending();
-          spawnNewSuspendedRenderTask(
+          const newTask = spawnNewSuspendedRenderTask(
             request,
             // $FlowFixMe: Refined.
             task,
             thenableState,
-            wakeable,
           );
+          const ping = newTask.ping;
+          wakeable.then(ping, ping);
 
           // Restore the context. We assume that this will be restored by the inner
           // functions in case nothing throws so we don't use "finally" here.
@@ -3540,6 +3567,39 @@ function renderNode(
           );
           trackPostpone(request, trackedPostpones, task, postponedSegment);
 
+          // Restore the context. We assume that this will be restored by the inner
+          // functions in case nothing throws so we don't use "finally" here.
+          task.formatContext = previousFormatContext;
+          if (!disableLegacyContext) {
+            task.legacyContext = previousLegacyContext;
+          }
+          task.context = previousContext;
+          task.keyPath = previousKeyPath;
+          task.treeContext = previousTreeContext;
+          task.componentStack = previousComponentStack;
+          if (__DEV__ && enableOwnerStacks) {
+            task.debugTask = previousDebugTask;
+          }
+          // Restore all active ReactContexts to what they were before.
+          switchContext(previousContext);
+          return;
+        }
+        if (x.message === 'Maximum call stack size exceeded') {
+          // This was a stack overflow. We do a lot of recursion in React by default for
+          // performance but it can lead to stack overflows in extremely deep trees.
+          // We do have the ability to create a trampoile if this happens which makes
+          // this kind of zero-cost.
+          const thenableState = getThenableStateAfterSuspending();
+          const newTask = spawnNewSuspendedRenderTask(
+            request,
+            // $FlowFixMe: Refined.
+            task,
+            thenableState,
+          );
+
+          // Immediately schedule the task for retrying.
+          request.pingedTasks.push(newTask);
+
           // Restore the context. We assume that this will be restored by the inner
           // functions in case nothing throws so we don't use "finally" here.
           task.formatContext = previousFormatContext;

From f2841c2a490b4b776b98568871b69693fedf985c Mon Sep 17 00:00:00 2001
From: Mike Vitousek <mmvitousek@gmail.com>
Date: Tue, 27 Aug 2024 10:11:50 -0700
Subject: [PATCH 09/10] [compiler] Fixture to demonstrate issue with returning
 object containing ref

Summary:
We currently can return a ref from a hook but not an object containing a ref.

ghstack-source-id: 8b1de4991eb2731b7f758e685ba62d9f07d584b2
Pull Request resolved: https://github.com/facebook/react/pull/30820
---
 ...or.return-ref-callback-structure.expect.md | 45 +++++++++++++++++++
 .../error.return-ref-callback-structure.js    | 20 +++++++++
 2 files changed, 65 insertions(+)
 create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.return-ref-callback-structure.expect.md
 create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.return-ref-callback-structure.js

diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.return-ref-callback-structure.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.return-ref-callback-structure.expect.md
new file mode 100644
index 0000000000000..866d2e2fea657
--- /dev/null
+++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.return-ref-callback-structure.expect.md
@@ -0,0 +1,45 @@
+
+## Input
+
+```javascript
+// @flow @validateRefAccessDuringRender @validatePreserveExistingMemoizationGuarantees
+
+import {useRef} from 'react';
+
+component Foo(cond: boolean, cond2: boolean) {
+  const ref = useRef();
+
+  const s = () => {
+    return ref.current;
+  };
+
+  if (cond) return [s];
+  else if (cond2) return {s};
+  else return {s: [s]};
+}
+
+export const FIXTURE_ENTRYPOINT = {
+  fn: Foo,
+  params: [{cond: false, cond2: false}],
+};
+
+```
+
+
+## Error
+
+```
+  10 |   };
+  11 |
+> 12 |   if (cond) return [s];
+     |                     ^ InvalidReact: Ref values (the `current` property) may not be accessed during render. (https://react.dev/reference/react/useRef) (12:12)
+
+InvalidReact: Ref values (the `current` property) may not be accessed during render. (https://react.dev/reference/react/useRef) (13:13)
+
+InvalidReact: Ref values (the `current` property) may not be accessed during render. (https://react.dev/reference/react/useRef) (14:14)
+  13 |   else if (cond2) return {s};
+  14 |   else return {s: [s]};
+  15 | }
+```
+          
+      
\ No newline at end of file
diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.return-ref-callback-structure.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.return-ref-callback-structure.js
new file mode 100644
index 0000000000000..e37acbde348d1
--- /dev/null
+++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.return-ref-callback-structure.js
@@ -0,0 +1,20 @@
+// @flow @validateRefAccessDuringRender @validatePreserveExistingMemoizationGuarantees
+
+import {useRef} from 'react';
+
+component Foo(cond: boolean, cond2: boolean) {
+  const ref = useRef();
+
+  const s = () => {
+    return ref.current;
+  };
+
+  if (cond) return [s];
+  else if (cond2) return {s};
+  else return {s: [s]};
+}
+
+export const FIXTURE_ENTRYPOINT = {
+  fn: Foo,
+  params: [{cond: false, cond2: false}],
+};

From 7771d3a7972cc2483c45fde51b7ec2d926cba097 Mon Sep 17 00:00:00 2001
From: Mike Vitousek <mmvitousek@gmail.com>
Date: Tue, 27 Aug 2024 10:11:50 -0700
Subject: [PATCH 10/10] [compiler] Track refs through object expressions and
 property lookups

Summary:
This addresses the issue of the compiler being overly restrictive about refs escaping into object expressions. Rather than erroring whenever a ref flows into an object, we will now treat the object itself as a ref, and apply the same escape rules to it. Whenever we look up a property from a ref value, we now don't know whether that value is itself a ref or a ref value, so we assume it's both.

The same logic applies to ref-accessing functions--if such a function is stored in an object, we'll propagate that property to the object itself and any properties looked up from it.

ghstack-source-id: 5c6fcb895d4a1658ce9dddec286aad3a57a4c9f1
Pull Request resolved: https://github.com/facebook/react/pull/30821
---
 .../Validation/ValidateNoRefAccesInRender.ts  | 253 ++++++++++++------
 ...f-added-to-dep-without-type-info.expect.md |  16 +-
 ...or.return-ref-callback-structure.expect.md |  45 ----
 .../return-ref-callback-structure.expect.md   |  87 ++++++
 ...re.js => return-ref-callback-structure.js} |   0
 5 files changed, 273 insertions(+), 128 deletions(-)
 delete mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.return-ref-callback-structure.expect.md
 create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/return-ref-callback-structure.expect.md
 rename compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/{error.return-ref-callback-structure.js => return-ref-callback-structure.js} (100%)

diff --git a/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateNoRefAccesInRender.ts b/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateNoRefAccesInRender.ts
index df6241a73f448..8a65b4709c174 100644
--- a/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateNoRefAccesInRender.ts
+++ b/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidateNoRefAccesInRender.ts
@@ -11,12 +11,12 @@ import {
   IdentifierId,
   Place,
   SourceLocation,
-  isRefOrRefValue,
   isRefValueType,
   isUseRefType,
 } from '../HIR';
 import {
   eachInstructionValueOperand,
+  eachPatternOperand,
   eachTerminalOperand,
 } from '../HIR/visitors';
 import {Err, Ok, Result} from '../Utils/Result';
@@ -42,58 +42,165 @@ import {isEffectHook} from './ValidateMemoizedEffectDependencies';
  * In the future we may reject more cases, based on either object names (`fooRef.current` is likely a ref)
  * or based on property name alone (`foo.current` might be a ref).
  */
+type State = {
+  refs: Set<IdentifierId>;
+  refValues: Map<IdentifierId, SourceLocation | null>;
+  refAccessingFunctions: Set<IdentifierId>;
+};
+
 export function validateNoRefAccessInRender(fn: HIRFunction): void {
-  const refAccessingFunctions: Set<IdentifierId> = new Set();
-  validateNoRefAccessInRenderImpl(fn, refAccessingFunctions).unwrap();
+  const state = {
+    refs: new Set<IdentifierId>(),
+    refValues: new Map<IdentifierId, SourceLocation | null>(),
+    refAccessingFunctions: new Set<IdentifierId>(),
+  };
+  validateNoRefAccessInRenderImpl(fn, state).unwrap();
 }
 
 function validateNoRefAccessInRenderImpl(
   fn: HIRFunction,
-  refAccessingFunctions: Set<IdentifierId>,
+  state: State,
 ): Result<void, CompilerError> {
+  let place;
+  for (const param of fn.params) {
+    if (param.kind === 'Identifier') {
+      place = param;
+    } else {
+      place = param.place;
+    }
+
+    if (isRefValueType(place.identifier)) {
+      state.refValues.set(place.identifier.id, null);
+    }
+    if (isUseRefType(place.identifier)) {
+      state.refs.add(place.identifier.id);
+    }
+  }
   const errors = new CompilerError();
-  const lookupLocations: Map<IdentifierId, SourceLocation> = new Map();
   for (const [, block] of fn.body.blocks) {
+    for (const phi of block.phis) {
+      phi.operands.forEach(operand => {
+        if (state.refs.has(operand.id) || isUseRefType(phi.id)) {
+          state.refs.add(phi.id.id);
+        }
+        const refValue = state.refValues.get(operand.id);
+        if (refValue !== undefined || isRefValueType(operand)) {
+          state.refValues.set(
+            phi.id.id,
+            refValue ?? state.refValues.get(phi.id.id) ?? null,
+          );
+        }
+        if (state.refAccessingFunctions.has(operand.id)) {
+          state.refAccessingFunctions.add(phi.id.id);
+        }
+      });
+    }
+
     for (const instr of block.instructions) {
+      for (const operand of eachInstructionValueOperand(instr.value)) {
+        if (isRefValueType(operand.identifier)) {
+          CompilerError.invariant(state.refValues.has(operand.identifier.id), {
+            reason: 'Expected ref value to be in state',
+            loc: operand.loc,
+          });
+        }
+        if (isUseRefType(operand.identifier)) {
+          CompilerError.invariant(state.refs.has(operand.identifier.id), {
+            reason: 'Expected ref to be in state',
+            loc: operand.loc,
+          });
+        }
+      }
+
       switch (instr.value.kind) {
         case 'JsxExpression':
         case 'JsxFragment': {
           for (const operand of eachInstructionValueOperand(instr.value)) {
-            validateNoDirectRefValueAccess(errors, operand, lookupLocations);
+            validateNoDirectRefValueAccess(errors, operand, state);
           }
           break;
         }
+        case 'ComputedLoad':
         case 'PropertyLoad': {
+          if (typeof instr.value.property !== 'string') {
+            validateNoRefValueAccess(errors, state, instr.value.property);
+          }
           if (
-            isRefValueType(instr.lvalue.identifier) &&
-            instr.value.property === 'current'
+            state.refAccessingFunctions.has(instr.value.object.identifier.id)
           ) {
-            lookupLocations.set(instr.lvalue.identifier.id, instr.loc);
+            state.refAccessingFunctions.add(instr.lvalue.identifier.id);
+          }
+          if (state.refs.has(instr.value.object.identifier.id)) {
+            /*
+             * Once an object contains a ref at any level, we treat it as a ref.
+             * If we look something up from it, that value may either be a ref
+             * or the ref value (or neither), so we conservatively assume it's both.
+             */
+            state.refs.add(instr.lvalue.identifier.id);
+            state.refValues.set(instr.lvalue.identifier.id, instr.loc);
           }
           break;
         }
+        case 'LoadContext':
         case 'LoadLocal': {
-          if (refAccessingFunctions.has(instr.value.place.identifier.id)) {
-            refAccessingFunctions.add(instr.lvalue.identifier.id);
+          if (
+            state.refAccessingFunctions.has(instr.value.place.identifier.id)
+          ) {
+            state.refAccessingFunctions.add(instr.lvalue.identifier.id);
           }
-          if (isRefValueType(instr.lvalue.identifier)) {
-            const loc = lookupLocations.get(instr.value.place.identifier.id);
-            if (loc !== undefined) {
-              lookupLocations.set(instr.lvalue.identifier.id, loc);
-            }
+          const refValue = state.refValues.get(instr.value.place.identifier.id);
+          if (refValue !== undefined) {
+            state.refValues.set(instr.lvalue.identifier.id, refValue);
+          }
+          if (state.refs.has(instr.value.place.identifier.id)) {
+            state.refs.add(instr.lvalue.identifier.id);
           }
           break;
         }
+        case 'StoreContext':
         case 'StoreLocal': {
-          if (refAccessingFunctions.has(instr.value.value.identifier.id)) {
-            refAccessingFunctions.add(instr.value.lvalue.place.identifier.id);
-            refAccessingFunctions.add(instr.lvalue.identifier.id);
+          if (
+            state.refAccessingFunctions.has(instr.value.value.identifier.id)
+          ) {
+            state.refAccessingFunctions.add(
+              instr.value.lvalue.place.identifier.id,
+            );
+            state.refAccessingFunctions.add(instr.lvalue.identifier.id);
+          }
+          const refValue = state.refValues.get(instr.value.value.identifier.id);
+          if (
+            refValue !== undefined ||
+            isRefValueType(instr.value.lvalue.place.identifier)
+          ) {
+            state.refValues.set(
+              instr.value.lvalue.place.identifier.id,
+              refValue ?? null,
+            );
+            state.refValues.set(instr.lvalue.identifier.id, refValue ?? null);
+          }
+          if (state.refs.has(instr.value.value.identifier.id)) {
+            state.refs.add(instr.value.lvalue.place.identifier.id);
+            state.refs.add(instr.lvalue.identifier.id);
           }
-          if (isRefValueType(instr.value.lvalue.place.identifier)) {
-            const loc = lookupLocations.get(instr.value.value.identifier.id);
-            if (loc !== undefined) {
-              lookupLocations.set(instr.value.lvalue.place.identifier.id, loc);
-              lookupLocations.set(instr.lvalue.identifier.id, loc);
+          break;
+        }
+        case 'Destructure': {
+          const destructuredFunction = state.refAccessingFunctions.has(
+            instr.value.value.identifier.id,
+          );
+          const destructuredRef = state.refs.has(
+            instr.value.value.identifier.id,
+          );
+          for (const lval of eachPatternOperand(instr.value.lvalue.pattern)) {
+            if (isUseRefType(lval.identifier)) {
+              state.refs.add(lval.identifier.id);
+            }
+            if (destructuredRef || isRefValueType(lval.identifier)) {
+              state.refs.add(lval.identifier.id);
+              state.refValues.set(lval.identifier.id, null);
+            }
+            if (destructuredFunction) {
+              state.refAccessingFunctions.add(lval.identifier.id);
             }
           }
           break;
@@ -107,32 +214,27 @@ function validateNoRefAccessInRenderImpl(
              */
             [...eachInstructionValueOperand(instr.value)].some(
               operand =>
-                isRefValueType(operand.identifier) ||
-                refAccessingFunctions.has(operand.identifier.id),
+                state.refValues.has(operand.identifier.id) ||
+                state.refAccessingFunctions.has(operand.identifier.id),
             ) ||
             // check for cases where .current is accessed through an aliased ref
             ([...eachInstructionValueOperand(instr.value)].some(operand =>
-              isUseRefType(operand.identifier),
+              state.refs.has(operand.identifier.id),
             ) &&
               validateNoRefAccessInRenderImpl(
                 instr.value.loweredFunc.func,
-                refAccessingFunctions,
+                state,
               ).isErr())
           ) {
             // This function expression unconditionally accesses a ref
-            refAccessingFunctions.add(instr.lvalue.identifier.id);
+            state.refAccessingFunctions.add(instr.lvalue.identifier.id);
           }
           break;
         }
         case 'MethodCall': {
           if (!isEffectHook(instr.value.property.identifier)) {
             for (const operand of eachInstructionValueOperand(instr.value)) {
-              validateNoRefAccess(
-                errors,
-                refAccessingFunctions,
-                operand,
-                operand.loc,
-              );
+              validateNoRefAccess(errors, state, operand, operand.loc);
             }
           }
           break;
@@ -142,7 +244,7 @@ function validateNoRefAccessInRenderImpl(
           const isUseEffect = isEffectHook(callee.identifier);
           if (!isUseEffect) {
             // Report a more precise error when calling a local function that accesses a ref
-            if (refAccessingFunctions.has(callee.identifier.id)) {
+            if (state.refAccessingFunctions.has(callee.identifier.id)) {
               errors.push({
                 severity: ErrorSeverity.InvalidReact,
                 reason:
@@ -159,9 +261,9 @@ function validateNoRefAccessInRenderImpl(
             for (const operand of eachInstructionValueOperand(instr.value)) {
               validateNoRefAccess(
                 errors,
-                refAccessingFunctions,
+                state,
                 operand,
-                lookupLocations.get(operand.identifier.id) ?? operand.loc,
+                state.refValues.get(operand.identifier.id) ?? operand.loc,
               );
             }
           }
@@ -170,12 +272,17 @@ function validateNoRefAccessInRenderImpl(
         case 'ObjectExpression':
         case 'ArrayExpression': {
           for (const operand of eachInstructionValueOperand(instr.value)) {
-            validateNoRefAccess(
-              errors,
-              refAccessingFunctions,
-              operand,
-              lookupLocations.get(operand.identifier.id) ?? operand.loc,
-            );
+            validateNoDirectRefValueAccess(errors, operand, state);
+            if (state.refAccessingFunctions.has(operand.identifier.id)) {
+              state.refAccessingFunctions.add(instr.lvalue.identifier.id);
+            }
+            if (state.refs.has(operand.identifier.id)) {
+              state.refs.add(instr.lvalue.identifier.id);
+            }
+            const refValue = state.refValues.get(operand.identifier.id);
+            if (refValue !== undefined) {
+              state.refValues.set(instr.lvalue.identifier.id, refValue);
+            }
           }
           break;
         }
@@ -185,20 +292,15 @@ function validateNoRefAccessInRenderImpl(
         case 'ComputedStore': {
           validateNoRefAccess(
             errors,
-            refAccessingFunctions,
+            state,
             instr.value.object,
-            lookupLocations.get(instr.value.object.identifier.id) ?? instr.loc,
+            state.refValues.get(instr.value.object.identifier.id) ?? instr.loc,
           );
           for (const operand of eachInstructionValueOperand(instr.value)) {
             if (operand === instr.value.object) {
               continue;
             }
-            validateNoRefValueAccess(
-              errors,
-              refAccessingFunctions,
-              lookupLocations,
-              operand,
-            );
+            validateNoRefValueAccess(errors, state, operand);
           }
           break;
         }
@@ -207,28 +309,27 @@ function validateNoRefAccessInRenderImpl(
           break;
         default: {
           for (const operand of eachInstructionValueOperand(instr.value)) {
-            validateNoRefValueAccess(
-              errors,
-              refAccessingFunctions,
-              lookupLocations,
-              operand,
-            );
+            validateNoRefValueAccess(errors, state, operand);
           }
           break;
         }
       }
+      if (isUseRefType(instr.lvalue.identifier)) {
+        state.refs.add(instr.lvalue.identifier.id);
+      }
+      if (
+        isRefValueType(instr.lvalue.identifier) &&
+        !state.refValues.has(instr.lvalue.identifier.id)
+      ) {
+        state.refValues.set(instr.lvalue.identifier.id, instr.loc);
+      }
     }
     for (const operand of eachTerminalOperand(block.terminal)) {
       if (block.terminal.kind !== 'return') {
-        validateNoRefValueAccess(
-          errors,
-          refAccessingFunctions,
-          lookupLocations,
-          operand,
-        );
+        validateNoRefValueAccess(errors, state, operand);
       } else {
         // Allow functions containing refs to be returned, but not direct ref values
-        validateNoDirectRefValueAccess(errors, operand, lookupLocations);
+        validateNoDirectRefValueAccess(errors, operand, state);
       }
     }
   }
@@ -242,19 +343,18 @@ function validateNoRefAccessInRenderImpl(
 
 function validateNoRefValueAccess(
   errors: CompilerError,
-  refAccessingFunctions: Set<IdentifierId>,
-  lookupLocations: Map<IdentifierId, SourceLocation>,
+  state: State,
   operand: Place,
 ): void {
   if (
-    isRefValueType(operand.identifier) ||
-    refAccessingFunctions.has(operand.identifier.id)
+    state.refValues.has(operand.identifier.id) ||
+    state.refAccessingFunctions.has(operand.identifier.id)
   ) {
     errors.push({
       severity: ErrorSeverity.InvalidReact,
       reason:
         'Ref values (the `current` property) may not be accessed during render. (https://react.dev/reference/react/useRef)',
-      loc: lookupLocations.get(operand.identifier.id) ?? operand.loc,
+      loc: state.refValues.get(operand.identifier.id) ?? operand.loc,
       description:
         operand.identifier.name !== null &&
         operand.identifier.name.kind === 'named'
@@ -267,13 +367,14 @@ function validateNoRefValueAccess(
 
 function validateNoRefAccess(
   errors: CompilerError,
-  refAccessingFunctions: Set<IdentifierId>,
+  state: State,
   operand: Place,
   loc: SourceLocation,
 ): void {
   if (
-    isRefOrRefValue(operand.identifier) ||
-    refAccessingFunctions.has(operand.identifier.id)
+    state.refs.has(operand.identifier.id) ||
+    state.refValues.has(operand.identifier.id) ||
+    state.refAccessingFunctions.has(operand.identifier.id)
   ) {
     errors.push({
       severity: ErrorSeverity.InvalidReact,
@@ -293,14 +394,14 @@ function validateNoRefAccess(
 function validateNoDirectRefValueAccess(
   errors: CompilerError,
   operand: Place,
-  lookupLocations: Map<IdentifierId, SourceLocation>,
+  state: State,
 ): void {
-  if (isRefValueType(operand.identifier)) {
+  if (state.refValues.has(operand.identifier.id)) {
     errors.push({
       severity: ErrorSeverity.InvalidReact,
       reason:
         'Ref values (the `current` property) may not be accessed during render. (https://react.dev/reference/react/useRef)',
-      loc: lookupLocations.get(operand.identifier.id) ?? operand.loc,
+      loc: state.refValues.get(operand.identifier.id) ?? operand.loc,
       description:
         operand.identifier.name !== null &&
         operand.identifier.name.kind === 'named'
diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-use-ref-added-to-dep-without-type-info.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-use-ref-added-to-dep-without-type-info.expect.md
index a28a74730bfb8..f576bac764613 100644
--- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-use-ref-added-to-dep-without-type-info.expect.md
+++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-use-ref-added-to-dep-without-type-info.expect.md
@@ -22,13 +22,15 @@ function Foo({a}) {
 ## Error
 
 ```
-  3 |   const ref = useRef();
-  4 |   // type information is lost here as we don't track types of fields
-> 5 |   const val = {ref};
-    |                ^^^ InvalidReact: Ref values (the `current` property) may not be accessed during render. (https://react.dev/reference/react/useRef) (5:5)
-  6 |   // without type info, we don't know that val.ref.current is a ref value so we
-  7 |   // *would* end up depending on val.ref.current
-  8 |   // however, this is an instance of accessing a ref during render and is disallowed
+   8 |   // however, this is an instance of accessing a ref during render and is disallowed
+   9 |   // under React's rules, so we reject this input
+> 10 |   const x = {a, val: val.ref.current};
+     |                      ^^^^^^^^^^^^^^^ InvalidReact: Ref values (the `current` property) may not be accessed during render. (https://react.dev/reference/react/useRef) (10:10)
+
+InvalidReact: Ref values (the `current` property) may not be accessed during render. (https://react.dev/reference/react/useRef) (10:10)
+  11 |
+  12 |   return <VideoList videos={x} />;
+  13 | }
 ```
           
       
\ No newline at end of file
diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.return-ref-callback-structure.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.return-ref-callback-structure.expect.md
deleted file mode 100644
index 866d2e2fea657..0000000000000
--- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.return-ref-callback-structure.expect.md
+++ /dev/null
@@ -1,45 +0,0 @@
-
-## Input
-
-```javascript
-// @flow @validateRefAccessDuringRender @validatePreserveExistingMemoizationGuarantees
-
-import {useRef} from 'react';
-
-component Foo(cond: boolean, cond2: boolean) {
-  const ref = useRef();
-
-  const s = () => {
-    return ref.current;
-  };
-
-  if (cond) return [s];
-  else if (cond2) return {s};
-  else return {s: [s]};
-}
-
-export const FIXTURE_ENTRYPOINT = {
-  fn: Foo,
-  params: [{cond: false, cond2: false}],
-};
-
-```
-
-
-## Error
-
-```
-  10 |   };
-  11 |
-> 12 |   if (cond) return [s];
-     |                     ^ InvalidReact: Ref values (the `current` property) may not be accessed during render. (https://react.dev/reference/react/useRef) (12:12)
-
-InvalidReact: Ref values (the `current` property) may not be accessed during render. (https://react.dev/reference/react/useRef) (13:13)
-
-InvalidReact: Ref values (the `current` property) may not be accessed during render. (https://react.dev/reference/react/useRef) (14:14)
-  13 |   else if (cond2) return {s};
-  14 |   else return {s: [s]};
-  15 | }
-```
-          
-      
\ No newline at end of file
diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/return-ref-callback-structure.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/return-ref-callback-structure.expect.md
new file mode 100644
index 0000000000000..95976383cbff8
--- /dev/null
+++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/return-ref-callback-structure.expect.md
@@ -0,0 +1,87 @@
+
+## Input
+
+```javascript
+// @flow @validateRefAccessDuringRender @validatePreserveExistingMemoizationGuarantees
+
+import {useRef} from 'react';
+
+component Foo(cond: boolean, cond2: boolean) {
+  const ref = useRef();
+
+  const s = () => {
+    return ref.current;
+  };
+
+  if (cond) return [s];
+  else if (cond2) return {s};
+  else return {s: [s]};
+}
+
+export const FIXTURE_ENTRYPOINT = {
+  fn: Foo,
+  params: [{cond: false, cond2: false}],
+};
+
+```
+
+## Code
+
+```javascript
+import { c as _c } from "react/compiler-runtime";
+
+import { useRef } from "react";
+
+function Foo(t0) {
+  const $ = _c(4);
+  const { cond, cond2 } = t0;
+  const ref = useRef();
+  let t1;
+  if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
+    t1 = () => ref.current;
+    $[0] = t1;
+  } else {
+    t1 = $[0];
+  }
+  const s = t1;
+  if (cond) {
+    let t2;
+    if ($[1] === Symbol.for("react.memo_cache_sentinel")) {
+      t2 = [s];
+      $[1] = t2;
+    } else {
+      t2 = $[1];
+    }
+    return t2;
+  } else {
+    if (cond2) {
+      let t2;
+      if ($[2] === Symbol.for("react.memo_cache_sentinel")) {
+        t2 = { s };
+        $[2] = t2;
+      } else {
+        t2 = $[2];
+      }
+      return t2;
+    } else {
+      let t2;
+      if ($[3] === Symbol.for("react.memo_cache_sentinel")) {
+        t2 = { s: [s] };
+        $[3] = t2;
+      } else {
+        t2 = $[3];
+      }
+      return t2;
+    }
+  }
+}
+
+export const FIXTURE_ENTRYPOINT = {
+  fn: Foo,
+  params: [{ cond: false, cond2: false }],
+};
+
+```
+      
+### Eval output
+(kind: ok) {"s":["[[ function params=0 ]]"]}
\ No newline at end of file
diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.return-ref-callback-structure.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/return-ref-callback-structure.js
similarity index 100%
rename from compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.return-ref-callback-structure.js
rename to compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/return-ref-callback-structure.js