From 70df94dbba3648e2928ad747dd68cc8ed566422e Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Wed, 24 Mar 2021 11:50:27 -0400 Subject: [PATCH 1/5] Add format context --- .../src/server/ReactDOMFizzServerBrowser.js | 6 ++- .../src/server/ReactDOMFizzServerNode.js | 6 ++- .../src/server/ReactDOMServerFormatConfig.js | 52 ++++++++++++++++++- .../server/ReactNativeServerFormatConfig.js | 27 ++++++++++ .../src/ReactNoopServer.js | 5 ++ packages/react-server/src/ReactFizzServer.js | 37 +++++++++++-- .../forks/ReactServerFormatConfig.custom.js | 2 + 7 files changed, 129 insertions(+), 6 deletions(-) diff --git a/packages/react-dom/src/server/ReactDOMFizzServerBrowser.js b/packages/react-dom/src/server/ReactDOMFizzServerBrowser.js index 95a58f0688dd8..254d2380d5ab2 100644 --- a/packages/react-dom/src/server/ReactDOMFizzServerBrowser.js +++ b/packages/react-dom/src/server/ReactDOMFizzServerBrowser.js @@ -16,7 +16,10 @@ import { abort, } from 'react-server/src/ReactFizzServer'; -import {createResponseState} from './ReactDOMServerFormatConfig'; +import { + createResponseState, + createRootFormatContext, +} from './ReactDOMServerFormatConfig'; type Options = { identifierPrefix?: string, @@ -46,6 +49,7 @@ function renderToReadableStream( children, controller, createResponseState(options ? options.identifierPrefix : undefined), + createRootFormatContext(), // We call this here in case we need options to initialize it. options ? options.progressiveChunkSize : undefined, options ? options.onError : undefined, options ? options.onCompleteAll : undefined, diff --git a/packages/react-dom/src/server/ReactDOMFizzServerNode.js b/packages/react-dom/src/server/ReactDOMFizzServerNode.js index 51a091948d4b1..c9d0cbc303148 100644 --- a/packages/react-dom/src/server/ReactDOMFizzServerNode.js +++ b/packages/react-dom/src/server/ReactDOMFizzServerNode.js @@ -17,7 +17,10 @@ import { abort, } from 'react-server/src/ReactFizzServer'; -import {createResponseState} from './ReactDOMServerFormatConfig'; +import { + createResponseState, + createRootFormatContext, +} from './ReactDOMServerFormatConfig'; function createDrainHandler(destination, request) { return () => startFlowing(request); @@ -46,6 +49,7 @@ function pipeToNodeWritable( children, destination, createResponseState(options ? options.identifierPrefix : undefined), + createRootFormatContext(), // We call this here in case we need options to initialize it. options ? options.progressiveChunkSize : undefined, options ? options.onError : undefined, options ? options.onCompleteAll : undefined, diff --git a/packages/react-dom/src/server/ReactDOMServerFormatConfig.js b/packages/react-dom/src/server/ReactDOMServerFormatConfig.js index c0e55b0a4601d..f7ed7ccf93971 100644 --- a/packages/react-dom/src/server/ReactDOMServerFormatConfig.js +++ b/packages/react-dom/src/server/ReactDOMServerFormatConfig.js @@ -22,7 +22,7 @@ import { import escapeTextForBrowser from './escapeTextForBrowser'; import invariant from 'shared/invariant'; -// Per response, +// Per response, global state that is not contextual to the rendering subtree. export type ResponseState = { placeholderPrefix: PrecomputedChunk, segmentPrefix: PrecomputedChunk, @@ -50,6 +50,56 @@ export function createResponseState( }; } +// Constants for the namespace we use. We don't actually provide the namespace but conditionally +// use different segment parents based on namespace. Therefore we use constants instead of the string. +const ROOT_NAMESPACE = 0; // At the root we don't need to know which namespace it is. We just need to know that it's already the right one. +const HTML_NAMESPACE = 1; +const SVG_NAMESPACE = 2; +const MATHML_NAMESPACE = 3; + +type NamespaceFlag = 0 | 1 | 2 | 3; + +// Lets us keep track of contextual state and pick it back up after suspending. +export type FormatContext = { + namespace: NamespaceFlag, // root/svg/html/mathml + selectedValue: null | string, // the selected value(s) inside a +}; + +function createFormatContext( + namespace: NamespaceFlag, + selectedValue: null | string, +): FormatContext { + return { + namespace, + selectedValue, + }; +} + +export function createRootFormatContext(): FormatContext { + return createFormatContext(ROOT_NAMESPACE, null); +} + +export function getChildFormatContext( + parentContext: FormatContext, + type: string, + props: Object, +): FormatContext { + switch (type) { + case 'select': + return createFormatContext( + parentContext.namespace, + props.value != null ? props.value : props.defaultValue, + ); + case 'svg': + return createFormatContext(SVG_NAMESPACE, null); + case 'math': + return createFormatContext(MATHML_NAMESPACE, null); + case 'foreignObject': + return createFormatContext(HTML_NAMESPACE, null); + } + return parentContext; +} + // This object is used to lazily reuse the ID of the first generated node, or assign one. // We can't assign an ID up front because the node we're attaching it to might already // have one. So we need to lazily use that if it's available. diff --git a/packages/react-native-renderer/src/server/ReactNativeServerFormatConfig.js b/packages/react-native-renderer/src/server/ReactNativeServerFormatConfig.js index d89fafea818e3..3e6b37e76c18a 100644 --- a/packages/react-native-renderer/src/server/ReactNativeServerFormatConfig.js +++ b/packages/react-native-renderer/src/server/ReactNativeServerFormatConfig.js @@ -66,6 +66,33 @@ export function createResponseState(): ResponseState { }; } +// isInAParentText +export type FormatContext = boolean; + +export function createRootFormatContext(): FormatContext { + return false; +} + +export function getChildFormatContext( + parentContext: FormatContext, + type: string, + props: Object, +): FormatContext { + const prevIsInAParentText = parentContext; + const isInAParentText = + type === 'AndroidTextInput' || // Android + type === 'RCTMultilineTextInputView' || // iOS + type === 'RCTSinglelineTextInputView' || // iOS + type === 'RCTText' || + type === 'RCTVirtualText'; + + if (prevIsInAParentText !== isInAParentText) { + return isInAParentText; + } else { + return parentContext; + } +} + // This object is used to lazily reuse the ID of the first generated node, or assign one. // This is very specific to DOM where we can't assign an ID to. export type SuspenseBoundaryID = number; diff --git a/packages/react-noop-renderer/src/ReactNoopServer.js b/packages/react-noop-renderer/src/ReactNoopServer.js index feec884743efc..f384baa8d9f9e 100644 --- a/packages/react-noop-renderer/src/ReactNoopServer.js +++ b/packages/react-noop-renderer/src/ReactNoopServer.js @@ -82,6 +82,10 @@ const ReactNoopServer = ReactFizzServer({ return {state: 'pending', children: []}; }, + getChildFormatContext(): null { + return null; + }, + pushTextInstance(target: Array, text: string): void { const textInstance: TextInstance = { text, @@ -236,6 +240,7 @@ function render(children: React$Element, options?: Options): Destination { children, destination, null, + null, options ? options.progressiveChunkSize : undefined, options ? options.onError : undefined, options ? options.onCompleteAll : undefined, diff --git a/packages/react-server/src/ReactFizzServer.js b/packages/react-server/src/ReactFizzServer.js index b936376e85eae..8805e0b0679f6 100644 --- a/packages/react-server/src/ReactFizzServer.js +++ b/packages/react-server/src/ReactFizzServer.js @@ -17,6 +17,7 @@ import type {ReactNodeList} from 'shared/ReactTypes'; import type { SuspenseBoundaryID, ResponseState, + FormatContext, } from './ReactServerFormatConfig'; import { @@ -44,6 +45,7 @@ import { pushStartInstance, pushEndInstance, createSuspenseBoundaryID, + getChildFormatContext, } from './ReactServerFormatConfig'; import {REACT_ELEMENT_TYPE, REACT_SUSPENSE_TYPE} from 'shared/ReactSymbols'; import ReactSharedInternals from 'shared/ReactSharedInternals'; @@ -69,6 +71,7 @@ type SuspendedWork = { blockedBoundary: Root | SuspenseBoundary, blockedSegment: Segment, // the segment we'll write to abortSet: Set, // the abortable set that this work belongs to + formatContext: FormatContext, assignID: null | SuspenseBoundaryID, // id to assign to the content }; @@ -142,6 +145,7 @@ export function createRequest( children: ReactNodeList, destination: Destination, responseState: ResponseState, + rootContext: FormatContext, progressiveChunkSize: number = DEFAULT_PROGRESSIVE_CHUNK_SIZE, onError: (error: mixed) => void = noop, onCompleteAll: () => void = noop, @@ -177,6 +181,7 @@ export function createRequest( null, rootSegment, abortSet, + rootContext, null, ); pingedWork.push(rootWork); @@ -213,6 +218,7 @@ function createSuspendedWork( blockedBoundary: Root | SuspenseBoundary, blockedSegment: Segment, abortSet: Set, + formatContext: FormatContext, assignID: null | SuspenseBoundaryID, ): SuspendedWork { request.allPendingWork++; @@ -227,6 +233,7 @@ function createSuspendedWork( blockedBoundary, blockedSegment, abortSet, + formatContext, assignID, }; abortSet.add(work); @@ -263,6 +270,20 @@ function fatalError(request: Request, error: mixed): void { closeWithError(request.destination, error); } +// TODO: Add legacy and regular contexts here too +let currentFormatContext: FormatContext; +function restoreContext(work: SuspendedWork): void { + currentFormatContext = work.formatContext; +} + +function saveContext(workToSaveTo: SuspendedWork): void { + workToSaveTo.formatContext = currentFormatContext; +} + +function resetContext(): void { + currentFormatContext = (undefined: any); +} + function renderNode( request: Request, parentBoundary: Root | SuspenseBoundary, @@ -315,11 +336,11 @@ function renderNode( parentBoundary, newSegment, abortSet, + currentFormatContext, assignID, ); const ping = suspendedWork.ping; x.then(ping, ping); - // TODO: Emit place holder } else { // We can rethrow to terminate the rest of this tree. throw x; @@ -333,6 +354,8 @@ function renderNode( request.responseState, assignID, ); + const prevContext = currentFormatContext; + currentFormatContext = getChildFormatContext(prevContext, type, props); renderNode( request, parentBoundary, @@ -341,6 +364,7 @@ function renderNode( abortSet, null, ); + currentFormatContext = prevContext; pushEndInstance(segment.chunks, type, props); } else if (type === REACT_SUSPENSE_TYPE) { // We need to push an "empty" thing here to identify the parent suspense boundary. @@ -372,6 +396,7 @@ function renderNode( parentBoundary, boundarySegment, fallbackAbortSet, + currentFormatContext, newBoundary.id, // This is the ID we want to give this fallback so we can replace it later. ); // TODO: This should be queued at a separate lower priority queue so that we only work @@ -383,15 +408,17 @@ function renderNode( // We mark the root segment as having its parent flushed. It's not really flushed but there is // no parent segment so there's nothing to wait on. contentRootSegment.parentFlushed = true; - // TODO: Currently this is running synchronously. We could instead schedule this to pingedWork. + // Currently this is running synchronously. We could instead schedule this to pingedWork. // I suspect that there might be some efficiency benefits from not creating the suspended work - // and instead just using the stack if possible. Particularly when we add contexts. + // and instead just using the stack if possible. + // TODO: Call this directly instead of messing with saving and restoring contexts. const contentWork = createSuspendedWork( request, content, newBoundary, contentRootSegment, abortSet, + currentFormatContext, null, ); retryWork(request, contentWork); @@ -542,6 +569,7 @@ function finishedWork( } function retryWork(request: Request, work: SuspendedWork): void { + restoreContext(work); const segment = work.blockedSegment; if (segment.status !== PENDING) { // We completed this by other means before we had a chance to retry it. @@ -572,6 +600,7 @@ function retryWork(request: Request, work: SuspendedWork): void { finishedWork(request, boundary, segment); } catch (x) { if (typeof x === 'object' && x !== null && typeof x.then === 'function') { + saveContext(work); // Something suspended again, let's pick it back up later. const ping = work.ping; x.then(ping, ping); @@ -580,6 +609,8 @@ function retryWork(request: Request, work: SuspendedWork): void { segment.status = ERRORED; erroredWork(request, boundary, segment, x); } + } finally { + resetContext(); } } diff --git a/packages/react-server/src/forks/ReactServerFormatConfig.custom.js b/packages/react-server/src/forks/ReactServerFormatConfig.custom.js index 76219e1a0748e..58f2d504b46b8 100644 --- a/packages/react-server/src/forks/ReactServerFormatConfig.custom.js +++ b/packages/react-server/src/forks/ReactServerFormatConfig.custom.js @@ -26,8 +26,10 @@ declare var $$$hostConfig: any; export opaque type Destination = mixed; // eslint-disable-line no-undef export opaque type ResponseState = mixed; +export opaque type FormatContext = mixed; export opaque type SuspenseBoundaryID = mixed; +export const getChildFormatContext = $$$hostConfig.getChildFormatContext; export const createSuspenseBoundaryID = $$$hostConfig.createSuspenseBoundaryID; export const pushEmpty = $$$hostConfig.pushEmpty; export const pushTextInstance = $$$hostConfig.pushTextInstance; From ecaa178a57a291fb115d82860cdc21df622d7f7b Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Thu, 25 Mar 2021 17:48:17 -0400 Subject: [PATCH 2/5] Let the Work node hold all working state for the recursive loop Stacks are nice and all but there's a cost to maintaining each frame both in terms of stack size usage and writing to it. --- packages/react-server/src/ReactFizzServer.js | 71 ++++++++++---------- 1 file changed, 37 insertions(+), 34 deletions(-) diff --git a/packages/react-server/src/ReactFizzServer.js b/packages/react-server/src/ReactFizzServer.js index 8805e0b0679f6..69db2e1f6cc59 100644 --- a/packages/react-server/src/ReactFizzServer.js +++ b/packages/react-server/src/ReactFizzServer.js @@ -286,26 +286,34 @@ function resetContext(): void { function renderNode( request: Request, - parentBoundary: Root | SuspenseBoundary, - segment: Segment, + work: SuspendedWork, node: ReactNodeList, - abortSet: Set, - assignID: null | SuspenseBoundaryID, ): void { if (typeof node === 'string') { - pushTextInstance(segment.chunks, node, request.responseState, assignID); + pushTextInstance( + work.blockedSegment.chunks, + node, + request.responseState, + work.assignID, + ); return; } if (Array.isArray(node)) { if (node.length > 0) { - // Only the first node gets assigned an ID. - renderNode(request, parentBoundary, segment, node[0], abortSet, assignID); + renderNode(request, work, node[0]); + // Only the first node gets assigned an ID. We've either handed it off to other work + // or assigned it already. + work.assignID = null; for (let i = 1; i < node.length; i++) { - renderNode(request, parentBoundary, segment, node[i], abortSet, null); + renderNode(request, work, node[i]); } } else { - pushEmpty(segment.chunks, request.responseState, assignID); + pushEmpty( + work.blockedSegment.chunks, + request.responseState, + work.assignID, + ); } return; } @@ -323,21 +331,22 @@ function renderNode( if (typeof type === 'function') { try { const result = type(props); - renderNode(request, parentBoundary, segment, result, abortSet, assignID); + renderNode(request, work, result); } catch (x) { if (typeof x === 'object' && x !== null && typeof x.then === 'function') { // Something suspended, we'll need to create a new segment and resolve it later. + const segment = work.blockedSegment; const insertionIndex = segment.chunks.length; const newSegment = createPendingSegment(request, insertionIndex, null); segment.children.push(newSegment); const suspendedWork = createSuspendedWork( request, node, - parentBoundary, + work.blockedBoundary, newSegment, - abortSet, + work.abortSet, currentFormatContext, - assignID, + work.assignID, ); const ping = suspendedWork.ping; x.then(ping, ping); @@ -348,27 +357,23 @@ function renderNode( } } else if (typeof type === 'string') { pushStartInstance( - segment.chunks, + work.blockedSegment.chunks, type, props, request.responseState, - assignID, + work.assignID, ); + // We must have assigned it already above so we don't need this anymore. + work.assignID = null; const prevContext = currentFormatContext; currentFormatContext = getChildFormatContext(prevContext, type, props); - renderNode( - request, - parentBoundary, - segment, - props.children, - abortSet, - null, - ); + renderNode(request, work, props.children); currentFormatContext = prevContext; - pushEndInstance(segment.chunks, type, props); + pushEndInstance(work.blockedSegment.chunks, type, props); } else if (type === REACT_SUSPENSE_TYPE) { + const segment = work.blockedSegment; // We need to push an "empty" thing here to identify the parent suspense boundary. - pushEmpty(segment.chunks, request.responseState, assignID); + pushEmpty(segment.chunks, request.responseState, work.assignID); // Each time we enter a suspense boundary, we split out into a new segment for // the fallback so that we can later replace that segment with the content. // This also lets us split out the main content even if it doesn't suspend, @@ -393,7 +398,7 @@ function renderNode( const suspendedFallbackWork = createSuspendedWork( request, fallback, - parentBoundary, + work.blockedBoundary, boundarySegment, fallbackAbortSet, currentFormatContext, @@ -417,7 +422,7 @@ function renderNode( content, newBoundary, contentRootSegment, - abortSet, + work.abortSet, currentFormatContext, null, ); @@ -575,8 +580,6 @@ function retryWork(request: Request, work: SuspendedWork): void { // We completed this by other means before we had a chance to retry it. return; } - const boundary = work.blockedBoundary; - const abortSet = work.abortSet; try { let node = work.node; while ( @@ -593,11 +596,11 @@ function retryWork(request: Request, work: SuspendedWork): void { node = element.type(element.props); } - renderNode(request, boundary, segment, node, abortSet, work.assignID); + renderNode(request, work, node); - abortSet.delete(work); + work.abortSet.delete(work); segment.status = COMPLETED; - finishedWork(request, boundary, segment); + finishedWork(request, work.blockedBoundary, segment); } catch (x) { if (typeof x === 'object' && x !== null && typeof x.then === 'function') { saveContext(work); @@ -605,9 +608,9 @@ function retryWork(request: Request, work: SuspendedWork): void { const ping = work.ping; x.then(ping, ping); } else { - abortSet.delete(work); + work.abortSet.delete(work); segment.status = ERRORED; - erroredWork(request, boundary, segment, x); + erroredWork(request, work.blockedBoundary, segment, x); } } finally { resetContext(); From 97e9d15ffeac5099e5447bf311b7ceee4ce36cc2 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Thu, 25 Mar 2021 17:59:45 -0400 Subject: [PATCH 3/5] Move current format context into work --- packages/react-server/src/ReactFizzServer.js | 32 +++++--------------- 1 file changed, 8 insertions(+), 24 deletions(-) diff --git a/packages/react-server/src/ReactFizzServer.js b/packages/react-server/src/ReactFizzServer.js index 69db2e1f6cc59..20b49f04eea16 100644 --- a/packages/react-server/src/ReactFizzServer.js +++ b/packages/react-server/src/ReactFizzServer.js @@ -270,20 +270,6 @@ function fatalError(request: Request, error: mixed): void { closeWithError(request.destination, error); } -// TODO: Add legacy and regular contexts here too -let currentFormatContext: FormatContext; -function restoreContext(work: SuspendedWork): void { - currentFormatContext = work.formatContext; -} - -function saveContext(workToSaveTo: SuspendedWork): void { - workToSaveTo.formatContext = currentFormatContext; -} - -function resetContext(): void { - currentFormatContext = (undefined: any); -} - function renderNode( request: Request, work: SuspendedWork, @@ -345,7 +331,7 @@ function renderNode( work.blockedBoundary, newSegment, work.abortSet, - currentFormatContext, + work.formatContext, work.assignID, ); const ping = suspendedWork.ping; @@ -365,10 +351,12 @@ function renderNode( ); // We must have assigned it already above so we don't need this anymore. work.assignID = null; - const prevContext = currentFormatContext; - currentFormatContext = getChildFormatContext(prevContext, type, props); + const prevContext = work.formatContext; + work.formatContext = getChildFormatContext(prevContext, type, props); renderNode(request, work, props.children); - currentFormatContext = prevContext; + // We expect that errors will fatal the whole work and that we don't need + // the correct context. Therefore this is not in a finally. + work.formatContext = prevContext; pushEndInstance(work.blockedSegment.chunks, type, props); } else if (type === REACT_SUSPENSE_TYPE) { const segment = work.blockedSegment; @@ -401,7 +389,7 @@ function renderNode( work.blockedBoundary, boundarySegment, fallbackAbortSet, - currentFormatContext, + work.formatContext, newBoundary.id, // This is the ID we want to give this fallback so we can replace it later. ); // TODO: This should be queued at a separate lower priority queue so that we only work @@ -423,7 +411,7 @@ function renderNode( newBoundary, contentRootSegment, work.abortSet, - currentFormatContext, + work.formatContext, null, ); retryWork(request, contentWork); @@ -574,7 +562,6 @@ function finishedWork( } function retryWork(request: Request, work: SuspendedWork): void { - restoreContext(work); const segment = work.blockedSegment; if (segment.status !== PENDING) { // We completed this by other means before we had a chance to retry it. @@ -603,7 +590,6 @@ function retryWork(request: Request, work: SuspendedWork): void { finishedWork(request, work.blockedBoundary, segment); } catch (x) { if (typeof x === 'object' && x !== null && typeof x.then === 'function') { - saveContext(work); // Something suspended again, let's pick it back up later. const ping = work.ping; x.then(ping, ping); @@ -612,8 +598,6 @@ function retryWork(request: Request, work: SuspendedWork): void { segment.status = ERRORED; erroredWork(request, work.blockedBoundary, segment, x); } - } finally { - resetContext(); } } From 27cf4056fc790b59da880a5965eae546fc9ebca0 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Thu, 25 Mar 2021 20:39:51 -0400 Subject: [PATCH 4/5] Synchronously render children of a Suspense boundary We don't have to spawn work and snapshot the context. Instead we can try to render the boundary immediately in case it works. --- packages/react-server/src/ReactFizzServer.js | 61 +++++++++++++------- 1 file changed, 40 insertions(+), 21 deletions(-) diff --git a/packages/react-server/src/ReactFizzServer.js b/packages/react-server/src/ReactFizzServer.js index 20b49f04eea16..c4cf864c2e46e 100644 --- a/packages/react-server/src/ReactFizzServer.js +++ b/packages/react-server/src/ReactFizzServer.js @@ -282,16 +282,13 @@ function renderNode( request.responseState, work.assignID, ); + work.assignID = null; return; } if (Array.isArray(node)) { if (node.length > 0) { - renderNode(request, work, node[0]); - // Only the first node gets assigned an ID. We've either handed it off to other work - // or assigned it already. - work.assignID = null; - for (let i = 1; i < node.length; i++) { + for (let i = 0; i < node.length; i++) { renderNode(request, work, node[i]); } } else { @@ -300,6 +297,7 @@ function renderNode( request.responseState, work.assignID, ); + work.assignID = null; } return; } @@ -334,6 +332,8 @@ function renderNode( work.formatContext, work.assignID, ); + // We've delegated the assignment. + work.assignID = null; const ping = suspendedWork.ping; x.then(ping, ping); } else { @@ -359,9 +359,12 @@ function renderNode( work.formatContext = prevContext; pushEndInstance(work.blockedSegment.chunks, type, props); } else if (type === REACT_SUSPENSE_TYPE) { - const segment = work.blockedSegment; + const parentBoundary = work.blockedBoundary; + const parentSegment = work.blockedSegment; + // We need to push an "empty" thing here to identify the parent suspense boundary. - pushEmpty(segment.chunks, request.responseState, work.assignID); + pushEmpty(parentSegment.chunks, request.responseState, work.assignID); + work.assignID = null; // Each time we enter a suspense boundary, we split out into a new segment for // the fallback so that we can later replace that segment with the content. // This also lets us split out the main content even if it doesn't suspend, @@ -371,22 +374,21 @@ function renderNode( const fallbackAbortSet: Set = new Set(); const newBoundary = createSuspenseBoundary(request, fallbackAbortSet); - - const insertionIndex = segment.chunks.length; + const insertionIndex = parentSegment.chunks.length; // The children of the boundary segment is actually the fallback. const boundarySegment = createPendingSegment( request, insertionIndex, newBoundary, ); - segment.children.push(boundarySegment); + parentSegment.children.push(boundarySegment); // We create suspended work for the fallback because we don't want to actually work // on it yet in case we finish the main content, so we queue for later. const suspendedFallbackWork = createSuspendedWork( request, fallback, - work.blockedBoundary, + parentBoundary, boundarySegment, fallbackAbortSet, work.formatContext, @@ -401,20 +403,37 @@ function renderNode( // We mark the root segment as having its parent flushed. It's not really flushed but there is // no parent segment so there's nothing to wait on. contentRootSegment.parentFlushed = true; + // Currently this is running synchronously. We could instead schedule this to pingedWork. // I suspect that there might be some efficiency benefits from not creating the suspended work // and instead just using the stack if possible. // TODO: Call this directly instead of messing with saving and restoring contexts. - const contentWork = createSuspendedWork( - request, - content, - newBoundary, - contentRootSegment, - work.abortSet, - work.formatContext, - null, - ); - retryWork(request, contentWork); + + // We can reuse the current context and work to render the content immediately without + // context switching. We just need to temporarily switch which boundary and which segment + // we're writing to. If something suspends, it'll spawn new suspended work with that context. + work.blockedBoundary = newBoundary; + work.blockedSegment = contentRootSegment; + try { + renderNode(request, work, content); + contentRootSegment.status = COMPLETED; + newBoundary.completedSegments.push(contentRootSegment); + if (newBoundary.pendingWork === 0) { + // This must have been the last segment we were waiting on. This boundary is now complete. + // We can now cancel any pending work on the fallback since we won't need to show it anymore. + newBoundary.fallbackAbortableWork.forEach(abortWorkSoft, request); + newBoundary.fallbackAbortableWork.clear(); + } + } catch (error) { + contentRootSegment.status = ERRORED; + reportError(request, error); + newBoundary.forceClientRender = true; + // We don't need to decrement any work numbers because we didn't spawn any new work. + // We don't need to schedule any work because we know the parent has written yet. + } finally { + work.blockedBoundary = parentBoundary; + work.blockedSegment = parentSegment; + } } else { throw new Error('Not yet implemented element type.'); } From 6dcd49afbd8a6e77e36a2477fbaae8ecf5de8513 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Thu, 25 Mar 2021 20:45:41 -0400 Subject: [PATCH 5/5] Lazily create the fallback work Instead of eagerly create the fallback work and then immediately abort it. We can just avoid creating it if we finish synchronously. --- packages/react-server/src/ReactFizzServer.js | 37 ++++++++++---------- 1 file changed, 19 insertions(+), 18 deletions(-) diff --git a/packages/react-server/src/ReactFizzServer.js b/packages/react-server/src/ReactFizzServer.js index c4cf864c2e46e..92ff01c99a7af 100644 --- a/packages/react-server/src/ReactFizzServer.js +++ b/packages/react-server/src/ReactFizzServer.js @@ -383,21 +383,6 @@ function renderNode( ); parentSegment.children.push(boundarySegment); - // We create suspended work for the fallback because we don't want to actually work - // on it yet in case we finish the main content, so we queue for later. - const suspendedFallbackWork = createSuspendedWork( - request, - fallback, - parentBoundary, - boundarySegment, - fallbackAbortSet, - work.formatContext, - newBoundary.id, // This is the ID we want to give this fallback so we can replace it later. - ); - // TODO: This should be queued at a separate lower priority queue so that we only work - // on preparing fallbacks if we don't have any more main content to work on. - request.pingedWork.push(suspendedFallbackWork); - // This segment is the actual child content. We can start rendering that immediately. const contentRootSegment = createPendingSegment(request, 0, null); // We mark the root segment as having its parent flushed. It's not really flushed but there is @@ -420,9 +405,9 @@ function renderNode( newBoundary.completedSegments.push(contentRootSegment); if (newBoundary.pendingWork === 0) { // This must have been the last segment we were waiting on. This boundary is now complete. - // We can now cancel any pending work on the fallback since we won't need to show it anymore. - newBoundary.fallbackAbortableWork.forEach(abortWorkSoft, request); - newBoundary.fallbackAbortableWork.clear(); + // Therefore we won't need the fallback. We early return so that we don't have to create + // the fallback. + return; } } catch (error) { contentRootSegment.status = ERRORED; @@ -430,10 +415,26 @@ function renderNode( newBoundary.forceClientRender = true; // We don't need to decrement any work numbers because we didn't spawn any new work. // We don't need to schedule any work because we know the parent has written yet. + // We do need to fallthrough to create the fallback though. } finally { work.blockedBoundary = parentBoundary; work.blockedSegment = parentSegment; } + + // We create suspended work for the fallback because we don't want to actually work + // on it yet in case we finish the main content, so we queue for later. + const suspendedFallbackWork = createSuspendedWork( + request, + fallback, + parentBoundary, + boundarySegment, + fallbackAbortSet, + work.formatContext, + newBoundary.id, // This is the ID we want to give this fallback so we can replace it later. + ); + // TODO: This should be queued at a separate lower priority queue so that we only work + // on preparing fallbacks if we don't have any more main content to work on. + request.pingedWork.push(suspendedFallbackWork); } else { throw new Error('Not yet implemented element type.'); }