Repository: facebook/react
Pull request: [DevTools] Get source location from structured callsites in prepareStackTrace

When we get the source location for "View source for this element" we should be using the enclosing function of the callsite of the child. So that we don't just point to some random line within the component.

This is similar to the technique in #33136.

This technique is now really better than the fake throw technique, when available. So I now favor the owner technique. The only problem it's only available in DEV and only if it has a child that's owned (and not filtered).

We could implement this same technique for the error that's thrown in the fake throwing solution. However, we really shouldn't need that at all because for client components we should be able to call `inspect(fn)` at least in Chrome which is even better.

Changed files:
filename: packages/react-devtools-shared/src/backend/fiber/renderer.js
patch: @@ -50,6 +50,7 @@ import {
   gt,
   gte,
   parseSourceFromComponentStack,
+  parseSourceFromOwnerStack,
   serializeToString,
 } from 'react-devtools-shared/src/backend/utils';
 import {
@@ -5805,15 +5806,13 @@ export function attach(
   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;
+    // Favor the owner source if we have one.
+    const ownerSource = getSourceForInstance(fiberInstance);
+    if (ownerSource !== null) {
+      return ownerSource;
     }
+
+    // Otherwise fallback to the throwing trick.
     const dispatcherRef = getDispatcherRef(renderer);
     const stackFrame =
       dispatcherRef == null
@@ -5824,18 +5823,15 @@ export function attach(
             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);
+      return null;
     }
     const source = parseSourceFromComponentStack(stackFrame);
     fiberInstance.source = source;
     return source;
   }
 
   function getSourceForInstance(instance: DevToolsInstance): Source | null {
-    let unresolvedSource = instance.source;
+    const 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.
@@ -5848,7 +5844,9 @@ export function attach(
     // 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));
+      return (instance.source = parseSourceFromOwnerStack(
+        (unresolvedSource: any),
+      ));
     }
     if (typeof unresolvedSource === 'string') {
       const idx = unresolvedSource.lastIndexOf('\n');

filename: packages/react-devtools-shared/src/backend/shared/DevToolsOwnerStack.js
patch: @@ -13,8 +13,12 @@ export function formatOwnerStack(error: Error): string {
   const prevPrepareStackTrace = Error.prepareStackTrace;
   // $FlowFixMe[incompatible-type] It does accept undefined.
   Error.prepareStackTrace = undefined;
-  let stack = error.stack;
+  const stack = error.stack;
   Error.prepareStackTrace = prevPrepareStackTrace;
+  return formatOwnerStackString(stack);
+}
+
+export function formatOwnerStackString(stack: string): string {
   if (stack.startsWith('Error: react-stack-top-frame\n')) {
     // V8's default formatting prefixes with the error message which we
     // don't want/need.

filename: packages/react-devtools-shared/src/backend/utils/index.js
patch: @@ -18,6 +18,8 @@ import type {DehydratedData} from 'react-devtools-shared/src/frontend/types';
 export {default as formatWithStyles} from './formatWithStyles';
 export {default as formatConsoleArguments} from './formatConsoleArguments';
 
+import {formatOwnerStackString} from '../shared/DevToolsOwnerStack';
+
 // TODO: update this to the first React version that has a corresponding DevTools backend
 const FIRST_DEVTOOLS_BACKEND_LOCKSTEP_VER = '999.9.9';
 export function hasAssignedBackend(version?: string): boolean {
@@ -345,6 +347,77 @@ export function parseSourceFromComponentStack(
   return parseSourceFromFirefoxStack(componentStack);
 }
 
+let collectedLocation: Source | null = null;
+
+function collectStackTrace(
+  error: Error,
+  structuredStackTrace: CallSite[],
+): string {
+  let result: null | Source = null;
+  // Collect structured stack traces from the callsites.
+  // We mirror how V8 serializes stack frames and how we later parse them.
+  for (let i = 0; i < structuredStackTrace.length; i++) {
+    const callSite = structuredStackTrace[i];
+    if (callSite.getFunctionName() === 'react-stack-bottom-frame') {
+      // We pick the last frame that matches before the bottom frame since
+      // that will be immediately inside the component as opposed to some helper.
+      // If we don't find a bottom frame then we bail to string parsing.
+      collectedLocation = result;
+      // Skip everything after the bottom frame since it'll be internals.
+      break;
+    } else {
+      const sourceURL = callSite.getScriptNameOrSourceURL();
+      const line =
+        // $FlowFixMe[prop-missing]
+        typeof callSite.getEnclosingLineNumber === 'function'
+          ? (callSite: any).getEnclosingLineNumber()
+          : callSite.getLineNumber();
+      const col =
+        // $FlowFixMe[prop-missing]
+        typeof callSite.getEnclosingColumnNumber === 'function'
+          ? (callSite: any).getEnclosingColumnNumber()
+          : callSite.getLineNumber();
+      if (!sourceURL || !line || !col) {
+        // Skip eval etc. without source url. They don't have location.
+        continue;
+      }
+      result = {
+        sourceURL,
+        line: line,
+        column: col,
+      };
+    }
+  }
+  // At the same time we generate a string stack trace just in case someone
+  // else reads it.
+  const name = error.name || 'Error';
+  const message = error.message || '';
+  let stack = name + ': ' + message;
+  for (let i = 0; i < structuredStackTrace.length; i++) {
+    stack += '\n    at ' + structuredStackTrace[i].toString();
+  }
+  return stack;
+}
+
+export function parseSourceFromOwnerStack(error: Error): Source | null {
+  // First attempt to collected the structured data using prepareStackTrace.
+  collectedLocation = null;
+  const previousPrepare = Error.prepareStackTrace;
+  Error.prepareStackTrace = collectStackTrace;
+  let stack;
+  try {
+    stack = error.stack;
+  } finally {
+    Error.prepareStackTrace = previousPrepare;
+  }
+  if (collectedLocation !== null) {
+    return collectedLocation;
+  }
+  // Fallback to parsing the string form.
+  const componentStack = formatOwnerStackString(stack);
+  return parseSourceFromComponentStack(componentStack);
+}
+
 // 0.123456789 => 0.123
 // Expects high-resolution timestamp in milliseconds, like from performance.now()
 // Mainly used for optimizing the size of serialized profiling payload