Remove top stack frame from getCurrentStack (#30306)

sebmarkbage · web-flow · commit 433068eece20 · 2024-07-11T18:34:41.000-04:00
The full stack is the current execution stack (`new Error().stack`) +
the current owner stack (`React.captureOwnerStack()`).

The idea with the top frame was that when we append it to console.error
we'd include both since otherwise the true reason would be obscured
behind the little `&gt;` to expand. So we'd just put both stack front and
center. By adding this into getCurrentStack it was easy to use the same
filtering. I never implemented in Fizz or Flight though.

However, with the public API `React.captureOwnerStack()` it's not
necessary to include the current stack since you already have it and
you'd have filtering capabilities in user space too.

Since I'm removing the component stacks from React itself we no longer
need this. It's expected that maybe RDT or framework polyfill would
include this same technique though.
diff --git a/packages/react-reconciler/src/ReactCurrentFiber.js b/packages/react-reconciler/src/ReactCurrentFiber.js
@@ -33,7 +33,7 @@ export function getCurrentFiberOwnerNameInDevOrNull(): string | null {
   return null;
 }
 
-function getCurrentFiberStackInDev(stack: null | Error): string {
+function getCurrentFiberStackInDev(): string {
   if (__DEV__) {
     if (current === null) {
       return '';
@@ -43,7 +43,7 @@ function getCurrentFiberStackInDev(stack: null | Error): string {
     // TODO: The above comment is not actually true. We might be
     // in a commit phase or preemptive set state callback.
     if (enableOwnerStacks) {
-      return getOwnerStackByFiberInDev(current, stack);
+      return getOwnerStackByFiberInDev(current);
     }
     return getStackByFiberInDevAndProd(current);
   }
diff --git a/packages/react-reconciler/src/ReactFiberComponentStack.js b/packages/react-reconciler/src/ReactFiberComponentStack.js
@@ -91,27 +91,13 @@ function describeFunctionComponentFrameWithoutLineNumber(fn: Function): string {
   return name ? describeBuiltInComponentFrame(name) : '';
 }
 
-export function getOwnerStackByFiberInDev(
-  workInProgress: Fiber,
-  topStack: null | Error,
-): string {
+export function getOwnerStackByFiberInDev(workInProgress: Fiber): string {
   if (!enableOwnerStacks || !__DEV__) {
     return '';
   }
   try {
     let info = '';
 
-    if (topStack) {
-      // Prefix with a filtered version of the currently executing
-      // stack. This information will be available in the native
-      // stack regardless but it's hidden since we're reprinting
-      // the stack on top of it.
-      const formattedTopStack = formatOwnerStack(topStack);
-      if (formattedTopStack !== '') {
-        info += '\n' + formattedTopStack;
-      }
-    }
-
     if (workInProgress.tag === HostText) {
       // Text nodes never have an owner/stack because they're not created through JSX.
       // We use the parent since text nodes are always created through a host parent.
diff --git a/packages/react-reconciler/src/__tests__/ReactLazy-test.internal.js b/packages/react-reconciler/src/__tests__/ReactLazy-test.internal.js
@@ -213,14 +213,18 @@ describe('ReactLazy', () => {
       unstable_isConcurrent: true,
     });
 
+    function App() {
+      return (
+        <Suspense fallback={<Text text="Loading..." />}>
+          <LazyText text="Hi" />
+        </Suspense>
+      );
+    }
+
     let error;
     try {
       await act(() => {
-        root.update(
-          <Suspense fallback={<Text text="Loading..." />}>
-            <LazyText text="Hi" />
-          </Suspense>,
-        );
+        root.update(<App />);
       });
     } catch (e) {
       error = e;
diff --git a/packages/react/src/ReactOwnerStack.js b/packages/react/src/ReactOwnerStack.js
@@ -20,5 +20,5 @@ export function captureOwnerStack(): null | string {
   }
   // The current stack will be the owner stack if enableOwnerStacks is true
   // which it is always here. Otherwise it's the parent stack.
-  return getCurrentStack(null);
+  return getCurrentStack();
 }
diff --git a/packages/react/src/ReactSharedInternalsClient.js b/packages/react/src/ReactSharedInternalsClient.js
@@ -35,7 +35,7 @@ export type SharedStateClient = {
   thrownErrors: Array<mixed>,
 
   // ReactDebugCurrentFrame
-  getCurrentStack: null | ((stack: null | Error) => string),
+  getCurrentStack: null | (() => string),
 };
 
 export type RendererTask = boolean => RendererTask | null;
@@ -54,9 +54,7 @@ if (__DEV__) {
   ReactSharedInternals.didUsePromise = false;
   ReactSharedInternals.thrownErrors = [];
   // Stack implementation injected by the current renderer.
-  ReactSharedInternals.getCurrentStack = (null:
-    | null
-    | ((stack: null | Error) => string));
+  ReactSharedInternals.getCurrentStack = (null: null | (() => string));
 }
 
 export default ReactSharedInternals;
diff --git a/packages/react/src/ReactSharedInternalsServer.js b/packages/react/src/ReactSharedInternalsServer.js
@@ -38,7 +38,7 @@ export type SharedStateServer = {
   // DEV-only
 
   // ReactDebugCurrentFrame
-  getCurrentStack: null | ((stack: null | Error) => string),
+  getCurrentStack: null | (() => string),
 };
 
 export type RendererTask = boolean => RendererTask | null;
@@ -58,9 +58,7 @@ if (enableTaint) {
 
 if (__DEV__) {
   // Stack implementation injected by the current renderer.
-  ReactSharedInternals.getCurrentStack = (null:
-    | null
-    | ((stack: null | Error) => string));
+  ReactSharedInternals.getCurrentStack = (null: null | (() => string));
 }
 
 export default ReactSharedInternals;
diff --git a/packages/shared/forks/consoleWithStackDev.rn.js b/packages/shared/forks/consoleWithStackDev.rn.js
@@ -23,25 +23,25 @@ export function setSuppressWarning(newSuppressWarning) {
 export function warn(format, ...args) {
   if (__DEV__) {
     if (!suppressWarning) {
-      printWarning('warn', format, args, new Error('react-stack-top-frame'));
+      printWarning('warn', format, args);
     }
   }
 }
 
 export function error(format, ...args) {
   if (__DEV__) {
     if (!suppressWarning) {
-      printWarning('error', format, args, new Error('react-stack-top-frame'));
+      printWarning('error', format, args);
     }
   }
 }
 
 export let isWritingAppendedStack = false;
 
-function printWarning(level, format, args, currentStack) {
+function printWarning(level, format, args) {
   if (__DEV__) {
     if (ReactSharedInternals.getCurrentStack) {
-      const stack = ReactSharedInternals.getCurrentStack(currentStack);
+      const stack = ReactSharedInternals.getCurrentStack();
       if (stack !== '') {
         isWritingAppendedStack = true;
         format += '%s';
diff --git a/packages/shared/forks/consoleWithStackDev.www.js b/packages/shared/forks/consoleWithStackDev.www.js
@@ -18,27 +18,27 @@ export function setSuppressWarning(newSuppressWarning) {
 export function warn(format, ...args) {
   if (__DEV__) {
     if (!suppressWarning) {
-      printWarning('warn', format, args, new Error('react-stack-top-frame'));
+      printWarning('warn', format, args);
     }
   }
 }
 
 export function error(format, ...args) {
   if (__DEV__) {
     if (!suppressWarning) {
-      printWarning('error', format, args, new Error('react-stack-top-frame'));
+      printWarning('error', format, args);
     }
   }
 }
 
-function printWarning(level, format, args, currentStack) {
+function printWarning(level, format, args) {
   if (__DEV__) {
     const React = require('react');
     const ReactSharedInternals =
       React.__CLIENT_INTERNALS_DO_NOT_USE_OR_WARN_USERS_THEY_CANNOT_UPGRADE;
     // Defensive in case this is fired before React is initialized.
     if (ReactSharedInternals != null && ReactSharedInternals.getCurrentStack) {
-      const stack = ReactSharedInternals.getCurrentStack(currentStack);
+      const stack = ReactSharedInternals.getCurrentStack();
       if (stack !== '') {
         format += '%s';
         args.push(stack);
