From 654e387d7eac113ddbf85f8a9029d1af7117679e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= Date: Mon, 30 Sep 2024 22:39:20 -0700 Subject: [PATCH] [Flight] Serialize Server Components Props in DEV (#31105) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This allows us to show props in React DevTools when inspecting a Server Component. I currently drastically limit the object depth that's serialized since this is very implicit and you can have heavy objects on the server. We previously was using the general outlineModel to outline ReactComponentInfo but we weren't consistently using it everywhere which could cause some bugs with the parsing when it got deduped on the client. It also lead to the weird feature detect of `isReactComponent`. It also meant that this serialization was using the plain serialization instead of `renderConsoleValue` which means we couldn't safely serialize arbitrary debug info that isn't serializable there. So the main change here is to call `outlineComponentInfo` and have that always write every "Server Component" instance as outlined and in a way that lets its props be serialized using `renderConsoleValue`. Screenshot 2024-10-01 at 1 25 05 AM --- .../react-client/src/ReactFlightClient.js | 21 +- .../src/__tests__/ReactFlight-test.js | 33 +++ .../src/backend/fiber/renderer.js | 3 +- .../react-devtools-shared/src/hydration.js | 58 ++++- .../__tests__/ReactFlightDOMBrowser-test.js | 8 +- .../react-server/src/ReactFlightServer.js | 218 +++++++++++------- packages/shared/ReactTypes.js | 1 + 7 files changed, 245 insertions(+), 97 deletions(-) diff --git a/packages/react-client/src/ReactFlightClient.js b/packages/react-client/src/ReactFlightClient.js index 98a52c4ec4a9a..2f60ccddb4b4d 100644 --- a/packages/react-client/src/ReactFlightClient.js +++ b/packages/react-client/src/ReactFlightClient.js @@ -2640,11 +2640,22 @@ function processFullStringRow( } case 68 /* "D" */: { if (__DEV__) { - const debugInfo: ReactComponentInfo | ReactAsyncInfo = parseModel( - response, - row, - ); - resolveDebugInfo(response, id, debugInfo); + const chunk: ResolvedModelChunk = + createResolvedModelChunk(response, row); + initializeModelChunk(chunk); + const initializedChunk: SomeChunk = + chunk; + if (initializedChunk.status === INITIALIZED) { + resolveDebugInfo(response, id, initializedChunk.value); + } else { + // TODO: This is not going to resolve in the right order if there's more than one. + chunk.then( + v => resolveDebugInfo(response, id, v), + e => { + // Ignore debug info errors for now. Unnecessary noise. + }, + ); + } return; } // Fallthrough to share the error with Console entries. diff --git a/packages/react-client/src/__tests__/ReactFlight-test.js b/packages/react-client/src/__tests__/ReactFlight-test.js index 27db069ef324f..857ce99868d9b 100644 --- a/packages/react-client/src/__tests__/ReactFlight-test.js +++ b/packages/react-client/src/__tests__/ReactFlight-test.js @@ -308,6 +308,10 @@ describe('ReactFlight', () => { stack: gate(flag => flag.enableOwnerStacks) ? ' in Object. (at **)' : undefined, + props: { + firstName: 'Seb', + lastName: 'Smith', + }, }, ] : undefined, @@ -347,6 +351,10 @@ describe('ReactFlight', () => { stack: gate(flag => flag.enableOwnerStacks) ? ' in Object. (at **)' : undefined, + props: { + firstName: 'Seb', + lastName: 'Smith', + }, }, ] : undefined, @@ -2665,6 +2673,9 @@ describe('ReactFlight', () => { stack: gate(flag => flag.enableOwnerStacks) ? ' in Object. (at **)' : undefined, + props: { + transport: expect.arrayContaining([]), + }, }, ] : undefined, @@ -2683,6 +2694,7 @@ describe('ReactFlight', () => { stack: gate(flag => flag.enableOwnerStacks) ? ' in Object. (at **)' : undefined, + props: {}, }, ] : undefined, @@ -2698,6 +2710,7 @@ describe('ReactFlight', () => { stack: gate(flag => flag.enableOwnerStacks) ? ' in myLazy (at **)\n in lazyInitializer (at **)' : undefined, + props: {}, }, ] : undefined, @@ -2713,6 +2726,7 @@ describe('ReactFlight', () => { stack: gate(flag => flag.enableOwnerStacks) ? ' in Object. (at **)' : undefined, + props: {}, }, ] : undefined, @@ -2787,6 +2801,9 @@ describe('ReactFlight', () => { stack: gate(flag => flag.enableOwnerStacks) ? ' in Object. (at **)' : undefined, + props: { + transport: expect.arrayContaining([]), + }, }, ] : undefined, @@ -2804,6 +2821,9 @@ describe('ReactFlight', () => { stack: gate(flag => flag.enableOwnerStacks) ? ' in ServerComponent (at **)' : undefined, + props: { + children: {}, + }, }, ] : undefined, @@ -2820,6 +2840,7 @@ describe('ReactFlight', () => { stack: gate(flag => flag.enableOwnerStacks) ? ' in Object. (at **)' : undefined, + props: {}, }, ] : undefined, @@ -2978,6 +2999,7 @@ describe('ReactFlight', () => { stack: gate(flag => flag.enableOwnerStacks) ? ' in Object. (at **)' : undefined, + props: {}, }, { env: 'B', @@ -3108,6 +3130,9 @@ describe('ReactFlight', () => { stack: gate(flag => flag.enableOwnerStacks) ? ' in Object. (at **)' : undefined, + props: { + firstName: 'Seb', + }, }; expect(getDebugInfo(greeting)).toEqual([ greetInfo, @@ -3119,6 +3144,14 @@ describe('ReactFlight', () => { stack: gate(flag => flag.enableOwnerStacks) ? ' in Greeting (at **)' : undefined, + props: { + children: expect.objectContaining({ + type: 'span', + props: { + children: ['Hello, ', 'Seb'], + }, + }), + }, }, ]); // The owner that created the span was the outer server component. diff --git a/packages/react-devtools-shared/src/backend/fiber/renderer.js b/packages/react-devtools-shared/src/backend/fiber/renderer.js index 4fac8839ae799..9732bf105ca6c 100644 --- a/packages/react-devtools-shared/src/backend/fiber/renderer.js +++ b/packages/react-devtools-shared/src/backend/fiber/renderer.js @@ -4348,8 +4348,7 @@ export function attach( const componentInfo = virtualInstance.data; const key = typeof componentInfo.key === 'string' ? componentInfo.key : null; - const props = null; // TODO: Track props on ReactComponentInfo; - + const props = componentInfo.props == null ? null : componentInfo.props; const owners: null | Array = getOwnersListFromInstance(virtualInstance); diff --git a/packages/react-devtools-shared/src/hydration.js b/packages/react-devtools-shared/src/hydration.js index c5b78135e74a2..c21efe40a88fa 100644 --- a/packages/react-devtools-shared/src/hydration.js +++ b/packages/react-devtools-shared/src/hydration.js @@ -216,16 +216,19 @@ export function dehydrate( if (level >= LEVEL_THRESHOLD && !isPathAllowedCheck) { return createDehydrated(type, true, data, cleaned, path); } - return data.map((item, i) => - dehydrate( - item, + const arr: Array = []; + for (let i = 0; i < data.length; i++) { + arr[i] = dehydrateKey( + data, + i, cleaned, unserializable, path.concat([i]), isPathAllowed, isPathAllowedCheck ? 1 : level + 1, - ), - ); + ); + } + return arr; case 'html_all_collection': case 'typed_array': @@ -311,8 +314,9 @@ export function dehydrate( } = {}; getAllEnumerableKeys(data).forEach(key => { const name = key.toString(); - object[name] = dehydrate( - data[key], + object[name] = dehydrateKey( + data, + key, cleaned, unserializable, path.concat([name]), @@ -373,6 +377,46 @@ export function dehydrate( } } +function dehydrateKey( + parent: Object, + key: number | string | symbol, + cleaned: Array>, + unserializable: Array>, + path: Array, + isPathAllowed: (path: Array) => boolean, + level: number = 0, +): $PropertyType { + try { + return dehydrate( + parent[key], + cleaned, + unserializable, + path, + isPathAllowed, + level, + ); + } catch (error) { + let preview = ''; + if ( + typeof error === 'object' && + error !== null && + typeof error.stack === 'string' + ) { + preview = error.stack; + } else if (typeof error === 'string') { + preview = error; + } + cleaned.push(path); + return { + inspectable: false, + preview_short: '[Exception]', + preview_long: preview ? '[Exception: ' + preview + ']' : '[Exception]', + name: preview, + type: 'unknown', + }; + } +} + export function fillInPath( object: Object, data: DehydratedData, diff --git a/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOMBrowser-test.js b/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOMBrowser-test.js index 0408a63fc512f..882c0bbb01969 100644 --- a/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOMBrowser-test.js +++ b/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOMBrowser-test.js @@ -709,7 +709,7 @@ describe('ReactFlightDOMBrowser', () => { expect(container.innerHTML).toBe(expectedHtml); if (__DEV__) { - const resolvedPath1b = await response.value[0].props.children[1]._payload; + const resolvedPath1b = response.value[0].props.children[1]; expect(resolvedPath1b._owner).toEqual( expect.objectContaining({ @@ -1028,8 +1028,10 @@ describe('ReactFlightDOMBrowser', () => { expect(flightResponse).toContain('(loading everything)'); expect(flightResponse).toContain('(loading sidebar)'); expect(flightResponse).toContain('(loading posts)'); - expect(flightResponse).not.toContain(':friends:'); - expect(flightResponse).not.toContain(':name:'); + if (!__DEV__) { + expect(flightResponse).not.toContain(':friends:'); + expect(flightResponse).not.toContain(':name:'); + } await serverAct(() => { resolveFriends(); diff --git a/packages/react-server/src/ReactFlightServer.js b/packages/react-server/src/ReactFlightServer.js index 19c40c214b918..5db03d628146f 100644 --- a/packages/react-server/src/ReactFlightServer.js +++ b/packages/react-server/src/ReactFlightServer.js @@ -1148,14 +1148,20 @@ function renderFunctionComponent( ? null : filterStackTrace(request, task.debugStack, 1); // $FlowFixMe[cannot-write] + componentDebugInfo.props = props; + // $FlowFixMe[cannot-write] componentDebugInfo.debugStack = task.debugStack; // $FlowFixMe[cannot-write] componentDebugInfo.debugTask = task.debugTask; + } else { + // $FlowFixMe[cannot-write] + componentDebugInfo.props = props; } // We outline this model eagerly so that we can refer to by reference as an owner. // If we had a smarter way to dedupe we might not have to do this if there ends up // being no references to this as an owner. - outlineModel(request, componentDebugInfo); + + outlineComponentInfo(request, componentDebugInfo); emitDebugChunk(request, componentDebugID, componentDebugInfo); // We've emitted the latest environment for this task so we track that. @@ -1582,6 +1588,13 @@ function renderClientElement( } else if (keyPath !== null) { key = keyPath + ',' + key; } + if (__DEV__) { + if (task.debugOwner !== null) { + // Ensure we outline this owner if it is the first time we see it. + // So that we can refer to it directly. + outlineComponentInfo(request, task.debugOwner); + } + } const element = __DEV__ ? enableOwnerStacks ? [ @@ -1702,6 +1715,7 @@ function renderElement( task.debugStack === null ? null : filterStackTrace(request, task.debugStack, 1), + props: props, debugStack: task.debugStack, debugTask: task.debugTask, }; @@ -2128,7 +2142,7 @@ function serializeSet(request: Request, set: Set): string { function serializeConsoleMap( request: Request, - counter: {objectCount: number}, + counter: {objectLimit: number}, map: Map, ): string { // Like serializeMap but for renderConsoleValue. @@ -2139,7 +2153,7 @@ function serializeConsoleMap( function serializeConsoleSet( request: Request, - counter: {objectCount: number}, + counter: {objectLimit: number}, set: Set, ): string { // Like serializeMap but for renderConsoleValue. @@ -2263,23 +2277,6 @@ function escapeStringValue(value: string): string { } } -function isReactComponentInfo(value: any): boolean { - // TODO: We don't currently have a brand check on ReactComponentInfo. Reconsider. - return ( - ((typeof value.debugTask === 'object' && - value.debugTask !== null && - // $FlowFixMe[method-unbinding] - typeof value.debugTask.run === 'function') || - value.debugStack instanceof Error) && - (enableOwnerStacks - ? isArray((value: any).stack) || (value: any).stack === null - : typeof (value: any).stack === 'undefined') && - typeof value.name === 'string' && - typeof value.env === 'string' && - value.owner !== undefined - ); -} - let modelRoot: null | ReactClientValue = false; function renderModel( @@ -2795,25 +2792,6 @@ function renderModelDestructive( ); } if (__DEV__) { - if (isReactComponentInfo(value)) { - // This looks like a ReactComponentInfo. We can't serialize the ConsoleTask object so we - // need to omit it before serializing. - const componentDebugInfo: Omit< - ReactComponentInfo, - 'debugTask' | 'debugStack', - > = { - name: (value: any).name, - env: (value: any).env, - key: (value: any).key, - owner: (value: any).owner, - }; - if (enableOwnerStacks) { - // $FlowFixMe[cannot-write] - componentDebugInfo.stack = (value: any).stack; - } - return componentDebugInfo; - } - if (objectName(value) !== 'Object') { callWithDebugContextInDEV(request, task, () => { console.error( @@ -3241,7 +3219,7 @@ function emitDebugChunk( // We use the console encoding so that we can dedupe objects but don't necessarily // use the full serialization that requires a task. - const counter = {objectCount: 0}; + const counter = {objectLimit: 500}; function replacer( this: | {+[key: string | number]: ReactClientValue} @@ -3265,6 +3243,61 @@ function emitDebugChunk( request.completedRegularChunks.push(processedChunk); } +function outlineComponentInfo( + request: Request, + componentInfo: ReactComponentInfo, +): void { + if (!__DEV__) { + // These errors should never make it into a build so we don't need to encode them in codes.json + // eslint-disable-next-line react-internal/prod-error-codes + throw new Error( + 'outlineComponentInfo should never be called in production mode. This is a bug in React.', + ); + } + + if (request.writtenObjects.has(componentInfo)) { + // Already written + return; + } + + if (componentInfo.owner != null) { + // Ensure the owner is already outlined. + outlineComponentInfo(request, componentInfo.owner); + } + + // Limit the number of objects we write to prevent emitting giant props objects. + let objectLimit = 10; + if (componentInfo.stack != null) { + // Ensure we have enough object limit to encode the stack trace. + objectLimit += componentInfo.stack.length; + } + + // We use the console encoding so that we can dedupe objects but don't necessarily + // use the full serialization that requires a task. + const counter = {objectLimit}; + + // We can't serialize the ConsoleTask/Error objects so we need to omit them before serializing. + const componentDebugInfo: Omit< + ReactComponentInfo, + 'debugTask' | 'debugStack', + > = { + name: componentInfo.name, + env: componentInfo.env, + key: componentInfo.key, + owner: componentInfo.owner, + }; + if (enableOwnerStacks) { + // $FlowFixMe[cannot-write] + componentDebugInfo.stack = componentInfo.stack; + } + // Ensure we serialize props after the stack to favor the stack being complete. + // $FlowFixMe[cannot-write] + componentDebugInfo.props = componentInfo.props; + + const id = outlineConsoleValue(request, counter, componentDebugInfo); + request.writtenObjects.set(componentInfo, serializeByValueID(id)); +} + function emitTypedArrayChunk( request: Request, id: number, @@ -3322,7 +3355,7 @@ function serializeEval(source: string): string { // in the depth it can encode. function renderConsoleValue( request: Request, - counter: {objectCount: number}, + counter: {objectLimit: number}, parent: | {+[propertyName: string | number]: ReactClientValue} | $ReadOnlyArray, @@ -3366,23 +3399,64 @@ function renderConsoleValue( } } - if (counter.objectCount > 500) { + const writtenObjects = request.writtenObjects; + const existingReference = writtenObjects.get(value); + if (existingReference !== undefined) { + // We've already emitted this as a real object, so we can + // just refer to that by its existing reference. + return existingReference; + } + + if (counter.objectLimit <= 0) { // We've reached our max number of objects to serialize across the wire so we serialize this // as a marker so that the client can error when this is accessed by the console. return serializeLimitedObject(); } - counter.objectCount++; + counter.objectLimit--; - const writtenObjects = request.writtenObjects; - const existingReference = writtenObjects.get(value); - // $FlowFixMe[method-unbinding] - if (typeof value.then === 'function') { - if (existingReference !== undefined) { - // We've seen this promise before, so we can just refer to the same result. - return existingReference; + switch ((value: any).$$typeof) { + case REACT_ELEMENT_TYPE: { + const element: ReactElement = (value: any); + + if (element._owner != null) { + outlineComponentInfo(request, element._owner); + } + if (enableOwnerStacks) { + let debugStack: null | ReactStackTrace = null; + if (element._debugStack != null) { + // Outline the debug stack so that it doesn't get cut off. + debugStack = filterStackTrace(request, element._debugStack, 1); + const stackId = outlineConsoleValue( + request, + {objectLimit: debugStack.length + 2}, + debugStack, + ); + request.writtenObjects.set(debugStack, serializeByValueID(stackId)); + } + return [ + REACT_ELEMENT_TYPE, + element.type, + element.key, + element.props, + element._owner, + debugStack, + element._store.validated, + ]; + } + + return [ + REACT_ELEMENT_TYPE, + element.type, + element.key, + element.props, + element._owner, + ]; } + } + // $FlowFixMe[method-unbinding] + if (typeof value.then === 'function') { const thenable: Thenable = (value: any); switch (thenable.status) { case 'fulfilled': { @@ -3416,12 +3490,6 @@ function renderConsoleValue( return serializeInfinitePromise(); } - if (existingReference !== undefined) { - // We've already emitted this as a real object, so we can - // just refer to that by its existing reference. - return existingReference; - } - if (isArray(value)) { return value; } @@ -3503,25 +3571,6 @@ function renderConsoleValue( return Array.from((value: any)); } - if (isReactComponentInfo(value)) { - // This looks like a ReactComponentInfo. We can't serialize the ConsoleTask object so we - // need to omit it before serializing. - const componentDebugInfo: Omit< - ReactComponentInfo, - 'debugTask' | 'debugStack', - > = { - name: (value: any).name, - env: (value: any).env, - key: (value: any).key, - owner: (value: any).owner, - }; - if (enableOwnerStacks) { - // $FlowFixMe[cannot-write] - componentDebugInfo.stack = (value: any).stack; - } - return componentDebugInfo; - } - // $FlowFixMe[incompatible-return] return value; } @@ -3602,7 +3651,7 @@ function renderConsoleValue( function outlineConsoleValue( request: Request, - counter: {objectCount: number}, + counter: {objectLimit: number}, model: ReactClientValue, ): number { if (!__DEV__) { @@ -3629,7 +3678,9 @@ function outlineConsoleValue( value, ); } catch (x) { - return 'unknown value'; + return ( + 'Unknown Value: React could not send it from the server.\n' + x.message + ); } } @@ -3660,7 +3711,7 @@ function emitConsoleChunk( ); } - const counter = {objectCount: 0}; + const counter = {objectLimit: 500}; function replacer( this: | {+[key: string | number]: ReactClientValue} @@ -3677,10 +3728,17 @@ function emitConsoleChunk( value, ); } catch (x) { - return 'unknown value'; + return ( + 'Unknown Value: React could not send it from the server.\n' + x.message + ); } } + // Ensure the owner is already outlined. + if (owner != null) { + outlineComponentInfo(request, owner); + } + // TODO: Don't double badge if this log came from another Flight Client. const env = (0, request.environmentName)(); const payload = [methodName, stackTrace, owner, env]; @@ -3704,7 +3762,7 @@ function forwardDebugInfo( // We outline this model eagerly so that we can refer to by reference as an owner. // If we had a smarter way to dedupe we might not have to do this if there ends up // being no references to this as an owner. - outlineModel(request, debugInfo[i]); + outlineComponentInfo(request, (debugInfo[i]: any)); } emitDebugChunk(request, id, debugInfo[i]); } diff --git a/packages/shared/ReactTypes.js b/packages/shared/ReactTypes.js index 7d51fbe4725d3..54eccd5538dd8 100644 --- a/packages/shared/ReactTypes.js +++ b/packages/shared/ReactTypes.js @@ -193,6 +193,7 @@ export type ReactComponentInfo = { +key?: null | string, +owner?: null | ReactComponentInfo, +stack?: null | ReactStackTrace, + +props?: null | {[name: string]: mixed}, // Stashed Data for the Specific Execution Environment. Not part of the transport protocol +debugStack?: null | Error, +debugTask?: null | ConsoleTask,