From ebb22110f445d42222913f3bc4a8796a0e99c33c Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Mon, 30 Sep 2024 16:23:37 -0400 Subject: [PATCH 01/12] Outline Server Component Info in a special helper that uses console value serialization --- .../react-server/src/ReactFlightServer.js | 75 +++++++++++++------ 1 file changed, 54 insertions(+), 21 deletions(-) diff --git a/packages/react-server/src/ReactFlightServer.js b/packages/react-server/src/ReactFlightServer.js index 19c40c214b9..3a092e4e902 100644 --- a/packages/react-server/src/ReactFlightServer.js +++ b/packages/react-server/src/ReactFlightServer.js @@ -1155,7 +1155,7 @@ function renderFunctionComponent( // 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); + outlineDebugObject(request, (componentDebugInfo: any)); emitDebugChunk(request, componentDebugID, componentDebugInfo); // We've emitted the latest environment for this task so we track that. @@ -1582,6 +1582,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. + outlineDebugObject(request, (task.debugOwner: any)); + } + } const element = __DEV__ ? enableOwnerStacks ? [ @@ -2795,25 +2802,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( @@ -3265,6 +3253,51 @@ function emitDebugChunk( request.completedRegularChunks.push(processedChunk); } +function outlineDebugObject(request: Request, value: ReactClientObject): 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( + 'outlineDebugObject should never be called in production mode. This is a bug in React.', + ); + } + + if (request.writtenObjects.has(value)) { + // Already written + return; + } + + // 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}; + function replacer( + this: + | {+[key: string | number]: ReactClientValue} + | $ReadOnlyArray, + parentPropertyName: string, + value: ReactClientValue, + ): ReactJSONValue { + return renderConsoleValue( + request, + counter, + this, + parentPropertyName, + value, + ); + } + + request.pendingChunks++; + const id = request.nextChunkId++; + + // $FlowFixMe[incompatible-type] stringify can return null + const json: string = stringify(value, replacer); + const row = id.toString(16) + ':' + json + '\n'; + const processedChunk = stringToChunk(row); + request.completedRegularChunks.push(processedChunk); + + request.writtenObjects.set(value, serializeByValueID(id)); +} + function emitTypedArrayChunk( request: Request, id: number, @@ -3704,7 +3737,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]); + outlineDebugObject(request, (debugInfo[i]: any)); } emitDebugChunk(request, id, debugInfo[i]); } From f5bd8f0a136a817b4c1ab4c87b4bb8a757a33ef6 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Mon, 30 Sep 2024 16:32:52 -0400 Subject: [PATCH 02/12] Flip objectCount to objectLimit so that the limit can vary by caller --- .../react-server/src/ReactFlightServer.js | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/packages/react-server/src/ReactFlightServer.js b/packages/react-server/src/ReactFlightServer.js index 3a092e4e902..6509ce8895b 100644 --- a/packages/react-server/src/ReactFlightServer.js +++ b/packages/react-server/src/ReactFlightServer.js @@ -1155,6 +1155,7 @@ function renderFunctionComponent( // 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. + outlineDebugObject(request, (componentDebugInfo: any)); emitDebugChunk(request, componentDebugID, componentDebugInfo); @@ -2135,7 +2136,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. @@ -2146,7 +2147,7 @@ function serializeConsoleMap( function serializeConsoleSet( request: Request, - counter: {objectCount: number}, + counter: {objectLimit: number}, set: Set, ): string { // Like serializeMap but for renderConsoleValue. @@ -3229,7 +3230,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} @@ -3269,7 +3270,7 @@ function outlineDebugObject(request: Request, value: ReactClientObject): void { // 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} @@ -3355,7 +3356,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, @@ -3399,13 +3400,13 @@ function renderConsoleValue( } } - if (counter.objectCount > 500) { + 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); @@ -3635,7 +3636,7 @@ function renderConsoleValue( function outlineConsoleValue( request: Request, - counter: {objectCount: number}, + counter: {objectLimit: number}, model: ReactClientValue, ): number { if (!__DEV__) { @@ -3693,7 +3694,7 @@ function emitConsoleChunk( ); } - const counter = {objectCount: 0}; + const counter = {objectLimit: 500}; function replacer( this: | {+[key: string | number]: ReactClientValue} From b9284ee3976aa0eb8e9af6b888faf79aef7cf877 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Mon, 30 Sep 2024 16:34:52 -0400 Subject: [PATCH 03/12] Rename to specifically component info --- .../react-server/src/ReactFlightServer.js | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/packages/react-server/src/ReactFlightServer.js b/packages/react-server/src/ReactFlightServer.js index 6509ce8895b..17c89ec9d74 100644 --- a/packages/react-server/src/ReactFlightServer.js +++ b/packages/react-server/src/ReactFlightServer.js @@ -1156,7 +1156,7 @@ function renderFunctionComponent( // 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. - outlineDebugObject(request, (componentDebugInfo: any)); + outlineComponentInfo(request, componentDebugInfo); emitDebugChunk(request, componentDebugID, componentDebugInfo); // We've emitted the latest environment for this task so we track that. @@ -1587,7 +1587,7 @@ function renderClientElement( 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. - outlineDebugObject(request, (task.debugOwner: any)); + outlineComponentInfo(request, task.debugOwner); } } const element = __DEV__ @@ -3254,16 +3254,19 @@ function emitDebugChunk( request.completedRegularChunks.push(processedChunk); } -function outlineDebugObject(request: Request, value: ReactClientObject): void { +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( - 'outlineDebugObject should never be called in production mode. This is a bug in React.', + 'outlineComponentInfo should never be called in production mode. This is a bug in React.', ); } - if (request.writtenObjects.has(value)) { + if (request.writtenObjects.has(componentInfo)) { // Already written return; } @@ -3291,12 +3294,12 @@ function outlineDebugObject(request: Request, value: ReactClientObject): void { const id = request.nextChunkId++; // $FlowFixMe[incompatible-type] stringify can return null - const json: string = stringify(value, replacer); + const json: string = stringify(componentInfo, replacer); const row = id.toString(16) + ':' + json + '\n'; const processedChunk = stringToChunk(row); request.completedRegularChunks.push(processedChunk); - request.writtenObjects.set(value, serializeByValueID(id)); + request.writtenObjects.set(componentInfo, serializeByValueID(id)); } function emitTypedArrayChunk( @@ -3738,7 +3741,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. - outlineDebugObject(request, (debugInfo[i]: any)); + outlineComponentInfo(request, (debugInfo[i]: any)); } emitDebugChunk(request, id, debugInfo[i]); } From 3053087227676f4c0a356e1f2df00bdbfe21d172 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Mon, 30 Sep 2024 16:56:40 -0400 Subject: [PATCH 04/12] Always outline component info so we don't need to feature test it --- .../react-server/src/ReactFlightServer.js | 63 ++++++++----------- 1 file changed, 26 insertions(+), 37 deletions(-) diff --git a/packages/react-server/src/ReactFlightServer.js b/packages/react-server/src/ReactFlightServer.js index 17c89ec9d74..56ae4b37450 100644 --- a/packages/react-server/src/ReactFlightServer.js +++ b/packages/react-server/src/ReactFlightServer.js @@ -2271,23 +2271,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( @@ -3271,6 +3254,11 @@ function outlineComponentInfo( return; } + if (componentInfo.owner != null) { + // Ensure the owner is already outlined. + outlineComponentInfo(request, componentInfo.owner); + } + // 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: 500}; @@ -3293,8 +3281,23 @@ function outlineComponentInfo( request.pendingChunks++; const id = request.nextChunkId++; + // 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; + } + // $FlowFixMe[incompatible-type] stringify can return null - const json: string = stringify(componentInfo, replacer); + const json: string = stringify(componentDebugInfo, replacer); const row = id.toString(16) + ':' + json + '\n'; const processedChunk = stringToChunk(row); request.completedRegularChunks.push(processedChunk); @@ -3540,25 +3543,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; } @@ -3718,6 +3702,11 @@ function emitConsoleChunk( } } + // 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]; From 0752d74e9c8a3e9bd0a10127db60a99a14331424 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Mon, 30 Sep 2024 16:45:06 -0400 Subject: [PATCH 05/12] Limit the object count for outlined ReactComponentInfo to avoid writing very large props console.log:ed objects can have a higher limit since they are explicit where as props of Server Components is not something you can limit yourself. --- .../react-server/src/ReactFlightServer.js | 30 ++++++++++--------- 1 file changed, 16 insertions(+), 14 deletions(-) diff --git a/packages/react-server/src/ReactFlightServer.js b/packages/react-server/src/ReactFlightServer.js index 56ae4b37450..c2a62ce61f8 100644 --- a/packages/react-server/src/ReactFlightServer.js +++ b/packages/react-server/src/ReactFlightServer.js @@ -3259,9 +3259,16 @@ function outlineComponentInfo( 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: 500}; + const counter = {objectLimit}; function replacer( this: | {+[key: string | number]: ReactClientValue} @@ -3406,6 +3413,14 @@ function renderConsoleValue( } } + 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. @@ -3414,15 +3429,8 @@ function renderConsoleValue( 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; - } - const thenable: Thenable = (value: any); switch (thenable.status) { case 'fulfilled': { @@ -3456,12 +3464,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; } From 0dad2dfd0674cb86d0a0178eedc3f343c74866fa Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Mon, 30 Sep 2024 16:49:00 -0400 Subject: [PATCH 06/12] Emit props in ReactComponentInfo --- .../src/__tests__/ReactFlight-test.js | 33 +++++++++++++++++ .../react-server/src/ReactFlightServer.js | 37 ++++++++++++++----- packages/shared/ReactTypes.js | 1 + 3 files changed, 62 insertions(+), 9 deletions(-) diff --git a/packages/react-client/src/__tests__/ReactFlight-test.js b/packages/react-client/src/__tests__/ReactFlight-test.js index 27db069ef32..857ce99868d 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-server/src/ReactFlightServer.js b/packages/react-server/src/ReactFlightServer.js index c2a62ce61f8..657cbb3930f 100644 --- a/packages/react-server/src/ReactFlightServer.js +++ b/packages/react-server/src/ReactFlightServer.js @@ -1148,9 +1148,14 @@ 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 @@ -1710,6 +1715,7 @@ function renderElement( task.debugStack === null ? null : filterStackTrace(request, task.debugStack, 1), + props: props, debugStack: task.debugStack, debugTask: task.debugTask, }; @@ -3276,13 +3282,19 @@ function outlineComponentInfo( parentPropertyName: string, value: ReactClientValue, ): ReactJSONValue { - return renderConsoleValue( - request, - counter, - this, - parentPropertyName, - value, - ); + try { + return renderConsoleValue( + request, + counter, + this, + parentPropertyName, + value, + ); + } catch (x) { + return ( + 'Unknown Value: React could not send it from the server.\n' + x.message + ); + } } request.pendingChunks++; @@ -3302,6 +3314,9 @@ function outlineComponentInfo( // $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; // $FlowFixMe[incompatible-type] stringify can return null const json: string = stringify(componentDebugInfo, replacer); @@ -3652,7 +3667,9 @@ function outlineConsoleValue( value, ); } catch (x) { - return 'unknown value'; + return ( + 'Unknown Value: React could not send it from the server.\n' + x.message + ); } } @@ -3700,7 +3717,9 @@ function emitConsoleChunk( value, ); } catch (x) { - return 'unknown value'; + return ( + 'Unknown Value: React could not send it from the server.\n' + x.message + ); } } diff --git a/packages/shared/ReactTypes.js b/packages/shared/ReactTypes.js index 7d51fbe4725..54eccd5538d 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, From 6d083895c31b99e92bf29e665d15f4d26903e303 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Mon, 30 Sep 2024 17:46:42 -0400 Subject: [PATCH 07/12] Serialize elements in console value This is not strictly necessary but it ensure they get the right props etc --- .../react-server/src/ReactFlightServer.js | 30 +++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/packages/react-server/src/ReactFlightServer.js b/packages/react-server/src/ReactFlightServer.js index 657cbb3930f..f7fd3aaaa79 100644 --- a/packages/react-server/src/ReactFlightServer.js +++ b/packages/react-server/src/ReactFlightServer.js @@ -3444,6 +3444,36 @@ function renderConsoleValue( counter.objectLimit--; + switch ((value: any).$$typeof) { + case REACT_ELEMENT_TYPE: { + const element: ReactElement = (value: any); + + if (element._owner != null) { + outlineComponentInfo(request, element._owner); + } + + return enableOwnerStacks + ? [ + REACT_ELEMENT_TYPE, + element.type, + element.key, + element.props, + element._owner, + element._debugStack == null + ? null + : filterStackTrace(request, element._debugStack, 1), + element._store.validated, + ] + : [ + REACT_ELEMENT_TYPE, + element.type, + element.key, + element.props, + element._owner, + ]; + } + } + // $FlowFixMe[method-unbinding] if (typeof value.then === 'function') { const thenable: Thenable = (value: any); From 0a4ee9bbee456fbecbb86c69ea7b5652f62ccd82 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Mon, 30 Sep 2024 23:06:52 -0400 Subject: [PATCH 08/12] Better handle future references in debug info Still not great because it should really block listeners on the row itself so that we have all the debug info before resolving the promise. It also doesn't handle debug info in the right order. --- .../react-client/src/ReactFlightClient.js | 21 ++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/packages/react-client/src/ReactFlightClient.js b/packages/react-client/src/ReactFlightClient.js index 98a52c4ec4a..2f60ccddb4b 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. From dbed231ad27c495ce4b15495331a53de0f0c9f9f Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Mon, 30 Sep 2024 23:18:05 -0400 Subject: [PATCH 09/12] Fix tests --- .../src/__tests__/ReactFlightDOMBrowser-test.js | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) 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 0408a63fc51..882c0bbb019 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(); From 33aa9c5cc17beec9062f99b1b5f39eaa7854c075 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Tue, 1 Oct 2024 00:06:56 -0400 Subject: [PATCH 10/12] Outline stacks so they don't get cut off by the object limit --- .../react-server/src/ReactFlightServer.js | 79 +++++++------------ 1 file changed, 30 insertions(+), 49 deletions(-) diff --git a/packages/react-server/src/ReactFlightServer.js b/packages/react-server/src/ReactFlightServer.js index f7fd3aaaa79..5db03d62814 100644 --- a/packages/react-server/src/ReactFlightServer.js +++ b/packages/react-server/src/ReactFlightServer.js @@ -3275,30 +3275,6 @@ function outlineComponentInfo( // 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}; - function replacer( - this: - | {+[key: string | number]: ReactClientValue} - | $ReadOnlyArray, - parentPropertyName: string, - value: ReactClientValue, - ): ReactJSONValue { - try { - return renderConsoleValue( - request, - counter, - this, - parentPropertyName, - value, - ); - } catch (x) { - return ( - 'Unknown Value: React could not send it from the server.\n' + x.message - ); - } - } - - request.pendingChunks++; - const id = request.nextChunkId++; // We can't serialize the ConsoleTask/Error objects so we need to omit them before serializing. const componentDebugInfo: Omit< @@ -3318,12 +3294,7 @@ function outlineComponentInfo( // $FlowFixMe[cannot-write] componentDebugInfo.props = componentInfo.props; - // $FlowFixMe[incompatible-type] stringify can return null - const json: string = stringify(componentDebugInfo, replacer); - const row = id.toString(16) + ':' + json + '\n'; - const processedChunk = stringToChunk(row); - request.completedRegularChunks.push(processedChunk); - + const id = outlineConsoleValue(request, counter, componentDebugInfo); request.writtenObjects.set(componentInfo, serializeByValueID(id)); } @@ -3451,26 +3422,36 @@ function renderConsoleValue( 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 enableOwnerStacks - ? [ - REACT_ELEMENT_TYPE, - element.type, - element.key, - element.props, - element._owner, - element._debugStack == null - ? null - : filterStackTrace(request, element._debugStack, 1), - element._store.validated, - ] - : [ - REACT_ELEMENT_TYPE, - element.type, - element.key, - element.props, - element._owner, - ]; + return [ + REACT_ELEMENT_TYPE, + element.type, + element.key, + element.props, + element._owner, + ]; } } From 60dfd2f28e6a3cbda3948484c9e175fcdfaf9e98 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Tue, 1 Oct 2024 00:08:55 -0400 Subject: [PATCH 11/12] Expose to devtools --- packages/react-devtools-shared/src/backend/fiber/renderer.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/react-devtools-shared/src/backend/fiber/renderer.js b/packages/react-devtools-shared/src/backend/fiber/renderer.js index 4fac8839ae7..9732bf105ca 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); From a78d6212a7ba62800bd60747cff4c33887ae10d2 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Tue, 1 Oct 2024 01:25:42 -0400 Subject: [PATCH 12/12] Safely hydrate keys that throw --- .../react-devtools-shared/src/hydration.js | 58 ++++++++++++++++--- 1 file changed, 51 insertions(+), 7 deletions(-) diff --git a/packages/react-devtools-shared/src/hydration.js b/packages/react-devtools-shared/src/hydration.js index c5b78135e74..c21efe40a88 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,