From a03905ff7ff7f3a1529f71863ee47ca921c5339b Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Mon, 8 Jan 2024 21:06:59 -0500 Subject: [PATCH 1/7] attemptResolveElement -> renderElement Start unifying naming with Fizz. They use a slightly different code structure so it won't use the same recursion yet at least. --- packages/react-server/src/ReactFlightServer.js | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/react-server/src/ReactFlightServer.js b/packages/react-server/src/ReactFlightServer.js index 2ce2260a4852f..b9d55cae22437 100644 --- a/packages/react-server/src/ReactFlightServer.js +++ b/packages/react-server/src/ReactFlightServer.js @@ -504,7 +504,7 @@ function createLazyWrapperAroundWakeable(wakeable: Wakeable) { return lazyType; } -function attemptResolveElement( +function renderElement( request: Request, type: any, key: null | React$Key, @@ -574,7 +574,7 @@ function attemptResolveElement( const payload = type._payload; const init = type._init; const wrappedType = init(payload); - return attemptResolveElement( + return renderElement( request, wrappedType, key, @@ -589,7 +589,7 @@ function attemptResolveElement( return render(props, undefined); } case REACT_MEMO_TYPE: { - return attemptResolveElement( + return renderElement( request, type.type, key, @@ -1018,7 +1018,7 @@ function resolveModelToJSON( // TODO: Concatenate keys of parents onto children. const element: React$Element = (value: any); // Attempt to render the Server Component. - value = attemptResolveElement( + value = renderElement( request, element.type, element.key, @@ -1551,7 +1551,7 @@ function retryTask(request: Request, task: Task): void { // Doing this here lets us reuse this same task if the next component // also suspends. task.model = value; - value = attemptResolveElement( + value = renderElement( request, element.type, element.key, @@ -1576,7 +1576,7 @@ function retryTask(request: Request, task: Task): void { // TODO: Concatenate keys of parents onto children. const nextElement: React$Element = (value: any); task.model = value; - value = attemptResolveElement( + value = renderElement( request, nextElement.type, nextElement.key, From 7ed9706226ee71a2e185ce1ad702138b4b11d0af Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Tue, 23 Jan 2024 13:49:00 -0500 Subject: [PATCH 2/7] Move toJSON closure to one per task so that we can pass the task to it --- .../react-server/src/ReactFlightServer.js | 38 ++++++++++--------- 1 file changed, 20 insertions(+), 18 deletions(-) diff --git a/packages/react-server/src/ReactFlightServer.js b/packages/react-server/src/ReactFlightServer.js index b9d55cae22437..9348c3d16e4fb 100644 --- a/packages/react-server/src/ReactFlightServer.js +++ b/packages/react-server/src/ReactFlightServer.js @@ -180,6 +180,7 @@ type Task = { status: 0 | 1 | 3 | 4, model: ReactClientValue, ping: () => void, + toJSON: (key: string, value: ReactClientValue) => ReactJSONValue, context: ContextSnapshot, thenableState: ThenableState | null, }; @@ -212,7 +213,6 @@ export type Request = { taintCleanupQueue: Array, onError: (error: mixed) => ?string, onPostpone: (reason: string) => void, - toJSON: (key: string, value: ReactClientValue) => ReactJSONValue, }; const { @@ -311,10 +311,6 @@ export function createRequest( taintCleanupQueue: cleanupQueue, onError: onError === undefined ? defaultErrorHandler : onError, onPostpone: onPostpone === undefined ? defaultPostponeHandler : onPostpone, - // $FlowFixMe[missing-this-annot] - toJSON: function (key: string, value: ReactClientValue): ReactJSONValue { - return resolveModelToJSON(request, this, key, value); - }, }; request.pendingChunks++; const rootContext = createRootContext(context); @@ -654,6 +650,15 @@ function createTask( model, context, ping: () => pingTask(request, task), + toJSON: function ( + this: + | {+[key: string | number]: ReactClientValue} + | $ReadOnlyArray, + key: string, + value: ReactClientValue, + ): ReactJSONValue { + return renderModel(request, task, this, key, value); + }, thenableState: null, }; abortSet.add(task); @@ -911,8 +916,9 @@ let insideContextProps = null; let isInsideContextValue = false; let modelRoot: null | ReactClientValue = false; -function resolveModelToJSON( +function renderModel( request: Request, + task: Task, parent: | {+[key: string | number]: ReactClientValue} | $ReadOnlyArray, @@ -1508,17 +1514,7 @@ function emitProviderChunk( request.completedRegularChunks.push(processedChunk); } -function emitModelChunk( - request: Request, - id: number, - model: ReactClientValue, -): void { - // Track the root so we know that we have to emit this object even though it - // already has an ID. This is needed because we might see this object twice - // in the same toJSON if it is cyclic. - modelRoot = model; - // $FlowFixMe[incompatible-type] stringify can return null - const json: string = stringify(model, request.toJSON); +function emitModelChunk(request: Request, id: number, json: string): void { const row = id.toString(16) + ':' + json + '\n'; const processedChunk = stringToChunk(row); request.completedRegularChunks.push(processedChunk); @@ -1592,7 +1588,13 @@ function retryTask(request: Request, task: Task): void { request.writtenObjects.set(value, task.id); } - emitModelChunk(request, task.id, value); + // Track the root so we know that we have to emit this object even though it + // already has an ID. This is needed because we might see this object twice + // in the same toJSON if it is cyclic. + modelRoot = value; + // $FlowFixMe[incompatible-type] stringify can return null + const json: string = stringify(value, task.toJSON); + emitModelChunk(request, task.id, json); request.abortableTasks.delete(task); task.status = COMPLETED; } catch (thrownValue) { From 15f377f3aedbdde81634e9df852f3b4ae7ed9f15 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Tue, 23 Jan 2024 18:06:35 -0500 Subject: [PATCH 3/7] Switch to recursive renderModel approach Unlike Fizz, the split between renderModel/renderModelDestructive isn't used because we don't bother catching suspense from anything but Lazy which is already handling this separately. In Fizz only Suspense boundaries can catch errors but in Flight any Element can catch errors because it can be replaced by a Lazy. --- packages/react-server/src/ReactFizzServer.js | 6 +- .../react-server/src/ReactFlightServer.js | 334 ++++++++++-------- 2 files changed, 189 insertions(+), 151 deletions(-) diff --git a/packages/react-server/src/ReactFizzServer.js b/packages/react-server/src/ReactFizzServer.js index 16357649bbb3d..f77b455a2fa82 100644 --- a/packages/react-server/src/ReactFizzServer.js +++ b/packages/react-server/src/ReactFizzServer.js @@ -2201,8 +2201,12 @@ function renderNodeDestructive( task.node = node; task.childIndex = childIndex; + if (node === null) { + return; + } + // Handle object types - if (typeof node === 'object' && node !== null) { + if (typeof node === 'object') { switch ((node: any).$$typeof) { case REACT_ELEMENT_TYPE: { const element: any = node; diff --git a/packages/react-server/src/ReactFlightServer.js b/packages/react-server/src/ReactFlightServer.js index 9348c3d16e4fb..ed36a27996307 100644 --- a/packages/react-server/src/ReactFlightServer.js +++ b/packages/react-server/src/ReactFlightServer.js @@ -657,7 +657,57 @@ function createTask( key: string, value: ReactClientValue, ): ReactJSONValue { - return renderModel(request, task, this, key, value); + const parent = this; + // Make sure that `parent[key]` wasn't JSONified before `value` was passed to us + if (__DEV__) { + // $FlowFixMe[incompatible-use] + const originalValue = parent[key]; + if ( + typeof originalValue === 'object' && + originalValue !== value && + !(originalValue instanceof Date) + ) { + if (objectName(originalValue) !== 'Object') { + const jsxParentType = jsxChildrenParents.get(parent); + if (typeof jsxParentType === 'string') { + console.error( + '%s objects cannot be rendered as text children. Try formatting it using toString().%s', + objectName(originalValue), + describeObjectForErrorMessage(parent, key), + ); + } else { + console.error( + 'Only plain objects can be passed to Client Components from Server Components. ' + + '%s objects are not supported.%s', + objectName(originalValue), + describeObjectForErrorMessage(parent, key), + ); + } + } else { + console.error( + 'Only plain objects can be passed to Client Components from Server Components. ' + + 'Objects with toJSON methods are not supported. Convert it manually ' + + 'to a simple value before passing it to props.%s', + describeObjectForErrorMessage(parent, key), + ); + } + } + + if ( + enableServerContext && + parent[0] === REACT_ELEMENT_TYPE && + parent[1] && + (parent[1]: any).$$typeof === REACT_PROVIDER_TYPE && + key === '3' + ) { + insideContextProps = value; + } else if (insideContextProps === parent && key === 'value') { + isInsideContextValue = true; + } else if (insideContextProps === parent && key === 'children') { + isInsideContextValue = false; + } + } + return renderModel(request, task, parent, key, value); }, thenableState: null, }; @@ -916,6 +966,59 @@ let insideContextProps = null; let isInsideContextValue = false; let modelRoot: null | ReactClientValue = false; +// A thrown value whether Suspense or Error needs to be serialized into a new chunk. +// This creates a new chunk and returns the new ID which can be either a direct or +// indirect reference. +function serializeCaughtValue( + request: Request, + task: Task, + thrownValue: mixed, +): number { + const x = + thrownValue === SuspenseException + ? // This is a special type of exception used for Suspense. For historical + // reasons, the rest of the Suspense implementation expects the thrown + // value to be a thenable, because before `use` existed that was the + // (unstable) API for suspending. This implementation detail can change + // later, once we deprecate the old API in favor of `use`. + getSuspendedThenable() + : thrownValue; + if (typeof x === 'object' && x !== null) { + // $FlowFixMe[method-unbinding] + if (typeof x.then === 'function') { + // Something suspended, we'll need to create a new task and resolve it later. + request.pendingChunks++; + const newTask = createTask( + request, + task.model, + getActiveContext(), + request.abortableTasks, + ); + const ping = newTask.ping; + (x: any).then(ping, ping); + newTask.thenableState = getThenableStateAfterSuspending(); + return newTask.id; + } else if (enablePostpone && x.$$typeof === REACT_POSTPONE_TYPE) { + // Something postponed. We'll still send everything we have up until this point. + // We'll replace this element with a lazy reference that postpones on the client. + const postponeInstance: Postpone = (x: any); + request.pendingChunks++; + const postponeId = request.nextChunkId++; + logPostpone(request, postponeInstance.message); + emitPostponeChunk(request, postponeId, postponeInstance); + return postponeId; + } + } + // Something errored. We'll still send everything we have up until this point. + // We'll replace this element with a lazy reference that throws on the client + // once it gets rendered. + request.pendingChunks++; + const errorId = request.nextChunkId++; + const digest = logRecoverableError(request, x); + emitErrorChunk(request, errorId, digest, x); + return errorId; +} + function renderModel( request: Request, task: Task, @@ -925,106 +1028,72 @@ function renderModel( key: string, value: ReactClientValue, ): ReactJSONValue { - // Make sure that `parent[key]` wasn't JSONified before `value` was passed to us - if (__DEV__) { - // $FlowFixMe[incompatible-use] - const originalValue = parent[key]; - if ( - typeof originalValue === 'object' && - originalValue !== value && - !(originalValue instanceof Date) - ) { - if (objectName(originalValue) !== 'Object') { - const jsxParentType = jsxChildrenParents.get(parent); - if (typeof jsxParentType === 'string') { - console.error( - '%s objects cannot be rendered as text children. Try formatting it using toString().%s', - objectName(originalValue), - describeObjectForErrorMessage(parent, key), - ); - } else { - console.error( - 'Only plain objects can be passed to Client Components from Server Components. ' + - '%s objects are not supported.%s', - objectName(originalValue), - describeObjectForErrorMessage(parent, key), - ); - } - } else { - console.error( - 'Only plain objects can be passed to Client Components from Server Components. ' + - 'Objects with toJSON methods are not supported. Convert it manually ' + - 'to a simple value before passing it to props.%s', - describeObjectForErrorMessage(parent, key), - ); - } - } - } + // This function only exists for parity with Fizz, but it's not currently used. + // We could add a catch here which returns serializeByValueID(serializeCaughtValue()). + // If it's an error that doesn't really help but if something suspends we can still + // the surrounding tree early and still reconstruct it as a single tree on the client. + // However, the only things that really should suspend are Components or Lazy which + // already serialize as lazy references instead. In theory though a Proxy could also + // suspend or error when accessed and currently that suspends the parent row or Lazy. + return renderModelDestructive(request, task, parent, key, value); +} - // Special Symbols - switch (value) { - case REACT_ELEMENT_TYPE: - return '$'; - } +function renderModelDestructive( + request: Request, + task: Task, + parent: + | {+[key: string | number]: ReactClientValue} + | $ReadOnlyArray, + key: string, + value: ReactClientValue, +): ReactJSONValue { + // Set the currently rendering model + task.model = value; - if (__DEV__) { - if ( - enableServerContext && - parent[0] === REACT_ELEMENT_TYPE && - parent[1] && - (parent[1]: any).$$typeof === REACT_PROVIDER_TYPE && - key === '3' - ) { - insideContextProps = value; - } else if (insideContextProps === parent && key === 'value') { - isInsideContextValue = true; - } else if (insideContextProps === parent && key === 'children') { - isInsideContextValue = false; - } + // Special Symbol, that's very common. + if (value === REACT_ELEMENT_TYPE) { + return '$'; } - // Resolve Server Components. - while ( - typeof value === 'object' && - value !== null && - ((value: any).$$typeof === REACT_ELEMENT_TYPE || - (value: any).$$typeof === REACT_LAZY_TYPE) - ) { - if (__DEV__) { - if (enableServerContext && isInsideContextValue) { - console.error('React elements are not allowed in ServerContext'); - } - } + if (value === null) { + return null; + } - try { - switch ((value: any).$$typeof) { - case REACT_ELEMENT_TYPE: { - const writtenObjects = request.writtenObjects; - const existingId = writtenObjects.get(value); - if (existingId !== undefined) { - if (existingId === -1) { - // Seen but not yet outlined. - const newId = outlineModel(request, value); - return serializeByValueID(newId); - } else if (modelRoot === value) { - // This is the ID we're currently emitting so we need to write it - // once but if we discover it again, we refer to it by id. - modelRoot = null; - } else { - // We've already emitted this as an outlined object, so we can - // just refer to that by its existing ID. - return serializeByValueID(existingId); - } + if (typeof value === 'object') { + switch ((value: any).$$typeof) { + case REACT_ELEMENT_TYPE: { + if (__DEV__) { + if (enableServerContext && isInsideContextValue) { + console.error('React elements are not allowed in ServerContext'); + } + } + const writtenObjects = request.writtenObjects; + const existingId = writtenObjects.get(value); + if (existingId !== undefined) { + if (existingId === -1) { + // Seen but not yet outlined. + const newId = outlineModel(request, value); + return serializeByValueID(newId); + } else if (modelRoot === value) { + // This is the ID we're currently emitting so we need to write it + // once but if we discover it again, we refer to it by id. + modelRoot = null; } else { - // This is the first time we've seen this object. We may never see it again - // so we'll inline it. Mark it as seen. If we see it again, we'll outline. - writtenObjects.set(value, -1); + // We've already emitted this as an outlined object, so we can + // just refer to that by its existing ID. + return serializeByValueID(existingId); } + } else { + // This is the first time we've seen this object. We may never see it again + // so we'll inline it. Mark it as seen. If we see it again, we'll outline. + writtenObjects.set(value, -1); + } + try { // TODO: Concatenate keys of parents onto children. const element: React$Element = (value: any); // Attempt to render the Server Component. - value = renderElement( + const result = renderElement( request, element.type, element.key, @@ -1032,76 +1101,39 @@ function renderModel( element.props, null, ); - break; + return renderModelDestructive(request, task, parent, key, result); + } catch (x) { + return serializeLazyID(serializeCaughtValue(request, task, x)); } - case REACT_LAZY_TYPE: { + } + case REACT_LAZY_TYPE: { + try { const payload = (value: any)._payload; const init = (value: any)._init; - value = init(payload); - break; - } - } - } catch (thrownValue) { - const x = - thrownValue === SuspenseException - ? // This is a special type of exception used for Suspense. For historical - // reasons, the rest of the Suspense implementation expects the thrown - // value to be a thenable, because before `use` existed that was the - // (unstable) API for suspending. This implementation detail can change - // later, once we deprecate the old API in favor of `use`. - getSuspendedThenable() - : thrownValue; - if (typeof x === 'object' && x !== null) { - // $FlowFixMe[method-unbinding] - if (typeof x.then === 'function') { - // Something suspended, we'll need to create a new task and resolve it later. - request.pendingChunks++; - const newTask = createTask( + const resolvedModel = init(payload); + return renderModelDestructive( request, - value, - getActiveContext(), - request.abortableTasks, + task, + parent, + key, + resolvedModel, ); - const ping = newTask.ping; - x.then(ping, ping); - newTask.thenableState = getThenableStateAfterSuspending(); - return serializeLazyID(newTask.id); - } else if (enablePostpone && x.$$typeof === REACT_POSTPONE_TYPE) { - // Something postponed. We'll still send everything we have up until this point. - // We'll replace this element with a lazy reference that postpones on the client. - const postponeInstance: Postpone = (x: any); - request.pendingChunks++; - const postponeId = request.nextChunkId++; - logPostpone(request, postponeInstance.message); - emitPostponeChunk(request, postponeId, postponeInstance); - return serializeLazyID(postponeId); + } catch (x) { + return serializeLazyID(serializeCaughtValue(request, task, x)); } } - // Something errored. We'll still send everything we have up until this point. - // We'll replace this element with a lazy reference that throws on the client - // once it gets rendered. - request.pendingChunks++; - const errorId = request.nextChunkId++; - const digest = logRecoverableError(request, x); - emitErrorChunk(request, errorId, digest, x); - return serializeLazyID(errorId); } - } - if (value === null) { - return null; - } + if (isClientReference(value)) { + return serializeClientReference(request, parent, key, (value: any)); + } - if (typeof value === 'object') { if (enableTaint) { const tainted = TaintRegistryObjects.get(value); if (tainted !== undefined) { throwTaintViolation(tainted); } } - if (isClientReference(value)) { - return serializeClientReference(request, parent, key, (value: any)); - } const writtenObjects = request.writtenObjects; const existingId = writtenObjects.get(value); @@ -1318,18 +1350,20 @@ function renderModel( } if (typeof value === 'function') { - if (enableTaint) { - const tainted = TaintRegistryObjects.get(value); - if (tainted !== undefined) { - throwTaintViolation(tainted); - } - } if (isClientReference(value)) { return serializeClientReference(request, parent, key, (value: any)); } if (isServerReference(value)) { return serializeServerReference(request, parent, key, (value: any)); } + + if (enableTaint) { + const tainted = TaintRegistryObjects.get(value); + if (tainted !== undefined) { + throwTaintViolation(tainted); + } + } + if (/^on[A-Z]/.test(key)) { throw new Error( 'Event handlers cannot be passed to Client Component props.' + From 9e50bc851a5b9afbc7cdbd0a263c44c39cd6806a Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Tue, 23 Jan 2024 20:44:23 -0500 Subject: [PATCH 4/7] Retry a task using the recursive loop This will render into a JSON value but will not be recursively resolved. If it's a terminal JSON value shouldn't double encode it. --- .../react-server/src/ReactFlightServer.js | 194 +++++++++--------- 1 file changed, 95 insertions(+), 99 deletions(-) diff --git a/packages/react-server/src/ReactFlightServer.js b/packages/react-server/src/ReactFlightServer.js index ed36a27996307..9a91836f88564 100644 --- a/packages/react-server/src/ReactFlightServer.js +++ b/packages/react-server/src/ReactFlightServer.js @@ -644,6 +644,10 @@ function createTask( abortSet: Set, ): Task { const id = request.nextChunkId++; + if (typeof model === 'object' && model !== null) { + // Register this model as having the ID we're about to write. + request.writtenObjects.set(model, id); + } const task: Task = { id, status: PENDING, @@ -654,14 +658,14 @@ function createTask( this: | {+[key: string | number]: ReactClientValue} | $ReadOnlyArray, - key: string, + parentPropertyName: string, value: ReactClientValue, ): ReactJSONValue { const parent = this; - // Make sure that `parent[key]` wasn't JSONified before `value` was passed to us + // Make sure that `parent[parentPropertyName]` wasn't JSONified before `value` was passed to us if (__DEV__) { // $FlowFixMe[incompatible-use] - const originalValue = parent[key]; + const originalValue = parent[parentPropertyName]; if ( typeof originalValue === 'object' && originalValue !== value && @@ -673,14 +677,14 @@ function createTask( console.error( '%s objects cannot be rendered as text children. Try formatting it using toString().%s', objectName(originalValue), - describeObjectForErrorMessage(parent, key), + describeObjectForErrorMessage(parent, parentPropertyName), ); } else { console.error( 'Only plain objects can be passed to Client Components from Server Components. ' + '%s objects are not supported.%s', objectName(originalValue), - describeObjectForErrorMessage(parent, key), + describeObjectForErrorMessage(parent, parentPropertyName), ); } } else { @@ -688,7 +692,7 @@ function createTask( 'Only plain objects can be passed to Client Components from Server Components. ' + 'Objects with toJSON methods are not supported. Convert it manually ' + 'to a simple value before passing it to props.%s', - describeObjectForErrorMessage(parent, key), + describeObjectForErrorMessage(parent, parentPropertyName), ); } } @@ -698,16 +702,22 @@ function createTask( parent[0] === REACT_ELEMENT_TYPE && parent[1] && (parent[1]: any).$$typeof === REACT_PROVIDER_TYPE && - key === '3' + parentPropertyName === '3' ) { insideContextProps = value; - } else if (insideContextProps === parent && key === 'value') { + } else if ( + insideContextProps === parent && + parentPropertyName === 'value' + ) { isInsideContextValue = true; - } else if (insideContextProps === parent && key === 'children') { + } else if ( + insideContextProps === parent && + parentPropertyName === 'children' + ) { isInsideContextValue = false; } } - return renderModel(request, task, parent, key, value); + return renderModel(request, task, parent, parentPropertyName, value); }, thenableState: null, }; @@ -788,9 +798,9 @@ function encodeReferenceChunk( function serializeClientReference( request: Request, parent: - | {+[key: string | number]: ReactClientValue} + | {+[propertyName: string | number]: ReactClientValue} | $ReadOnlyArray, - key: string, + parentPropertyName: string, clientReference: ClientReference, ): string { const clientReferenceKey: ClientReferenceKey = @@ -798,7 +808,7 @@ function serializeClientReference( const writtenClientReferences = request.writtenClientReferences; const existingId = writtenClientReferences.get(clientReferenceKey); if (existingId !== undefined) { - if (parent[0] === REACT_ELEMENT_TYPE && key === '1') { + if (parent[0] === REACT_ELEMENT_TYPE && parentPropertyName === '1') { // If we're encoding the "type" of an element, we can refer // to that by a lazy reference instead of directly since React // knows how to deal with lazy values. This lets us suspend @@ -815,7 +825,7 @@ function serializeClientReference( const importId = request.nextChunkId++; emitImportChunk(request, importId, clientReferenceMetadata); writtenClientReferences.set(clientReferenceKey, importId); - if (parent[0] === REACT_ELEMENT_TYPE && key === '1') { + if (parent[0] === REACT_ELEMENT_TYPE && parentPropertyName === '1') { // If we're encoding the "type" of an element, we can refer // to that by a lazy reference instead of directly since React // knows how to deal with lazy values. This lets us suspend @@ -833,7 +843,7 @@ function serializeClientReference( } } -function outlineModel(request: Request, value: any): number { +function outlineModel(request: Request, value: ReactClientValue): number { request.pendingChunks++; const newTask = createTask( request, @@ -847,10 +857,6 @@ function outlineModel(request: Request, value: any): number { function serializeServerReference( request: Request, - parent: - | {+[key: string | number]: ReactClientValue} - | $ReadOnlyArray, - key: string, serverReference: ServerReference, ): string { const writtenServerReferences = request.writtenServerReferences; @@ -1035,17 +1041,18 @@ function renderModel( // However, the only things that really should suspend are Components or Lazy which // already serialize as lazy references instead. In theory though a Proxy could also // suspend or error when accessed and currently that suspends the parent row or Lazy. - return renderModelDestructive(request, task, parent, key, value); + return renderModelDestructive(request, task, parent, key, value, null); } function renderModelDestructive( request: Request, task: Task, parent: - | {+[key: string | number]: ReactClientValue} + | {+[propertyName: string | number]: ReactClientValue} | $ReadOnlyArray, - key: string, + parentPropertyName: string, value: ReactClientValue, + prevThenableState: ThenableState | null, ): ReactJSONValue { // Set the currently rendering model task.model = value; @@ -1099,9 +1106,16 @@ function renderModelDestructive( element.key, element.ref, element.props, + prevThenableState, + ); + return renderModelDestructive( + request, + task, + emptyRoot, + '', + result, null, ); - return renderModelDestructive(request, task, parent, key, result); } catch (x) { return serializeLazyID(serializeCaughtValue(request, task, x)); } @@ -1114,9 +1128,10 @@ function renderModelDestructive( return renderModelDestructive( request, task, - parent, - key, + emptyRoot, + '', resolvedModel, + null, ); } catch (x) { return serializeLazyID(serializeCaughtValue(request, task, x)); @@ -1125,7 +1140,12 @@ function renderModelDestructive( } if (isClientReference(value)) { - return serializeClientReference(request, parent, key, (value: any)); + return serializeClientReference( + request, + parent, + parentPropertyName, + (value: any), + ); } if (enableTaint) { @@ -1161,7 +1181,7 @@ function renderModelDestructive( const providerKey = ((value: any): ReactProviderType)._context ._globalName; const writtenProviders = request.writtenProviders; - let providerId = writtenProviders.get(key); + let providerId = writtenProviders.get(providerKey); if (providerId === undefined) { request.pendingChunks++; providerId = request.nextChunkId++; @@ -1287,13 +1307,13 @@ function renderModelDestructive( 'Only plain objects can be passed to Client Components from Server Components. ' + '%s objects are not supported.%s', objectName(value), - describeObjectForErrorMessage(parent, key), + describeObjectForErrorMessage(parent, parentPropertyName), ); } else if (!isSimpleObject(value)) { console.error( 'Only plain objects can be passed to Client Components from Server Components. ' + 'Classes or other objects with methods are not supported.%s', - describeObjectForErrorMessage(parent, key), + describeObjectForErrorMessage(parent, parentPropertyName), ); } else if (Object.getOwnPropertySymbols) { const symbols = Object.getOwnPropertySymbols(value); @@ -1302,7 +1322,7 @@ function renderModelDestructive( 'Only plain objects can be passed to Client Components from Server Components. ' + 'Objects with symbol properties like %s are not supported.%s', symbols[0].description, - describeObjectForErrorMessage(parent, key), + describeObjectForErrorMessage(parent, parentPropertyName), ); } } @@ -1323,7 +1343,7 @@ function renderModelDestructive( if (value[value.length - 1] === 'Z') { // Possibly a Date, whose toJSON automatically calls toISOString // $FlowFixMe[incompatible-use] - const originalValue = parent[key]; + const originalValue = parent[parentPropertyName]; if (originalValue instanceof Date) { return serializeDateFromDateJSON(value); } @@ -1351,10 +1371,15 @@ function renderModelDestructive( if (typeof value === 'function') { if (isClientReference(value)) { - return serializeClientReference(request, parent, key, (value: any)); + return serializeClientReference( + request, + parent, + parentPropertyName, + (value: any), + ); } if (isServerReference(value)) { - return serializeServerReference(request, parent, key, (value: any)); + return serializeServerReference(request, (value: any)); } if (enableTaint) { @@ -1364,17 +1389,17 @@ function renderModelDestructive( } } - if (/^on[A-Z]/.test(key)) { + if (/^on[A-Z]/.test(parentPropertyName)) { throw new Error( 'Event handlers cannot be passed to Client Component props.' + - describeObjectForErrorMessage(parent, key) + + describeObjectForErrorMessage(parent, parentPropertyName) + '\nIf you need interactivity, consider converting part of this to a Client Component.', ); } else { throw new Error( 'Functions cannot be passed directly to Client Components ' + 'unless you explicitly expose it by marking it with "use server".' + - describeObjectForErrorMessage(parent, key), + describeObjectForErrorMessage(parent, parentPropertyName), ); } } @@ -1395,7 +1420,7 @@ function renderModelDestructive( // $FlowFixMe[incompatible-type] `description` might be undefined value.description }) cannot be found among global symbols.` + - describeObjectForErrorMessage(parent, key), + describeObjectForErrorMessage(parent, parentPropertyName), ); } @@ -1418,7 +1443,7 @@ function renderModelDestructive( throw new Error( `Type ${typeof value} is not supported in Client Component props.` + - describeObjectForErrorMessage(parent, key), + describeObjectForErrorMessage(parent, parentPropertyName), ); } @@ -1554,6 +1579,8 @@ function emitModelChunk(request: Request, id: number, json: string): void { request.completedRegularChunks.push(processedChunk); } +const emptyRoot = {}; + function retryTask(request: Request, task: Task): void { if (task.status !== PENDING) { // We completed this by other means before we had a chance to retry it. @@ -1562,73 +1589,42 @@ function retryTask(request: Request, task: Task): void { switchContext(task.context); try { - let value = task.model; - if ( - typeof value === 'object' && - value !== null && - (value: any).$$typeof === REACT_ELEMENT_TYPE - ) { - request.writtenObjects.set(value, task.id); - - // TODO: Concatenate keys of parents onto children. - const element: React$Element = (value: any); - - // When retrying a component, reuse the thenableState from the - // previous attempt. - const prevThenableState = task.thenableState; - - // Attempt to render the Server Component. - // Doing this here lets us reuse this same task if the next component - // also suspends. - task.model = value; - value = renderElement( - request, - element.type, - element.key, - element.ref, - element.props, - prevThenableState, - ); - - // Successfully finished this component. We're going to keep rendering - // using the same task, but we reset its thenable state before continuing. - task.thenableState = null; - - // Keep rendering and reuse the same task. This inner loop is separate - // from the render above because we don't need to reset the thenable state - // until the next time something suspends and retries. - while ( - typeof value === 'object' && - value !== null && - (value: any).$$typeof === REACT_ELEMENT_TYPE - ) { - request.writtenObjects.set(value, task.id); - // TODO: Concatenate keys of parents onto children. - const nextElement: React$Element = (value: any); - task.model = value; - value = renderElement( - request, - nextElement.type, - nextElement.key, - nextElement.ref, - nextElement.props, - null, - ); - } - } - - // Track that this object is outlined and has an id. - if (typeof value === 'object' && value !== null) { - request.writtenObjects.set(value, task.id); - } + // Reset the task's thenable state before continuing, so that if a later + // component suspends we can reuse the same task object. If the same + // component suspends again, the thenable state will be restored. + const prevThenableState = task.thenableState; + task.thenableState = null; // Track the root so we know that we have to emit this object even though it // already has an ID. This is needed because we might see this object twice // in the same toJSON if it is cyclic. - modelRoot = value; + modelRoot = task.model; + + // We call the destructive form that mutates this task. That way if something + // suspends again, we can reuse the same task instead of spawning a new one. + const resolvedModel = renderModelDestructive( + request, + task, + emptyRoot, + '', + task.model, + prevThenableState, + ); + + // Track the root again for the resolved object. + modelRoot = resolvedModel; + + // If the value is a string, it means it's a terminal value adn we already escaped it + // We don't need to escape it again so it's not passed the toJSON replacer. + // Object might contain unresolved values like additional elements. + // This is simulating what the JSON loop would do if this was part of it. // $FlowFixMe[incompatible-type] stringify can return null - const json: string = stringify(value, task.toJSON); + const json: string = + typeof resolvedModel === 'string' + ? stringify(resolvedModel) + : stringify(resolvedModel, task.toJSON); emitModelChunk(request, task.id, json); + request.abortableTasks.delete(task); task.status = COMPLETED; } catch (thrownValue) { From eabf7e4369761f246e88e27a19b92a3d135b7b2c Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Tue, 23 Jan 2024 23:23:33 -0500 Subject: [PATCH 5/7] We need to catch in renderModel after all Otherwise resuspending can't be handled by retryTask. Test that a sequence of server components is able to reuse the row --- .../src/__tests__/ReactFlightDOMEdge-test.js | 14 ++ .../react-server/src/ReactFlightServer.js | 196 +++++++++--------- 2 files changed, 111 insertions(+), 99 deletions(-) diff --git a/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOMEdge-test.js b/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOMEdge-test.js index 1cbac757430f1..5d966a16ded59 100644 --- a/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOMEdge-test.js +++ b/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOMEdge-test.js @@ -242,6 +242,20 @@ describe('ReactFlightDOMEdge', () => { expect(result).toEqual(resolvedChildren); }); + it('should execute repeated server components in a compact form', async () => { + async function ServerComponent({recurse}) { + if (recurse > 0) { + return ; + } + return
Fin
; + } + const stream = ReactServerDOMServer.renderToReadableStream( + , + ); + const serializedContent = await readResult(stream); + expect(serializedContent.length).toBeLessThan(150); + }); + // @gate enableBinaryFlight it('should be able to serialize any kind of typed array', async () => { const buffer = new Uint8Array([ diff --git a/packages/react-server/src/ReactFlightServer.js b/packages/react-server/src/ReactFlightServer.js index 9a91836f88564..ebb0e24e39c4e 100644 --- a/packages/react-server/src/ReactFlightServer.js +++ b/packages/react-server/src/ReactFlightServer.js @@ -972,59 +972,6 @@ let insideContextProps = null; let isInsideContextValue = false; let modelRoot: null | ReactClientValue = false; -// A thrown value whether Suspense or Error needs to be serialized into a new chunk. -// This creates a new chunk and returns the new ID which can be either a direct or -// indirect reference. -function serializeCaughtValue( - request: Request, - task: Task, - thrownValue: mixed, -): number { - const x = - thrownValue === SuspenseException - ? // This is a special type of exception used for Suspense. For historical - // reasons, the rest of the Suspense implementation expects the thrown - // value to be a thenable, because before `use` existed that was the - // (unstable) API for suspending. This implementation detail can change - // later, once we deprecate the old API in favor of `use`. - getSuspendedThenable() - : thrownValue; - if (typeof x === 'object' && x !== null) { - // $FlowFixMe[method-unbinding] - if (typeof x.then === 'function') { - // Something suspended, we'll need to create a new task and resolve it later. - request.pendingChunks++; - const newTask = createTask( - request, - task.model, - getActiveContext(), - request.abortableTasks, - ); - const ping = newTask.ping; - (x: any).then(ping, ping); - newTask.thenableState = getThenableStateAfterSuspending(); - return newTask.id; - } else if (enablePostpone && x.$$typeof === REACT_POSTPONE_TYPE) { - // Something postponed. We'll still send everything we have up until this point. - // We'll replace this element with a lazy reference that postpones on the client. - const postponeInstance: Postpone = (x: any); - request.pendingChunks++; - const postponeId = request.nextChunkId++; - logPostpone(request, postponeInstance.message); - emitPostponeChunk(request, postponeId, postponeInstance); - return postponeId; - } - } - // Something errored. We'll still send everything we have up until this point. - // We'll replace this element with a lazy reference that throws on the client - // once it gets rendered. - request.pendingChunks++; - const errorId = request.nextChunkId++; - const digest = logRecoverableError(request, x); - emitErrorChunk(request, errorId, digest, x); - return errorId; -} - function renderModel( request: Request, task: Task, @@ -1034,14 +981,73 @@ function renderModel( key: string, value: ReactClientValue, ): ReactJSONValue { - // This function only exists for parity with Fizz, but it's not currently used. - // We could add a catch here which returns serializeByValueID(serializeCaughtValue()). - // If it's an error that doesn't really help but if something suspends we can still - // the surrounding tree early and still reconstruct it as a single tree on the client. - // However, the only things that really should suspend are Components or Lazy which - // already serialize as lazy references instead. In theory though a Proxy could also - // suspend or error when accessed and currently that suspends the parent row or Lazy. - return renderModelDestructive(request, task, parent, key, value, null); + try { + return renderModelDestructive(request, task, parent, key, value, null); + } catch (thrownValue) { + const x = + thrownValue === SuspenseException + ? // This is a special type of exception used for Suspense. For historical + // reasons, the rest of the Suspense implementation expects the thrown + // value to be a thenable, because before `use` existed that was the + // (unstable) API for suspending. This implementation detail can change + // later, once we deprecate the old API in favor of `use`. + getSuspendedThenable() + : thrownValue; + // If the suspended/errored value was an element or lazy it can be reduced + // to a lazy reference, so that it doesn't error the parent. + const model = task.model; + const wasReactNode = + typeof model === 'object' && + model !== null && + ((model: any).$$typeof === REACT_ELEMENT_TYPE || + (model: any).$$typeof === REACT_LAZY_TYPE); + if (typeof x === 'object' && x !== null) { + // $FlowFixMe[method-unbinding] + if (typeof x.then === 'function') { + // Something suspended, we'll need to create a new task and resolve it later. + request.pendingChunks++; + const newTask = createTask( + request, + task.model, + getActiveContext(), + request.abortableTasks, + ); + const ping = newTask.ping; + (x: any).then(ping, ping); + newTask.thenableState = getThenableStateAfterSuspending(); + if (wasReactNode) { + return serializeLazyID(newTask.id); + } + return serializeByValueID(newTask.id); + } else if (enablePostpone && x.$$typeof === REACT_POSTPONE_TYPE) { + // Something postponed. We'll still send everything we have up until this point. + // We'll replace this element with a lazy reference that postpones on the client. + const postponeInstance: Postpone = (x: any); + request.pendingChunks++; + const postponeId = request.nextChunkId++; + logPostpone(request, postponeInstance.message); + emitPostponeChunk(request, postponeId, postponeInstance); + if (wasReactNode) { + return serializeLazyID(postponeId); + } + return serializeByValueID(postponeId); + } + } + if (wasReactNode) { + // Something errored. We'll still send everything we have up until this point. + // We'll replace this element with a lazy reference that throws on the client + // once it gets rendered. + request.pendingChunks++; + const errorId = request.nextChunkId++; + const digest = logRecoverableError(request, x); + emitErrorChunk(request, errorId, digest, x); + return serializeLazyID(errorId); + } + // Something errored but it was not in a React Node. There's no need to serialize + // it by value because it'll just error the whole parent row anyway so we can + // just stop any siblings and error the whole parent row. + throw x; + } } function renderModelDestructive( @@ -1096,46 +1102,38 @@ function renderModelDestructive( writtenObjects.set(value, -1); } - try { - // TODO: Concatenate keys of parents onto children. - const element: React$Element = (value: any); - // Attempt to render the Server Component. - const result = renderElement( - request, - element.type, - element.key, - element.ref, - element.props, - prevThenableState, - ); - return renderModelDestructive( - request, - task, - emptyRoot, - '', - result, - null, - ); - } catch (x) { - return serializeLazyID(serializeCaughtValue(request, task, x)); - } + // TODO: Concatenate keys of parents onto children. + const element: React$Element = (value: any); + // Attempt to render the Server Component. + const result = renderElement( + request, + element.type, + element.key, + element.ref, + element.props, + prevThenableState, + ); + return renderModelDestructive( + request, + task, + emptyRoot, + '', + result, + null, + ); } case REACT_LAZY_TYPE: { - try { - const payload = (value: any)._payload; - const init = (value: any)._init; - const resolvedModel = init(payload); - return renderModelDestructive( - request, - task, - emptyRoot, - '', - resolvedModel, - null, - ); - } catch (x) { - return serializeLazyID(serializeCaughtValue(request, task, x)); - } + const payload = (value: any)._payload; + const init = (value: any)._init; + const resolvedModel = init(payload); + return renderModelDestructive( + request, + task, + emptyRoot, + '', + resolvedModel, + null, + ); } } From f3e86814a0e60bc5612dae749c26b66e2c602b3a Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Tue, 23 Jan 2024 23:37:20 -0500 Subject: [PATCH 6/7] Tail recursively render the result of server components That way this only happens in the non-terminal branches instead of being called on the terminal branches too. --- .../react-server/src/ReactFlightServer.js | 43 ++++++++++++------- 1 file changed, 27 insertions(+), 16 deletions(-) diff --git a/packages/react-server/src/ReactFlightServer.js b/packages/react-server/src/ReactFlightServer.js index ebb0e24e39c4e..0e998c983151f 100644 --- a/packages/react-server/src/ReactFlightServer.js +++ b/packages/react-server/src/ReactFlightServer.js @@ -137,7 +137,7 @@ type ReactJSONValue = | boolean | number | null - | $ReadOnlyArray + | $ReadOnlyArray | ReactClientObject; // Serializable values @@ -502,12 +502,13 @@ function createLazyWrapperAroundWakeable(wakeable: Wakeable) { function renderElement( request: Request, + task: Task, type: any, key: null | React$Key, ref: mixed, props: any, prevThenableState: ThenableState | null, -): ReactClientValue { +): ReactJSONValue { if (ref !== null && ref !== undefined) { // When the ref moves to the regular props object this will implicitly // throw for functions. We could probably relax it to a DEV warning for other @@ -529,7 +530,7 @@ function renderElement( } // This is a server-side component. prepareToUseHooksForComponent(prevThenableState); - const result = type(props); + let result = type(props); if ( typeof result === 'object' && result !== null && @@ -543,9 +544,9 @@ function renderElement( } // TODO: Once we accept Promises as children on the client, we can just return // the thenable here. - return createLazyWrapperAroundWakeable(result); + result = createLazyWrapperAroundWakeable(result); } - return result; + return renderModelDestructive(request, task, emptyRoot, '', result, null); } else if (typeof type === 'string') { // This is a host element. E.g. HTML. return [REACT_ELEMENT_TYPE, type, key, props]; @@ -555,7 +556,14 @@ function renderElement( // it as a wrapper. // TODO: If a key is specified, we should propagate its key to any children. // Same as if a Server Component has a key. - return props.children; + return renderModelDestructive( + request, + task, + emptyRoot, + '', + props.children, + null, + ); } // This might be a built-in React component. We'll let the client decide. // Any built-in works as long as its props are serializable. @@ -572,6 +580,7 @@ function renderElement( const wrappedType = init(payload); return renderElement( request, + task, wrappedType, key, ref, @@ -582,11 +591,20 @@ function renderElement( case REACT_FORWARD_REF_TYPE: { const render = type.render; prepareToUseHooksForComponent(prevThenableState); - return render(props, undefined); + const result = render(props, undefined); + return renderModelDestructive( + request, + task, + emptyRoot, + '', + result, + null, + ); } case REACT_MEMO_TYPE: { return renderElement( request, + task, type.type, key, ref, @@ -1105,22 +1123,15 @@ function renderModelDestructive( // TODO: Concatenate keys of parents onto children. const element: React$Element = (value: any); // Attempt to render the Server Component. - const result = renderElement( + return renderElement( request, + task, element.type, element.key, element.ref, element.props, prevThenableState, ); - return renderModelDestructive( - request, - task, - emptyRoot, - '', - result, - null, - ); } case REACT_LAZY_TYPE: { const payload = (value: any)._payload; From c289c7ea04b340fefd668d60ef945e76268b1006 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Tue, 23 Jan 2024 23:45:19 -0500 Subject: [PATCH 7/7] Stash the active context on the task This doesn't come into play in Flight because the provider is not destructive so you can't suspend in the current task within a new context. The feature is also deprecated anyway. This is just for parity. --- packages/react-server/src/ReactFlightServer.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/react-server/src/ReactFlightServer.js b/packages/react-server/src/ReactFlightServer.js index 0e998c983151f..97f55ead28f42 100644 --- a/packages/react-server/src/ReactFlightServer.js +++ b/packages/react-server/src/ReactFlightServer.js @@ -614,7 +614,7 @@ function renderElement( } case REACT_PROVIDER_TYPE: { if (enableServerContext) { - pushProvider(type._context, props.value); + task.context = pushProvider(type._context, props.value); if (__DEV__) { const extraKeys = Object.keys(props).filter(value => { if (value === 'children' || value === 'value') { @@ -1199,7 +1199,7 @@ function renderModelDestructive( } return serializeByValueID(providerId); } else if (value === POP) { - popProvider(); + task.context = popProvider(); if (__DEV__) { insideContextProps = null; isInsideContextValue = false;