diff --git a/packages/react-devtools-shared/src/backend/DevToolsOwnerStack.js b/packages/react-devtools-shared/src/backend/DevToolsOwnerStack.js index 5a859ed95f038..e03d948a45d3a 100644 --- a/packages/react-devtools-shared/src/backend/DevToolsOwnerStack.js +++ b/packages/react-devtools-shared/src/backend/DevToolsOwnerStack.js @@ -7,20 +7,9 @@ * @flow */ -// This is a DevTools fork of ReactFiberOwnerStack. +// This is a DevTools fork of shared/ReactOwnerStackFrames. -// TODO: Make this configurable? -const externalRegExp = /\/node\_modules\/|\(\/; - -function isNotExternal(stackFrame: string): boolean { - return !externalRegExp.test(stackFrame); -} - -function filterDebugStack(error: Error): string { - // Since stacks can be quite large and we pass a lot of them, we filter them out eagerly - // to save bandwidth even in DEV. We'll also replay these stacks on the client so by - // stripping them early we avoid that overhead. Otherwise we'd normally just rely on - // the DevTools or framework's ignore lists to filter them out. +export function formatOwnerStack(error: Error): string { const prevPrepareStackTrace = Error.prepareStackTrace; // $FlowFixMe[incompatible-type] It does accept undefined. Error.prepareStackTrace = undefined; @@ -31,7 +20,12 @@ function filterDebugStack(error: Error): string { // don't want/need. stack = stack.slice(29); } - let idx = stack.indexOf('react-stack-bottom-frame'); + let idx = stack.indexOf('\n'); + if (idx !== -1) { + // Pop the JSX frame. + stack = stack.slice(idx + 1); + } + idx = stack.indexOf('react-stack-bottom-frame'); if (idx !== -1) { idx = stack.lastIndexOf('\n', idx); } @@ -44,10 +38,5 @@ function filterDebugStack(error: Error): string { // To keep things light we exclude the entire trace in this case. return ''; } - const frames = stack.split('\n').slice(1); // Pop the JSX frame. - return frames.filter(isNotExternal).join('\n'); -} - -export function formatOwnerStack(ownerStackTrace: Error): string { - return filterDebugStack(ownerStackTrace); + return stack; } diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js index f51101b4d40b6..ced32ff87ef12 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js @@ -1848,6 +1848,7 @@ describe('ReactDOMFizzServer', () => { (gate(flags => flags.enableOwnerStacks) ? ' in span (at **)\n' + ' in mapper (at **)\n' + + ' in Array.map (at **)\n' + ' in B (at **)\n' + ' in A (at **)' : ' in span (at **)\n' + diff --git a/packages/react-reconciler/src/ReactFiberComponentStack.js b/packages/react-reconciler/src/ReactFiberComponentStack.js index a24db0c8d1d07..ac2cdf20a4cd6 100644 --- a/packages/react-reconciler/src/ReactFiberComponentStack.js +++ b/packages/react-reconciler/src/ReactFiberComponentStack.js @@ -30,7 +30,7 @@ import { describeClassComponentFrame, describeDebugInfoFrame, } from 'shared/ReactComponentStackFrame'; -import {formatOwnerStack} from './ReactFiberOwnerStack'; +import {formatOwnerStack} from 'shared/ReactOwnerStackFrames'; function describeFiber(fiber: Fiber): string { switch (fiber.tag) { diff --git a/packages/react-reconciler/src/ReactFiberOwnerStack.js b/packages/react-reconciler/src/ReactFiberOwnerStack.js deleted file mode 100644 index fbca1c2de3808..0000000000000 --- a/packages/react-reconciler/src/ReactFiberOwnerStack.js +++ /dev/null @@ -1,47 +0,0 @@ -/** - * 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 - */ - -// TODO: Make this configurable on the root. -const externalRegExp = /\/node\_modules\/|\(\/; - -function isNotExternal(stackFrame: string): boolean { - return !externalRegExp.test(stackFrame); -} - -function filterDebugStack(error: Error): string { - // Since stacks can be quite large and we pass a lot of them, we filter them out eagerly - // to save bandwidth even in DEV. We'll also replay these stacks on the client so by - // stripping them early we avoid that overhead. Otherwise we'd normally just rely on - // the DevTools or framework's ignore lists to filter them out. - let stack = error.stack; - if (stack.startsWith('Error: react-stack-top-frame\n')) { - // V8's default formatting prefixes with the error message which we - // don't want/need. - stack = stack.slice(29); - } - let idx = stack.indexOf('react-stack-bottom-frame'); - if (idx !== -1) { - idx = stack.lastIndexOf('\n', idx); - } - if (idx !== -1) { - // Cut off everything after the bottom frame since it'll be internals. - stack = stack.slice(0, idx); - } else { - // We didn't find any internal callsite out to user space. - // This means that this was called outside an owner or the owner is fully internal. - // To keep things light we exclude the entire trace in this case. - return ''; - } - const frames = stack.split('\n').slice(1); // Pop the JSX frame. - return frames.filter(isNotExternal).join('\n'); -} - -export function formatOwnerStack(ownerStackTrace: Error): string { - return filterDebugStack(ownerStackTrace); -} diff --git a/packages/react-server/src/ReactFizzComponentStack.js b/packages/react-server/src/ReactFizzComponentStack.js index ab306047bfbe0..7bec67038c14d 100644 --- a/packages/react-server/src/ReactFizzComponentStack.js +++ b/packages/react-server/src/ReactFizzComponentStack.js @@ -27,7 +27,7 @@ import { import {enableOwnerStacks} from 'shared/ReactFeatureFlags'; -import {formatOwnerStack} from './ReactFizzOwnerStack'; +import {formatOwnerStack} from 'shared/ReactOwnerStackFrames'; export type ComponentStackNode = { parent: null | ComponentStackNode, diff --git a/packages/react-server/src/ReactFizzOwnerStack.js b/packages/react-server/src/ReactFizzOwnerStack.js deleted file mode 100644 index fbca1c2de3808..0000000000000 --- a/packages/react-server/src/ReactFizzOwnerStack.js +++ /dev/null @@ -1,47 +0,0 @@ -/** - * 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 - */ - -// TODO: Make this configurable on the root. -const externalRegExp = /\/node\_modules\/|\(\/; - -function isNotExternal(stackFrame: string): boolean { - return !externalRegExp.test(stackFrame); -} - -function filterDebugStack(error: Error): string { - // Since stacks can be quite large and we pass a lot of them, we filter them out eagerly - // to save bandwidth even in DEV. We'll also replay these stacks on the client so by - // stripping them early we avoid that overhead. Otherwise we'd normally just rely on - // the DevTools or framework's ignore lists to filter them out. - let stack = error.stack; - if (stack.startsWith('Error: react-stack-top-frame\n')) { - // V8's default formatting prefixes with the error message which we - // don't want/need. - stack = stack.slice(29); - } - let idx = stack.indexOf('react-stack-bottom-frame'); - if (idx !== -1) { - idx = stack.lastIndexOf('\n', idx); - } - if (idx !== -1) { - // Cut off everything after the bottom frame since it'll be internals. - stack = stack.slice(0, idx); - } else { - // We didn't find any internal callsite out to user space. - // This means that this was called outside an owner or the owner is fully internal. - // To keep things light we exclude the entire trace in this case. - return ''; - } - const frames = stack.split('\n').slice(1); // Pop the JSX frame. - return frames.filter(isNotExternal).join('\n'); -} - -export function formatOwnerStack(ownerStackTrace: Error): string { - return filterDebugStack(ownerStackTrace); -} diff --git a/packages/react-server/src/ReactFlightCallUserSpace.js b/packages/react-server/src/ReactFlightCallUserSpace.js index 01c0492cc3370..4f7e065153f94 100644 --- a/packages/react-server/src/ReactFlightCallUserSpace.js +++ b/packages/react-server/src/ReactFlightCallUserSpace.js @@ -15,13 +15,6 @@ import type {ReactClientValue} from './ReactFlightServer'; import {setCurrentOwner} from './flight/ReactFlightCurrentOwner'; -import { - supportsComponentStorage, - componentStorage, -} from './ReactFlightServerConfig'; - -import {enableOwnerStacks} from 'shared/ReactFeatureFlags'; - // These indirections exists so we can exclude its stack frame in DEV (and anything below it). // TODO: Consider marking the whole bundle instead of these boundaries. @@ -30,38 +23,12 @@ const callComponent = { Component: (p: Props, arg: void) => R, props: Props, componentDebugInfo: ReactComponentInfo, - debugTask: null | ConsoleTask, ): R { // The secondArg is always undefined in Server Components since refs error early. const secondArg = undefined; setCurrentOwner(componentDebugInfo); try { - if (supportsComponentStorage) { - // Run the component in an Async Context that tracks the current owner. - if (enableOwnerStacks && debugTask) { - return debugTask.run( - // $FlowFixMe[method-unbinding] - componentStorage.run.bind( - componentStorage, - componentDebugInfo, - Component, - props, - secondArg, - ), - ); - } - return componentStorage.run( - componentDebugInfo, - Component, - props, - secondArg, - ); - } else { - if (enableOwnerStacks && debugTask) { - return debugTask.run(Component.bind(null, props, secondArg)); - } - return Component(props, secondArg); - } + return Component(props, secondArg); } finally { setCurrentOwner(null); } @@ -72,7 +39,6 @@ export const callComponentInDEV: ( Component: (p: Props, arg: void) => R, props: Props, componentDebugInfo: ReactComponentInfo, - debugTask: null | ConsoleTask, ) => R = __DEV__ ? // We use this technique to trick minifiers to preserve the function name. (callComponent['react-stack-bottom-frame'].bind(callComponent): any) diff --git a/packages/react-server/src/ReactFlightOwnerStack.js b/packages/react-server/src/ReactFlightOwnerStack.js deleted file mode 100644 index 72951bfb4745b..0000000000000 --- a/packages/react-server/src/ReactFlightOwnerStack.js +++ /dev/null @@ -1,42 +0,0 @@ -/** - * 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 - */ - -// TODO: Make this configurable on the Request. -const externalRegExp = /\/node\_modules\/| \(node\:| node\:|\(\/; - -function isNotExternal(stackFrame: string): boolean { - return !externalRegExp.test(stackFrame); -} - -function filterDebugStack(error: Error): string { - // Since stacks can be quite large and we pass a lot of them, we filter them out eagerly - // to save bandwidth even in DEV. We'll also replay these stacks on the client so by - // stripping them early we avoid that overhead. Otherwise we'd normally just rely on - // the DevTools or framework's ignore lists to filter them out. - let stack = error.stack; - if (stack.startsWith('Error: react-stack-top-frame\n')) { - // V8's default formatting prefixes with the error message which we - // don't want/need. - stack = stack.slice(29); - } - let idx = stack.indexOf('react-stack-bottom-frame'); - if (idx !== -1) { - idx = stack.lastIndexOf('\n', idx); - } - if (idx !== -1) { - // Cut off everything after the bottom frame since it'll be internals. - stack = stack.slice(0, idx); - } - const frames = stack.split('\n').slice(1); // Pop the JSX frame. - return frames.filter(isNotExternal).join('\n'); -} - -export function formatOwnerStack(ownerStackTrace: Error): string { - return filterDebugStack(ownerStackTrace); -} diff --git a/packages/react-server/src/ReactFlightServer.js b/packages/react-server/src/ReactFlightServer.js index fe2b258dd0b61..ae031c65219a9 100644 --- a/packages/react-server/src/ReactFlightServer.js +++ b/packages/react-server/src/ReactFlightServer.js @@ -80,6 +80,8 @@ import { createHints, initAsyncDebugInfo, parseStackTrace, + supportsComponentStorage, + componentStorage, } from './ReactFlightServerConfig'; import { @@ -1035,12 +1037,38 @@ function renderFunctionComponent( } } prepareToUseHooksForComponent(prevThenableState, componentDebugInfo); - result = callComponentInDEV( - Component, - props, - componentDebugInfo, - task.debugTask, - ); + if (supportsComponentStorage) { + // Run the component in an Async Context that tracks the current owner. + if (enableOwnerStacks && task.debugTask) { + result = task.debugTask.run( + // $FlowFixMe[method-unbinding] + componentStorage.run.bind( + componentStorage, + componentDebugInfo, + callComponentInDEV, + Component, + props, + componentDebugInfo, + ), + ); + } else { + result = componentStorage.run( + componentDebugInfo, + callComponentInDEV, + Component, + props, + componentDebugInfo, + ); + } + } else { + if (enableOwnerStacks && task.debugTask) { + result = task.debugTask.run( + callComponentInDEV.bind(null, Component, props, componentDebugInfo), + ); + } else { + result = callComponentInDEV(Component, props, componentDebugInfo); + } + } } else { prepareToUseHooksForComponent(prevThenableState, null); // The secondArg is always undefined in Server Components since refs error early. @@ -1222,19 +1250,47 @@ function warnForMissingKey( // Call with the server component as the currently rendering component // for context. - callComponentInDEV( - () => { - console.error( - 'Each child in a list should have a unique "key" prop.' + - '%s%s See https://react.dev/link/warning-keys for more information.', - '', - '', + const logKeyError = () => { + console.error( + 'Each child in a list should have a unique "key" prop.' + + '%s%s See https://react.dev/link/warning-keys for more information.', + '', + '', + ); + }; + + if (supportsComponentStorage) { + // Run the component in an Async Context that tracks the current owner. + if (enableOwnerStacks && debugTask) { + debugTask.run( + // $FlowFixMe[method-unbinding] + componentStorage.run.bind( + componentStorage, + componentDebugInfo, + callComponentInDEV, + logKeyError, + null, + componentDebugInfo, + ), ); - }, - null, - componentDebugInfo, - debugTask, - ); + } else { + componentStorage.run( + componentDebugInfo, + callComponentInDEV, + logKeyError, + null, + componentDebugInfo, + ); + } + } else { + if (enableOwnerStacks && debugTask) { + debugTask.run( + callComponentInDEV.bind(null, logKeyError, null, componentDebugInfo), + ); + } else { + callComponentInDEV(logKeyError, null, componentDebugInfo); + } + } } } diff --git a/packages/react-server/src/flight/ReactFlightComponentStack.js b/packages/react-server/src/flight/ReactFlightComponentStack.js index 36ee51dcf01d4..e4e545edaef76 100644 --- a/packages/react-server/src/flight/ReactFlightComponentStack.js +++ b/packages/react-server/src/flight/ReactFlightComponentStack.js @@ -13,7 +13,7 @@ import {describeBuiltInComponentFrame} from 'shared/ReactComponentStackFrame'; import {enableOwnerStacks} from 'shared/ReactFeatureFlags'; -import {formatOwnerStack} from '../ReactFlightOwnerStack'; +import {formatOwnerStack} from 'shared/ReactOwnerStackFrames'; export function getOwnerStackByComponentInfoInDev( componentInfo: ReactComponentInfo, diff --git a/packages/shared/ReactOwnerStackFrames.js b/packages/shared/ReactOwnerStackFrames.js new file mode 100644 index 0000000000000..829930430ada8 --- /dev/null +++ b/packages/shared/ReactOwnerStackFrames.js @@ -0,0 +1,40 @@ +/** + * 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 function formatOwnerStack(error: Error): string { + const prevPrepareStackTrace = Error.prepareStackTrace; + // $FlowFixMe[incompatible-type] It does accept undefined. + Error.prepareStackTrace = undefined; + let stack = error.stack; + Error.prepareStackTrace = prevPrepareStackTrace; + if (stack.startsWith('Error: react-stack-top-frame\n')) { + // V8's default formatting prefixes with the error message which we + // don't want/need. + stack = stack.slice(29); + } + let idx = stack.indexOf('\n'); + if (idx !== -1) { + // Pop the JSX frame. + stack = stack.slice(idx + 1); + } + idx = stack.indexOf('react-stack-bottom-frame'); + if (idx !== -1) { + idx = stack.lastIndexOf('\n', idx); + } + if (idx !== -1) { + // Cut off everything after the bottom frame since it'll be internals. + stack = stack.slice(0, idx); + } else { + // We didn't find any internal callsite out to user space. + // This means that this was called outside an owner or the owner is fully internal. + // To keep things light we exclude the entire trace in this case. + return ''; + } + return stack; +}