From 156eab2f7b34fe78ef26a1752f95bfdc2092819d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= Date: Tue, 5 Nov 2024 15:05:04 -0500 Subject: [PATCH] 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. --- .../src/ReactFlightStackConfigV8.js | 15 +-- packages/shared/DefaultPrepareStackTrace.js | 12 +++ packages/shared/DefaultPrepareStackTraceV8.js | 25 +++++ packages/shared/ReactComponentStackFrame.js | 5 +- packages/shared/ReactOwnerStackFrames.js | 5 +- .../DefaultPrepareStackTrace.dom-edge.js | 10 ++ .../DefaultPrepareStackTrace.dom-node.js | 10 ++ .../forks/DefaultPrepareStackTrace.markup.js | 10 ++ scripts/rollup/forks.js | 30 ++++++ scripts/shared/inlinedHostConfigs.js | 100 ++++++++---------- 10 files changed, 150 insertions(+), 72 deletions(-) create mode 100644 packages/shared/DefaultPrepareStackTrace.js create mode 100644 packages/shared/DefaultPrepareStackTraceV8.js create mode 100644 packages/shared/forks/DefaultPrepareStackTrace.dom-edge.js create mode 100644 packages/shared/forks/DefaultPrepareStackTrace.dom-node.js create mode 100644 packages/shared/forks/DefaultPrepareStackTrace.markup.js diff --git a/packages/react-server/src/ReactFlightStackConfigV8.js b/packages/react-server/src/ReactFlightStackConfigV8.js index 71465e3c5f..981e62dbdb 100644 --- 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 new file mode 100644 index 0000000000..5e0e3dc0da --- /dev/null +++ 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 new file mode 100644 index 0000000000..200c64b69d --- /dev/null +++ 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 index bef56120ff..96f69617e6 100644 --- 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 index 829930430a..d7543858f1 100644 --- 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 new file mode 100644 index 0000000000..b69f14e36a --- /dev/null +++ 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 new file mode 100644 index 0000000000..b69f14e36a --- /dev/null +++ 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 new file mode 100644 index 0000000000..b69f14e36a --- /dev/null +++ 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 index 8539ce7f7c..2d6f718c76 100644 --- 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 index 514ecc4567..3fc9c7b010 100644 --- 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'],