Fork the "empty" prepareStackTrace case for Server builds (#31427)

We don't actually want the source mapped version of `.stack` from errors because that would cause us to not be able to associate it with a source map in the UIs that need it. The strategy in browsers is more correct where the display is responsible for source maps. That's why we disable any custom `prepareStackTrace` like the ones added by `source-map`. We reset it to `undefined`. However, when running node with `--enable-source-maps` the default for `prepareStackTrace` which is a V8 feature (but may exist elsewhere too like Bun) is a source mapped version of the stack. In those environments we need to reset it to a default implementation that doesn't apply source maps. We already did this in Flight using the `ReactFlightStackConfigV8.js` config. However, we need this more generally in the `shared/ReactComponentStackFrame` implementation. We could always set it to the default implementation instead of `undefined` but that's unnecessary code in browser builds and it might lead to slightly different results. For safety and code size, this PR does it with a fork instead. All builds specific to `node` or `edge` (or `markup` which is a server feature) gets the default implementation where as everything else (e.g. browsers) get `undefined` since it's expected that this is not source mapped. We don't have to do anything about the equivalent in React DevTools since React DevTools doesn't run on the server.
facebook · Nov 5, 2024 · 156eab2 · 156eab2
1 parent b81e6dd
commit 156eab2
Show file tree

Hide file tree

Showing 10 changed files with 150 additions and 72 deletions.
diff --git a/packages/react-server/src/ReactFlightStackConfigV8.js b/packages/react-server/src/ReactFlightStackConfigV8.js
@@ -9,18 +9,7 @@
 
 import type {ReactStackTrace} from 'shared/ReactTypes';
 
-function prepareStackTrace(
-  error: Error,
-  structuredStackTrace: CallSite[],
-): string {
-  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;
-}
+import DefaultPrepareStackTrace from 'shared/DefaultPrepareStackTrace';
 
 function getStack(error: Error): string {
   // We override Error.prepareStackTrace with our own version that normalizes
@@ -30,7 +19,7 @@ function getStack(error: Error): string {
   // eagerly on the server. If the stack has already been read, then we might
   // not get a normalized stack and it might still have been source mapped.
   const previousPrepare = Error.prepareStackTrace;
-  Error.prepareStackTrace = prepareStackTrace;
+  Error.prepareStackTrace = DefaultPrepareStackTrace;
   try {
     // eslint-disable-next-line react-internal/safe-string-coercion
     return String(error.stack);

diff --git a/packages/shared/DefaultPrepareStackTrace.js b/packages/shared/DefaultPrepareStackTrace.js
@@ -0,0 +1,12 @@
+/**
+ * Copyright (c) Meta Platforms, Inc. and affiliates.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ *
+ * @flow
+ */
+
+// This is forked in server builds where the default stack frame may be source mapped.
+
+export default ((undefined: any): (Error, CallSite[]) => string);
diff --git a/packages/shared/DefaultPrepareStackTraceV8.js b/packages/shared/DefaultPrepareStackTraceV8.js
@@ -0,0 +1,25 @@
+/**
+ * Copyright (c) Meta Platforms, Inc. and affiliates.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ *
+ * @flow
+ */
+
+// This file replaces DefaultPrepareStackTrace in Edge/Node Server builds.
+
+function prepareStackTrace(
+  error: Error,
+  structuredStackTrace: CallSite[],
+): string {
+  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 default prepareStackTrace;
diff --git a/packages/shared/ReactComponentStackFrame.js b/packages/shared/ReactComponentStackFrame.js
@@ -23,6 +23,8 @@ import {disableLogs, reenableLogs} from 'shared/ConsolePatchingDev';
 
 import ReactSharedInternals from 'shared/ReactSharedInternals';
 
+import DefaultPrepareStackTrace from 'shared/DefaultPrepareStackTrace';
+
 let prefix;
 let suffix;
 export function describeBuiltInComponentFrame(name: string): string {
@@ -92,8 +94,7 @@ export function describeNativeComponentFrame(
 
   reentry = true;
   const previousPrepareStackTrace = Error.prepareStackTrace;
-  // $FlowFixMe[incompatible-type] It does accept undefined.
-  Error.prepareStackTrace = undefined;
+  Error.prepareStackTrace = DefaultPrepareStackTrace;
   let previousDispatcher = null;
 
   if (__DEV__) {

diff --git a/packages/shared/ReactOwnerStackFrames.js b/packages/shared/ReactOwnerStackFrames.js
@@ -7,10 +7,11 @@
  * @flow
  */
 
+import DefaultPrepareStackTrace from 'shared/DefaultPrepareStackTrace';
+
 export function formatOwnerStack(error: Error): string {
   const prevPrepareStackTrace = Error.prepareStackTrace;
-  // $FlowFixMe[incompatible-type] It does accept undefined.
-  Error.prepareStackTrace = undefined;
+  Error.prepareStackTrace = DefaultPrepareStackTrace;
   let stack = error.stack;
   Error.prepareStackTrace = prevPrepareStackTrace;
   if (stack.startsWith('Error: react-stack-top-frame\n')) {

diff --git a/packages/shared/forks/DefaultPrepareStackTrace.dom-edge.js b/packages/shared/forks/DefaultPrepareStackTrace.dom-edge.js
@@ -0,0 +1,10 @@
+/**
+ * Copyright (c) Meta Platforms, Inc. and affiliates.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ *
+ * @flow
+ */
+
+export {default} from '../DefaultPrepareStackTraceV8';
diff --git a/packages/shared/forks/DefaultPrepareStackTrace.dom-node.js b/packages/shared/forks/DefaultPrepareStackTrace.dom-node.js
@@ -0,0 +1,10 @@
+/**
+ * Copyright (c) Meta Platforms, Inc. and affiliates.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ *
+ * @flow
+ */
+
+export {default} from '../DefaultPrepareStackTraceV8';
diff --git a/packages/shared/forks/DefaultPrepareStackTrace.markup.js b/packages/shared/forks/DefaultPrepareStackTrace.markup.js
@@ -0,0 +1,10 @@
+/**
+ * Copyright (c) Meta Platforms, Inc. and affiliates.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ *
+ * @flow
+ */
+
+export {default} from '../DefaultPrepareStackTraceV8';
diff --git a/scripts/rollup/forks.js b/scripts/rollup/forks.js
@@ -217,6 +217,36 @@ const forks = Object.freeze({
     }
   },
 
+  './packages/shared/DefaultPrepareStackTrace.js': (
+    bundleType,
+    entry,
+    dependencies,
+    moduleType
+  ) => {
+    if (moduleType !== RENDERER && moduleType !== RECONCILER) {
+      return null;
+    }
+    // eslint-disable-next-line no-for-of-loops/no-for-of-loops
+    for (let rendererInfo of inlinedHostConfigs) {
+      if (rendererInfo.entryPoints.indexOf(entry) !== -1) {
+        if (!rendererInfo.isServerSupported) {
+          return null;
+        }
+        const foundFork = findNearestExistingForkFile(
+          './packages/shared/forks/DefaultPrepareStackTrace.',
+          rendererInfo.shortName,
+          '.js'
+        );
+        if (foundFork) {
+          return foundFork;
+        }
+        // fall through to error
+        break;
+      }
+    }
+    return null;
+  },
+
   './packages/react-reconciler/src/ReactFiberConfig.js': (
     bundleType,
     entry,

diff --git a/scripts/shared/inlinedHostConfigs.js b/scripts/shared/inlinedHostConfigs.js
@@ -8,12 +8,55 @@
 
 module.exports = [
   {
-    shortName: 'dom-node',
+    shortName: 'dom-browser',
     entryPoints: [
       'react-dom',
       'react-dom/client',
       'react-dom/profiling',
       'react-dom/unstable_testing',
+      'react-dom/src/server/react-dom-server.browser.js',
+      'react-dom/static.browser',
+      'react-dom/unstable_server-external-runtime',
+      'react-server-dom-webpack/client.browser',
+      'react-server-dom-webpack/src/server/react-flight-dom-server.browser',
+    ],
+    paths: [
+      'react-dom',
+      'react-dom/src/ReactDOMReactServer.js',
+      'react-dom-bindings',
+      'react-dom/client',
+      'react-dom/profiling',
+      'react-dom/server.browser',
+      'react-dom/static.browser',
+      'react-dom/unstable_testing',
+      'react-dom/src/server/react-dom-server.browser',
+      'react-dom/src/server/ReactDOMFizzServerBrowser.js', // react-dom/server.browser
+      'react-dom/src/server/ReactDOMFizzStaticBrowser.js',
+      'react-dom-bindings/src/server/ReactDOMFlightServerHostDispatcher.js',
+      'react-dom-bindings/src/server/ReactFlightServerConfigDOM.js',
+      'react-dom-bindings/src/shared/ReactFlightClientConfigDOM.js',
+      'react-server-dom-webpack',
+      'react-server-dom-webpack/client',
+      'react-server-dom-webpack/client.browser',
+      'react-server-dom-webpack/server.browser',
+      'react-server-dom-webpack/static.browser',
+      'react-server-dom-webpack/src/client/ReactFlightDOMClientBrowser.js', // react-server-dom-webpack/client.browser
+      'react-server-dom-webpack/src/client/ReactFlightClientConfigBundlerWebpack.js',
+      'react-server-dom-webpack/src/client/ReactFlightClientConfigBundlerWebpackBrowser.js',
+      'react-server-dom-webpack/src/server/react-flight-dom-server.browser',
+      'react-server-dom-webpack/src/server/ReactFlightDOMServerBrowser.js', // react-server-dom-webpack/src/server/react-flight-dom-server.browser
+      'react-devtools',
+      'react-devtools-core',
+      'react-devtools-shell',
+      'react-devtools-shared',
+      'shared/ReactDOMSharedInternals',
+    ],
+    isFlowTyped: true,
+    isServerSupported: true,
+  },
+  {
+    shortName: 'dom-node',
+    entryPoints: [
       'react-dom/src/ReactDOMReactServer.js',
       'react-dom/src/server/react-dom-server.node.js',
       'react-dom/static.node',
@@ -148,13 +191,7 @@ module.exports = [
   },
   {
     shortName: 'dom-bun',
-    entryPoints: [
-      'react-dom',
-      'react-dom/client',
-      'react-dom/profiling',
-      'react-dom/unstable_testing',
-      'react-dom/src/server/react-dom-server.bun.js',
-    ],
+    entryPoints: ['react-dom/src/server/react-dom-server.bun.js'],
     paths: [
       'react-dom',
       'react-dom/client',
@@ -171,53 +208,6 @@ module.exports = [
     isFlowTyped: true,
     isServerSupported: true,
   },
-  {
-    shortName: 'dom-browser',
-    entryPoints: [
-      'react-dom',
-      'react-dom/client',
-      'react-dom/profiling',
-      'react-dom/unstable_testing',
-      'react-dom/src/server/react-dom-server.browser.js',
-      'react-dom/static.browser',
-      'react-dom/unstable_server-external-runtime',
-      'react-server-dom-webpack/client.browser',
-      'react-server-dom-webpack/src/server/react-flight-dom-server.browser',
-    ],
-    paths: [
-      'react-dom',
-      'react-dom/src/ReactDOMReactServer.js',
-      'react-dom-bindings',
-      'react-dom/client',
-      'react-dom/profiling',
-      'react-dom/server.browser',
-      'react-dom/static.browser',
-      'react-dom/unstable_testing',
-      'react-dom/src/server/react-dom-server.browser',
-      'react-dom/src/server/ReactDOMFizzServerBrowser.js', // react-dom/server.browser
-      'react-dom/src/server/ReactDOMFizzStaticBrowser.js',
-      'react-dom-bindings/src/server/ReactDOMFlightServerHostDispatcher.js',
-      'react-dom-bindings/src/server/ReactFlightServerConfigDOM.js',
-      'react-dom-bindings/src/shared/ReactFlightClientConfigDOM.js',
-      'react-server-dom-webpack',
-      'react-server-dom-webpack/client',
-      'react-server-dom-webpack/client.browser',
-      'react-server-dom-webpack/server.browser',
-      'react-server-dom-webpack/static.browser',
-      'react-server-dom-webpack/src/client/ReactFlightDOMClientBrowser.js', // react-server-dom-webpack/client.browser
-      'react-server-dom-webpack/src/client/ReactFlightClientConfigBundlerWebpack.js',
-      'react-server-dom-webpack/src/client/ReactFlightClientConfigBundlerWebpackBrowser.js',
-      'react-server-dom-webpack/src/server/react-flight-dom-server.browser',
-      'react-server-dom-webpack/src/server/ReactFlightDOMServerBrowser.js', // react-server-dom-webpack/src/server/react-flight-dom-server.browser
-      'react-devtools',
-      'react-devtools-core',
-      'react-devtools-shell',
-      'react-devtools-shared',
-      'shared/ReactDOMSharedInternals',
-    ],
-    isFlowTyped: true,
-    isServerSupported: true,
-  },
   {
     shortName: 'dom-browser-esm',
     entryPoints: ['react-server-dom-esm/client.browser'],