Repository: facebook/react
Pull request: [Fizz] Reuse rootSegmentID as the SuspenseBoundaryID

Originally the intension was to have React assign an ID to a user rendered DOM node inside a `fallback` while it was loading. If there already were an explicit `id` defined on the DOM element we would reuse that one instead. That's why this was a DOM Config option and not just built in to Fizz.

This became tricky since it can load late and so we'd have to transfer it down and detect it only once it finished rendering and if there is no DOM element it doesn't work anyway. So instead, what we do in practice is to always use a `<template>` tag with the ID. This has the downside of an extra useless node and shifting child CSS selectors.

Maybe we'll get around to fixing this properly but it might not be worth it.

This PR just gets rid of the SuspenseBoundaryID concept and instead we just use the same ID number as the root segment ID of the boundary to refer to the boundary to simplify the implementation.

This also solves the problem that SuspenseBoundaryID isn't currently serializable (although that's easily fixable by itself if necessary).

Changed files:
filename: packages/react-dom-bindings/src/server/ReactFizzConfigDOM.js
patch: @@ -123,7 +123,7 @@ export type RenderState = {
   // These can be recreated from resumable state.
   placeholderPrefix: PrecomputedChunk,
   segmentPrefix: PrecomputedChunk,
-  boundaryPrefix: string,
+  boundaryPrefix: PrecomputedChunk,
 
   // inline script streaming format, unused if using external runtime / data
   startInlineScript: PrecomputedChunk,
@@ -162,7 +162,7 @@ export type ResumableState = {
   externalRuntimeScript: null | ExternalRuntimeScript, // TODO: Move to a serializable format
   bootstrapChunks: Array<Chunk | PrecomputedChunk>, // TODO: Move to a serializable format.
   idPrefix: string,
-  nextSuspenseID: number,
+  nextFormID: number,
   streamingFormat: StreamingFormat,
 
   // state for script streaming format, unused if using external runtime / data
@@ -272,7 +272,7 @@ export function createRenderState(
   return {
     placeholderPrefix: stringToPrecomputedChunk(idPrefix + 'P:'),
     segmentPrefix: stringToPrecomputedChunk(idPrefix + 'S:'),
-    boundaryPrefix: idPrefix + 'B:',
+    boundaryPrefix: stringToPrecomputedChunk(idPrefix + 'B:'),
     startInlineScript: inlineScriptWithNonce,
     htmlChunks: null,
     headChunks: null,
@@ -353,7 +353,7 @@ export function createResumableState(
     externalRuntimeScript: externalRuntimeScript,
     bootstrapChunks: bootstrapChunks,
     idPrefix: idPrefix,
-    nextSuspenseID: 0,
+    nextFormID: 0,
     streamingFormat,
     instructions: NothingSent,
     hasBody: false,
@@ -605,20 +605,6 @@ export function getChildFormatContext(
   return parentContext;
 }
 
-export type SuspenseBoundaryID = null | PrecomputedChunk;
-
-export const UNINITIALIZED_SUSPENSE_BOUNDARY_ID: SuspenseBoundaryID = null;
-
-export function assignSuspenseBoundaryID(
-  renderState: RenderState,
-  resumableState: ResumableState,
-): SuspenseBoundaryID {
-  const generatedID = resumableState.nextSuspenseID++;
-  return stringToPrecomputedChunk(
-    renderState.boundaryPrefix + generatedID.toString(16),
-  );
-}
-
 export function makeId(
   resumableState: ResumableState,
   treeId: string,
@@ -806,9 +792,7 @@ function pushStringAttribute(
 }
 
 function makeFormFieldPrefix(resumableState: ResumableState): string {
-  // I'm just reusing this counter. It's not really the same namespace as "name".
-  // It could just be its own counter.
-  const id = resumableState.nextSuspenseID++;
+  const id = resumableState.nextFormID++;
   return resumableState.idPrefix + id;
 }
 
@@ -3503,7 +3487,7 @@ export function writeStartCompletedSuspenseBoundary(
 export function writeStartPendingSuspenseBoundary(
   destination: Destination,
   renderState: RenderState,
-  id: SuspenseBoundaryID,
+  id: number,
 ): boolean {
   writeChunk(destination, startPendingSuspenseBoundary1);
 
@@ -3513,7 +3497,8 @@ export function writeStartPendingSuspenseBoundary(
     );
   }
 
-  writeChunk(destination, id);
+  writeChunk(destination, renderState.boundaryPrefix);
+  writeChunk(destination, stringToChunk(id.toString(16)));
   return writeChunkAndReturn(destination, startPendingSuspenseBoundary2);
 }
 export function writeStartClientRenderedSuspenseBoundary(
@@ -3807,8 +3792,7 @@ export function writeCompletedBoundaryInstruction(
   destination: Destination,
   resumableState: ResumableState,
   renderState: RenderState,
-  boundaryID: SuspenseBoundaryID,
-  contentSegmentID: number,
+  id: number,
   boundaryResources: BoundaryResources,
 ): boolean {
   let requiresStyleInsertion;
@@ -3864,22 +3848,19 @@ export function writeCompletedBoundaryInstruction(
     }
   }
 
-  if (boundaryID === null) {
-    throw new Error(
-      'An ID must have been assigned before we can complete the boundary.',
-    );
-  }
+  const idChunk = stringToChunk(id.toString(16));
+
+  writeChunk(destination, renderState.boundaryPrefix);
+  writeChunk(destination, idChunk);
 
   // Write function arguments, which are string and array literals
-  const formattedContentID = stringToChunk(contentSegmentID.toString(16));
-  writeChunk(destination, boundaryID);
   if (scriptFormat) {
     writeChunk(destination, completeBoundaryScript2);
   } else {
     writeChunk(destination, completeBoundaryData2);
   }
   writeChunk(destination, renderState.segmentPrefix);
-  writeChunk(destination, formattedContentID);
+  writeChunk(destination, idChunk);
   if (enableFloat && requiresStyleInsertion) {
     // Script and data writers must format this differently:
     //  - script writer emits an array literal, whose string elements are
@@ -3928,7 +3909,7 @@ export function writeClientRenderBoundaryInstruction(
   destination: Destination,
   resumableState: ResumableState,
   renderState: RenderState,
-  boundaryID: SuspenseBoundaryID,
+  id: number,
   errorDigest: ?string,
   errorMessage?: string,
   errorComponentStack?: string,
@@ -3954,13 +3935,8 @@ export function writeClientRenderBoundaryInstruction(
     writeChunk(destination, clientRenderData1);
   }
 
-  if (boundaryID === null) {
-    throw new Error(
-      'An ID must have been assigned before we can complete the boundary.',
-    );
-  }
-
-  writeChunk(destination, boundaryID);
+  writeChunk(destination, renderState.boundaryPrefix);
+  writeChunk(destination, stringToChunk(id.toString(16)));
   if (scriptFormat) {
     // " needs to be inserted for scripts, since ArgInterstitual does not contain
     // leading or trailing quotes

filename: packages/react-dom-bindings/src/server/ReactFizzConfigDOMLegacy.js
patch: @@ -35,7 +35,7 @@ export type RenderState = {
   // Keep this in sync with ReactFizzConfigDOM
   placeholderPrefix: PrecomputedChunk,
   segmentPrefix: PrecomputedChunk,
-  boundaryPrefix: string,
+  boundaryPrefix: PrecomputedChunk,
   startInlineScript: PrecomputedChunk,
   htmlChunks: null | Array<Chunk | PrecomputedChunk>,
   headChunks: null | Array<Chunk | PrecomputedChunk>,
@@ -89,13 +89,10 @@ export type {
   ResumableState,
   BoundaryResources,
   FormatContext,
-  SuspenseBoundaryID,
 } from './ReactFizzConfigDOM';
 
 export {
   getChildFormatContext,
-  UNINITIALIZED_SUSPENSE_BOUNDARY_ID,
-  assignSuspenseBoundaryID,
   makeId,
   pushStartInstance,
   pushEndInstance,

filename: packages/react-noop-renderer/src/ReactNoopServer.js
patch: @@ -90,13 +90,6 @@ const ReactNoopServer = ReactFizzServer({
   closeWithError(destination: Destination, error: mixed): void {},
   flushBuffered(destination: Destination): void {},
 
-  UNINITIALIZED_SUSPENSE_BOUNDARY_ID: null,
-
-  assignSuspenseBoundaryID(): SuspenseInstance {
-    // The ID is a pointer to the boundary itself.
-    return {state: 'pending', children: []};
-  },
-
   getChildFormatContext(): null {
     return null;
   },

filename: packages/react-server/src/ReactFizzServer.js
patch: @@ -23,7 +23,6 @@ import type {
 } from 'shared/ReactTypes';
 import type {LazyComponent as LazyComponentType} from 'react/src/ReactLazy';
 import type {
-  SuspenseBoundaryID,
   RenderState,
   ResumableState,
   FormatContext,
@@ -64,8 +63,6 @@ import {
   pushStartCompletedSuspenseBoundary,
   pushEndCompletedSuspenseBoundary,
   pushSegmentFinale,
-  UNINITIALIZED_SUSPENSE_BOUNDARY_ID,
-  assignSuspenseBoundaryID,
   getChildFormatContext,
   writeResourcesForBoundary,
   writePreamble,
@@ -176,7 +173,6 @@ type ReplaySuspenseBoundary = [
   string | null /* name */,
   string | number /* key */,
   Array<ResumableNode> /* children */,
-  SuspenseBoundaryID /* id */,
   number /* rootSegmentID */,
 ];
 
@@ -193,7 +189,6 @@ type ResumeSuspenseBoundary = [
   3, // RESUME_SUSPENSE_BOUNDARY
   string | null /* name */,
   string | number /* key */,
-  SuspenseBoundaryID /* id */,
   number /* rootSegmentID */,
 ];
 
@@ -229,7 +224,6 @@ const CLIENT_RENDERED = 4; // if it errors or infinitely suspends
 
 type SuspenseBoundary = {
   status: 0 | 1 | 4 | 5,
-  id: SuspenseBoundaryID,
   rootSegmentID: number,
   errorDigest: ?string, // the error hash if it errors
   errorMessage?: string, // the error string if it errors
@@ -575,7 +569,6 @@ function createSuspenseBoundary(
 ): SuspenseBoundary {
   return {
     status: PENDING,
-    id: UNINITIALIZED_SUSPENSE_BOUNDARY_ID,
     rootSegmentID: -1,
     parentFlushed: false,
     pendingTasks: 0,
@@ -1004,8 +997,7 @@ function replaySuspenseBoundary(
   );
   resumedBoundary.parentFlushed = true;
   // We restore the same id of this boundary as was used during prerender.
-  resumedBoundary.id = replayNode[4];
-  resumedBoundary.rootSegmentID = replayNode[5];
+  resumedBoundary.rootSegmentID = replayNode[4];
 
   // We can reuse the current context and task to render the content immediately without
   // context switching. We just need to temporarily switch which boundary and replay node
@@ -1102,9 +1094,9 @@ function resumeSuspenseBoundary(
     task.keyPath,
   );
   resumedBoundary.parentFlushed = true;
+  const id = replayNode[3];
   // We restore the same id of this boundary as was used during prerender.
-  resumedBoundary.id = replayNode[3];
-  resumedBoundary.rootSegmentID = replayNode[4];
+  resumedBoundary.rootSegmentID = id;
 
   const resumedSegment = createPendingSegment(
     request,
@@ -1115,7 +1107,7 @@ function resumeSuspenseBoundary(
     false,
   );
   resumedSegment.parentFlushed = true;
-  resumedSegment.id = replayNode[4];
+  resumedSegment.id = id;
 
   // We can reuse the current context and task to render the content immediately without
   // context switching. We just need to temporarily switch which boundary and replay node
@@ -2647,10 +2639,6 @@ function trackPostpone(
     boundary.status = POSTPONED;
     // We need to eagerly assign it an ID because we'll need to refer to
     // it before flushing and we know that we can't inline it.
-    boundary.id = assignSuspenseBoundaryID(
-      request.renderState,
-      request.resumableState,
-    );
     boundary.rootSegmentID = request.nextSegmentId++;
 
     const boundaryKeyPath = boundary.keyPath;
@@ -2669,7 +2657,6 @@ function trackPostpone(
         RESUME_SUSPENSE_BOUNDARY,
         boundaryKeyPath[1],
         boundaryKeyPath[2],
-        boundary.id,
         boundary.rootSegmentID,
       ];
       addToReplayParent(boundaryNode, boundaryKeyPath[0], trackedPostpones);
@@ -2681,7 +2668,6 @@ function trackPostpone(
         boundaryKeyPath[1],
         boundaryKeyPath[2],
         children,
-        boundary.id,
         boundary.rootSegmentID,
       ];
       trackedPostpones.workingMap.set(boundaryKeyPath, boundaryNode);
@@ -3098,7 +3084,6 @@ function abortTaskSoft(this: Request, task: Task): void {
 
 function abortRemainingSuspenseBoundary(
   request: Request,
-  id: SuspenseBoundaryID,
   rootSegmentID: number,
   error: mixed,
   errorDigest: ?string,
@@ -3110,7 +3095,6 @@ function abortRemainingSuspenseBoundary(
   );
   resumedBoundary.parentFlushed = true;
   // We restore the same id of this boundary as was used during prerender.
-  resumedBoundary.id = id;
   resumedBoundary.rootSegmentID = rootSegmentID;
 
   resumedBoundary.status = CLIENT_RENDERED;
@@ -3159,11 +3143,9 @@ function abortRemainingResumableNodes(
       }
       case REPLAY_SUSPENSE_BOUNDARY: {
         const boundaryNode: ReplaySuspenseBoundary = node;
-        const id = boundaryNode[4];
-        const rootSegmentID = boundaryNode[5];
+        const rootSegmentID = boundaryNode[4];
         abortRemainingSuspenseBoundary(
           request,
-          id,
           rootSegmentID,
           error,
           errorDigest,
@@ -3172,11 +3154,9 @@ function abortRemainingResumableNodes(
       }
       case RESUME_SUSPENSE_BOUNDARY: {
         const boundaryNode: ResumeSuspenseBoundary = node;
-        const id = boundaryNode[3];
-        const rootSegmentID = boundaryNode[4];
+        const rootSegmentID = boundaryNode[3];
         abortRemainingSuspenseBoundary(
           request,
-          id,
           rootSegmentID,
           error,
           errorDigest,
@@ -3753,10 +3733,6 @@ function flushSegment(
     if (boundary.status === PENDING) {
       // For pending boundaries we lazily assign an ID to the boundary
       // and root segment.
-      boundary.id = assignSuspenseBoundaryID(
-        request.renderState,
-        request.resumableState,
-      );
       boundary.rootSegmentID = request.nextSegmentId++;
     }
 
@@ -3767,9 +3743,7 @@ function flushSegment(
 
     // This boundary is still loading. Emit a pending suspense boundary wrapper.
 
-    /// This is the first time we should have referenced this ID.
-    const id = boundary.id;
-
+    const id = boundary.rootSegmentID;
     writeStartPendingSuspenseBoundary(destination, request.renderState, id);
 
     // Flush the fallback.
@@ -3791,7 +3765,7 @@ function flushSegment(
     writeStartPendingSuspenseBoundary(
       destination,
       request.renderState,
-      boundary.id,
+      boundary.rootSegmentID,
     );
 
     // Flush the fallback.
@@ -3829,7 +3803,7 @@ function flushClientRenderedBoundary(
     destination,
     request.resumableState,
     request.renderState,
-    boundary.id,
+    boundary.rootSegmentID,
     boundary.errorDigest,
     boundary.errorMessage,
     boundary.errorComponentStack,
@@ -3882,7 +3856,6 @@ function flushCompletedBoundary(
     destination,
     request.resumableState,
     request.renderState,
-    boundary.id,
     boundary.rootSegmentID,
     boundary.resources,
   );

filename: packages/react-server/src/forks/ReactFizzConfig.custom.js
patch: @@ -32,7 +32,6 @@ export opaque type RenderState = mixed;
 export opaque type ResumableState = mixed;
 export opaque type BoundaryResources = mixed;
 export opaque type FormatContext = mixed;
-export opaque type SuspenseBoundaryID = mixed;
 export type {TransitionStatus};
 
 export const isPrimaryRenderer = false;
@@ -41,9 +40,6 @@ export const supportsRequestStorage = false;
 export const requestStorage: AsyncLocalStorage<Request> = (null: any);
 
 export const getChildFormatContext = $$$config.getChildFormatContext;
-export const UNINITIALIZED_SUSPENSE_BOUNDARY_ID =
-  $$$config.UNINITIALIZED_SUSPENSE_BOUNDARY_ID;
-export const assignSuspenseBoundaryID = $$$config.assignSuspenseBoundaryID;
 export const makeId = $$$config.makeId;
 export const pushTextInstance = $$$config.pushTextInstance;
 export const pushStartInstance = $$$config.pushStartInstance;