From 32f451bd7ac767feb30d896d53ffc83a20cf29b0 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Mon, 1 Jul 2024 13:12:36 -0400 Subject: [PATCH 1/2] Implement debugInfo in Fizz This lets us track Server Component parent stacks in Fizz which also lets us track the correct owner stack for lazy. In Fiber we're careful not to make any DEV only fibers but since the ReactFizzComponentStack data structures just exist for debug meta data anyway we can just expand on that. --- .../src/__tests__/ReactHTMLServer-test.js | 18 +- .../src/ReactFiberComponentStack.js | 8 +- .../src/ReactFizzComponentStack.js | 27 ++- packages/react-server/src/ReactFizzServer.js | 219 +++++++++++++----- 4 files changed, 192 insertions(+), 80 deletions(-) diff --git a/packages/react-html/src/__tests__/ReactHTMLServer-test.js b/packages/react-html/src/__tests__/ReactHTMLServer-test.js index 0d7b7c8da2d..01c5873b569 100644 --- a/packages/react-html/src/__tests__/ReactHTMLServer-test.js +++ b/packages/react-html/src/__tests__/ReactHTMLServer-test.js @@ -244,17 +244,19 @@ if (!__EXPERIMENTAL__) { expect(caughtErrors.length).toBe(1); expect(caughtErrors[0].error).toBe(thrownError); expect(normalizeCodeLocInfo(caughtErrors[0].parentStack)).toBe( - // TODO: Because Fizz doesn't yet implement debugInfo for parent stacks - // it doesn't have the Server Components in the parent stacks. - '\n in Lazy (at **)' + - '\n in div (at **)' + - '\n in div (at **)', + __DEV__ + ? '\n in Baz (at **)' + + '\n in div (at **)' + + '\n in Bar (at **)' + + '\n in Foo (at **)' + + '\n in div (at **)' + : '\n in Lazy (at **)' + + '\n in div (at **)' + + '\n in div (at **)', ); expect(normalizeCodeLocInfo(caughtErrors[0].ownerStack)).toBe( __DEV__ && gate(flags => flags.enableOwnerStacks) - ? // TODO: Because Fizz doesn't yet implement debugInfo for parent stacks - // it doesn't have the Server Components in the parent stacks. - '\n in Lazy (at **)' + ? '\n in Bar (at **)' + '\n in Foo (at **)' : null, ); }); diff --git a/packages/react-reconciler/src/ReactFiberComponentStack.js b/packages/react-reconciler/src/ReactFiberComponentStack.js index 8a69ba9ddfd..18f65530ea5 100644 --- a/packages/react-reconciler/src/ReactFiberComponentStack.js +++ b/packages/react-reconciler/src/ReactFiberComponentStack.js @@ -185,11 +185,11 @@ export function getOwnerStackByFiberInDev( // another code path anyway. I.e. this is likely NOT a V8 based browser. // This will cause some of the stack to have different formatting. // TODO: Normalize server component stacks to the client formatting. - if (owner.stack !== '') { - info += '\n' + owner.stack; + const ownerStack: string = owner.stack; + owner = owner.owner; + if (owner && ownerStack !== '') { + info += '\n' + ownerStack; } - const componentInfo: ReactComponentInfo = (owner: any); - owner = componentInfo.owner; } else { break; } diff --git a/packages/react-server/src/ReactFizzComponentStack.js b/packages/react-server/src/ReactFizzComponentStack.js index a16b2c5f913..ee8fd44c031 100644 --- a/packages/react-server/src/ReactFizzComponentStack.js +++ b/packages/react-server/src/ReactFizzComponentStack.js @@ -41,10 +41,19 @@ type ClassComponentStackNode = { owner?: null | ReactComponentInfo | ComponentStackNode, // DEV only stack?: null | string | Error, // DEV only }; +type ServerComponentStackNode = { + // DEV only + tag: 3, + parent: null | ComponentStackNode, + type: string, // name + env + owner?: null | ReactComponentInfo | ComponentStackNode, // DEV only + stack?: null | string | Error, // DEV only +}; export type ComponentStackNode = | BuiltInComponentStackNode | FunctionComponentStackNode - | ClassComponentStackNode; + | ClassComponentStackNode + | ServerComponentStackNode; export function getStackByComponentStackNode( componentStack: ComponentStackNode, @@ -63,6 +72,9 @@ export function getStackByComponentStackNode( case 2: info += describeClassComponentFrame(node.type); break; + case 3: + info += describeBuiltInComponentFrame(node.type); + break; } // $FlowFixMe[incompatible-type] we bail out when we get a null node = node.parent; @@ -110,6 +122,11 @@ export function getOwnerStackByComponentStackNodeInDev( ); } break; + case 3: + if (!componentStack.owner) { + info += describeBuiltInComponentFrame(componentStack.type); + } + break; } let owner: void | null | ComponentStackNode | ReactComponentInfo = @@ -137,11 +154,11 @@ export function getOwnerStackByComponentStackNodeInDev( } } else if (typeof owner.stack === 'string') { // Server Component - if (owner.stack !== '') { - info += '\n' + owner.stack; + const ownerStack: string = owner.stack; + owner = owner.owner; + if (owner && ownerStack !== '') { + info += '\n' + ownerStack; } - const componentInfo: ReactComponentInfo = (owner: any); - owner = componentInfo.owner; } else { break; } diff --git a/packages/react-server/src/ReactFizzServer.js b/packages/react-server/src/ReactFizzServer.js index 2c5dd0b1c3a..1fc905fe20d 100644 --- a/packages/react-server/src/ReactFizzServer.js +++ b/packages/react-server/src/ReactFizzServer.js @@ -21,6 +21,7 @@ import type { Thenable, ReactFormState, ReactComponentInfo, + ReactDebugInfo, } from 'shared/ReactTypes'; import type {LazyComponent as LazyComponentType} from 'react/src/ReactLazy'; import type { @@ -886,6 +887,41 @@ function createClassComponentStack( type, }; } +function createServerComponentStack( + task: Task, + debugInfo: void | null | ReactDebugInfo, +): null | ComponentStackNode { + // Build a Server Component parent stack from the debugInfo. + if (__DEV__) { + let node = task.componentStack; + if (debugInfo != null) { + const stack: ReactDebugInfo = debugInfo; + for (let i = 0; i < stack.length; i++) { + const componentInfo: ReactComponentInfo = (stack[i]: any); + if (typeof componentInfo.name !== 'string') { + continue; + } + let name = componentInfo.name; + const env = componentInfo.env; + if (env) { + name += ' (' + env + ')'; + } + node = { + tag: 3, + parent: node, + type: name, + owner: componentInfo.owner, + stack: componentInfo.stack, + }; + } + } + return node; + } + // eslint-disable-next-line react-internal/prod-error-codes + throw new Error( + 'createServerComponentStack should never be called in production. This is a bug in React.', + ); +} function createComponentStackFromType( task: Task, @@ -1982,6 +2018,7 @@ function renderLazyComponent( stack: null | Error, // DEV only ): void { const previousComponentStack = task.componentStack; + // TODO: Do we really need this stack frame? We don't on the client. task.componentStack = createBuiltInComponentStack(task, 'Lazy', owner, stack); let Component; if (__DEV__) { @@ -2533,72 +2570,90 @@ function renderNodeDestructive( const owner = __DEV__ ? element._owner : null; const stack = __DEV__ && enableOwnerStacks ? element._debugStack : null; + const previousComponentStack = task.componentStack; + if (__DEV__) { + task.componentStack = createServerComponentStack( + task, + element._debugInfo, + ); + } + const name = getComponentNameFromType(type); const keyOrIndex = key == null ? (childIndex === -1 ? 0 : childIndex) : key; const keyPath = [task.keyPath, name, keyOrIndex]; if (task.replay !== null) { - if (__DEV__ && enableOwnerStacks) { - const debugTask: null | ConsoleTask = element._debugTask; - if (debugTask) { - debugTask.run( - replayElement.bind( - null, - request, - task, - keyPath, - name, - keyOrIndex, - childIndex, - type, - props, - ref, - task.replay, - owner, - stack, - ), - ); - return; - } + const debugTask: null | ConsoleTask = + __DEV__ && enableOwnerStacks ? element._debugTask : null; + if (debugTask) { + debugTask.run( + replayElement.bind( + null, + request, + task, + keyPath, + name, + keyOrIndex, + childIndex, + type, + props, + ref, + task.replay, + owner, + stack, + ), + ); + } else { + replayElement( + request, + task, + keyPath, + name, + keyOrIndex, + childIndex, + type, + props, + ref, + task.replay, + owner, + stack, + ); } - replayElement( - request, - task, - keyPath, - name, - keyOrIndex, - childIndex, - type, - props, - ref, - task.replay, - owner, - stack, - ); // No matches found for this node. We assume it's already emitted in the // prelude and skip it during the replay. } else { // We're doing a plain render. - if (__DEV__ && enableOwnerStacks) { - const debugTask: null | ConsoleTask = element._debugTask; - if (debugTask) { - debugTask.run( - renderElement.bind( - null, - request, - task, - keyPath, - type, - props, - ref, - owner, - stack, - ), - ); - return; - } + const debugTask: null | ConsoleTask = + __DEV__ && enableOwnerStacks ? element._debugTask : null; + if (debugTask) { + debugTask.run( + renderElement.bind( + null, + request, + task, + keyPath, + type, + props, + ref, + owner, + stack, + ), + ); + } else { + renderElement( + request, + task, + keyPath, + type, + props, + ref, + owner, + stack, + ); } - renderElement(request, task, keyPath, type, props, ref, owner, stack); + } + if (__DEV__) { + task.componentStack = previousComponentStack; } return; } @@ -2608,14 +2663,23 @@ function renderNodeDestructive( 'Render them conditionally so that they only appear on the client render.', ); case REACT_LAZY_TYPE: { - const previousComponentStack = task.componentStack; - task.componentStack = createBuiltInComponentStack( - task, - 'Lazy', - null, - null, - ); const lazyNode: LazyComponentType = (node: any); + const previousComponentStack = task.componentStack; + if (__DEV__) { + task.componentStack = createServerComponentStack( + task, + lazyNode._debugInfo, + ); + } + if (!__DEV__ || task.componentStack === previousComponentStack) { + // TODO: Do we really need this stack frame? We don't on the client. + task.componentStack = createBuiltInComponentStack( + task, + 'Lazy', + null, + null, + ); + } let resolvedNode; if (__DEV__) { resolvedNode = callLazyInitInDEV(lazyNode); @@ -2746,12 +2810,23 @@ function renderNodeDestructive( // Clear any previous thenable state that was created by the unwrapping. task.thenableState = null; const thenable: Thenable = (maybeUsable: any); - return renderNodeDestructive( + const previousComponentStack = task.componentStack; + if (__DEV__) { + task.componentStack = createServerComponentStack( + task, + thenable._debugInfo, + ); + } + const result = renderNodeDestructive( request, task, unwrapThenable(thenable), childIndex, ); + if (__DEV__) { + task.componentStack = previousComponentStack; + } + return result; } if (maybeUsable.$$typeof === REACT_CONTEXT_TYPE) { @@ -2982,6 +3057,15 @@ function renderChildrenArray( childIndex: number, ): void { const prevKeyPath = task.keyPath; + const previousComponentStack = task.componentStack; + if (__DEV__) { + // We read debugInfo from task.node instead of children because it might have been an + // unwrapped iterable so we read from the original node. + task.componentStack = createServerComponentStack( + task, + (task.node: any)._debugInfo, + ); + } if (childIndex !== -1) { task.keyPath = [task.keyPath, 'Fragment', childIndex]; if (task.replay !== null) { @@ -2993,6 +3077,9 @@ function renderChildrenArray( childIndex, ); task.keyPath = prevKeyPath; + if (__DEV__) { + task.componentStack = previousComponentStack; + } return; } } @@ -3023,6 +3110,9 @@ function renderChildrenArray( } task.treeContext = prevTreeContext; task.keyPath = prevKeyPath; + if (__DEV__) { + task.componentStack = previousComponentStack; + } return; } } @@ -3042,6 +3132,9 @@ function renderChildrenArray( // only need to reset it to the previous value at the very end. task.treeContext = prevTreeContext; task.keyPath = prevKeyPath; + if (__DEV__) { + task.componentStack = previousComponentStack; + } } function trackPostpone( From 11fb115e610018242422fcf60ee8cf8ad5e71ded Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Tue, 2 Jul 2024 09:03:46 -0400 Subject: [PATCH 2/2] Gate Server Component stack frame outside of DEV --- packages/react-server/src/ReactFizzComponentStack.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/react-server/src/ReactFizzComponentStack.js b/packages/react-server/src/ReactFizzComponentStack.js index ee8fd44c031..6e3a31b2841 100644 --- a/packages/react-server/src/ReactFizzComponentStack.js +++ b/packages/react-server/src/ReactFizzComponentStack.js @@ -73,8 +73,10 @@ export function getStackByComponentStackNode( info += describeClassComponentFrame(node.type); break; case 3: - info += describeBuiltInComponentFrame(node.type); - break; + if (__DEV__) { + info += describeBuiltInComponentFrame(node.type); + break; + } } // $FlowFixMe[incompatible-type] we bail out when we get a null node = node.parent;