refactor to move embed status to the segment itself

packages/react-dom/src/server/ReactDOMServerFormatConfig.js

@@ -279,83 +279,35 @@ function encodeHTMLTextNode(text: string): string {
 }
 
 const textSeparator = stringToPrecomputedChunk('<!-- -->');
-let lastPushedText = false;
 
 export function pushTextInstance(
   target: Array<Chunk | PrecomputedChunk>,
   text: string,
   responseState: ResponseState,
-): void {
+  textEmbedded?: boolean,
+): boolean {
   if (text === '') {
     // Empty text doesn't have a DOM node representation and the hydration is aware of this.
-    return;
+    return textEmbedded;
   }
-  // We use a null charater to wrap every text chunk. we use this to boundary check whether we need
-  // to insert a Node textSeparator like <!-- --> when flushing segments between sets of chunks.
-  if (lastPushedText) {
+  if (textEmbedded) {
     target.push(textSeparator);
   }
-  lastPushedText = true;
   target.push(stringToChunk(encodeHTMLTextNode(text)));
+  return true;
 }
 
-// The Text Embedding flags allow the format config to know when to include or exclude
-// text separators between parent and child segments. We track the leading edge and
-// trailing edges separately however due to the fact that peeking ahead is usually
-// not possible we discern between knowing we need a separator at the leading edge and
-// assuming we might need one at the trailing edge. In the future we could get more
-// advanced with the tracking and drop the trailing edge in some cases when we know a segment
-// is followed by a Text or non-Text Node explicitly vs followed by another Segment
-export opaque type TextEmbedding = number;
-
-const NO_TEXT_EMBED = /*                    */ 0b0000;
-
-const LEADING_SEPARATOR_NEEDED = /*         */ 0b0011;
-const LEADING_TEXT_EMBED_KNOWN = /*         */ 0b0001;
-// const LEADING_TEXT_EMBED_POSSIBLE = /*   */ 0b0010;
-
-const TRAILING_SEPARATOR_NEEDED = /*        */ 0b1100;
-// const TRAILING_TEXT_EMBED_KNOWN = /*     */ 0b0100;
-const TRAILING_TEXT_EMBED_POSSIBLE = /*     */ 0b1000;
-
-// Suspense boundaries always emit a comment node at the leading and trailing edge and thus need
-// no additional separators. we also use this for the root segment even though it isn't a Boundary
-// because it cannot have pre or post text and thus matches the same embedding semantics or Boundaries
-export function textEmbeddingForBoundarySegment(): TextEmbedding {
-  lastPushedText = false;
-  return NO_TEXT_EMBED;
-}
-
-// Segments that are not the boundary segment can be truly embedded in Text. we determine the leading edge
-// based on what was last pushed. We cannot know the trailing edge so we conservatively assume we are embedded there
-export function textEmbeddingForSegment(): TextEmbedding {
-  let embedding = TRAILING_TEXT_EMBED_POSSIBLE;
-  embedding |= lastPushedText ? LEADING_TEXT_EMBED_KNOWN : NO_TEXT_EMBED;
-
-  lastPushedText = false;
-  return embedding;
-}
-
-// If a Segment is going to be flushed later than it's parent text separators arent needed
-// because the DOM patch will leavee the adjacent text as separate nodes
-export function textEmbeddingForDelayedSegment(): TextEmbedding {
-  return NO_TEXT_EMBED;
-}
-
-// Called when a segment is about to be rendered by a Task
-export function prepareForSegment(textEmbedding: TextEmbedding) {
-  lastPushedText = textEmbedding & LEADING_SEPARATOR_NEEDED;
-}
-
-// Called when a segment is exhaustively rendered
-export function finalizeForSegment(
+// Called when Fizz is done with a Segment. Currently the only purpose is to conditionally
+// emit a text separator when we don't know for sure it is safe to omit
+export function pushSegmentFinale(
   target: Array<Chunk | PrecomputedChunk>,
-  textEmbedding: TextEmbedding,
-) {
-  if (lastPushedText && textEmbedding & TRAILING_SEPARATOR_NEEDED) {
+  responseState: ResponseState,
+  lastPushedText: Boolean,
+  textEmbedded: Boolean,
+): void {
+  if (lastPushedText && textEmbedded) {
     target.push(textSeparator);
   }
-  lastPushedText = false;
 }
 
 const styleNameCache: Map<string, PrecomputedChunk> = new Map();
@@ -1389,7 +1341,6 @@ export function pushStartInstance(
   responseState: ResponseState,
   formatContext: FormatContext,
 ): ReactNodeList {
-  lastPushedText = false;
   if (__DEV__) {
     validateARIAProperties(type, props);
     validateInputProperties(type, props);
@@ -1502,7 +1453,6 @@ export function pushEndInstance(
   type: string,
   props: Object,
 ): void {
-  lastPushedText = false;
   switch (type) {
     // Omitted close tags
     // TODO: Instead of repeating this switch we could try to pass a flag from above.

packages/react-dom/src/server/ReactDOMServerLegacyFormatConfig.js

@@ -87,6 +87,7 @@ export {
   pushEndInstance,
   pushStartCompletedSuspenseBoundary,
   pushEndCompletedSuspenseBoundary,
+  pushSegmentFinale,
   writeStartSegment,
   writeEndSegment,
   writeCompletedSegmentInstruction,
@@ -96,11 +97,6 @@ export {
   writeEndPendingSuspenseBoundary,
   writePlaceholder,
   writeCompletedRoot,
-  textEmbeddingForBoundarySegment,
-  textEmbeddingForSegment,
-  textEmbeddingForDelayedSegment,
-  prepareForSegment,
-  finalizeForSegment,
 } from './ReactDOMServerFormatConfig';
 
 import {stringToChunk} from 'react-server/src/ReactServerStreamConfig';
@@ -111,11 +107,13 @@ export function pushTextInstance(
   target: Array<Chunk | PrecomputedChunk>,
   text: string,
   responseState: ResponseState,
-): void {
+  textEmbedded: boolean,
+): boolean {
   if (responseState.generateStaticMarkup) {
     target.push(stringToChunk(escapeTextForBrowser(text)));
+    return false;
   } else {
-    pushTextInstanceImpl(target, text, responseState);
+    return pushTextInstanceImpl(target, text, responseState, textEmbedded);
   }
 }
 

packages/react-native-renderer/src/server/ReactNativeServerFormatConfig.js

@@ -155,12 +155,8 @@ export function pushEndInstance(
 ): void {
   target.push(END);
 }
-export opaque type TextEmbedding = void;
-export function textEmbeddingForBoundarySegment() {}
-export function textEmbeddingForSegment() {}
-export function textEmbeddingForDelayedSegment() {}
-export function prepareForSegment() {}
-export function finalizeForSegment() {}
+
+export function pushSegmentFinale() {}
 
 export function writeCompletedRoot(
   destination: Destination,

packages/react-noop-renderer/src/ReactNoopServer.js

@@ -128,14 +128,7 @@ const ReactNoopServer = ReactFizzServer({
     target.push(POP);
   },
 
-  textEmbeddingForBoundarySegment() {
-    return null;
-  },
-  textEmbeddingForSegment() {
-    return null;
-  },
-  prepareForSegment() {},
-  finalizeForSegment() {},
+  pushSegmentFinale() {},
 
   writeCompletedRoot(
     destination: Destination,

packages/react-server/src/ReactFizzServer.js

@@ -26,7 +26,6 @@ import type {
 import type {ContextSnapshot} from './ReactFizzNewContext';
 import type {ComponentStackNode} from './ReactFizzComponentStack';
 import type {TreeContext} from './ReactFizzTreeContext';
-import type {TextEmbedding} from './ReactServerFormatConfig';
 
 import {
   scheduleWork,
@@ -57,6 +56,7 @@ import {
   pushEndInstance,
   pushStartCompletedSuspenseBoundary,
   pushEndCompletedSuspenseBoundary,
+  pushSegmentFinale,
   UNINITIALIZED_SUSPENSE_BOUNDARY_ID,
   assignSuspenseBoundaryID,
   getChildFormatContext,
@@ -175,8 +175,9 @@ type Segment = {
   formatContext: FormatContext,
   // If this segment represents a fallback, this is the content that will replace that fallback.
   +boundary: null | SuspenseBoundary,
-  // opaque data that tells the formatter uses to modify output based on how the Segment is embedded in other output
-  textEmbedding: TextEmbedding,
+  // used to discern when text separator boundaries are needed
+  lastPushedText: boolean,
+  textEmbedded: boolean,
 };
 
 const OPEN = 0;
@@ -280,7 +281,9 @@ export function createRequest(
     0,
     null,
     rootFormatContext,
-    textEmbeddingForBoundarySegment(),
+    // Root segments are never embedded in Text on either edge
+    false,
+    false,
   );
   // There is no parent so conceptually, we're unblocked to flush this segment.
   rootSegment.parentFlushed = true;
@@ -360,7 +363,8 @@ function createPendingSegment(
   index: number,
   boundary: null | SuspenseBoundary,
   formatContext: FormatContext,
-  textEmbedding: TextEmbedding,
+  lastPushedText: boolean,
+  textEmbedded: boolean,
 ): Segment {
   return {
     status: PENDING,
@@ -371,7 +375,8 @@ function createPendingSegment(
     children: [],
     formatContext,
     boundary,
-    textEmbedding,
+    lastPushedText,
+    textEmbedded,
   };
 }
 
@@ -470,23 +475,28 @@ function renderSuspenseBoundary(
   const newBoundary = createSuspenseBoundary(request, fallbackAbortSet);
   const insertionIndex = parentSegment.chunks.length;
   // The children of the boundary segment is actually the fallback.
-  const textEmbedding = textEmbeddingForBoundarySegment();
   const boundarySegment = createPendingSegment(
     request,
     insertionIndex,
     newBoundary,
     parentSegment.formatContext,
-    textEmbedding,
+    // boundaries never require text embedding at their edges because comment nodes bound them
+    false,
+    false,
   );
   parentSegment.children.push(boundarySegment);
+  // The parentSegment has a child Segment at this index so we reset the lastPushedText marker on the parent
+  parentSegment.lastPushedText = false;
 
   // This segment is the actual child content. We can start rendering that immediately.
   const contentRootSegment = createPendingSegment(
     request,
     0,
     null,
     parentSegment.formatContext,
-    textEmbedding,
+    // boundaries never require text embedding at their edges because comment nodes bound them
+    false,
+    false,
   );
   // 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.
@@ -504,11 +514,12 @@ function renderSuspenseBoundary(
   task.blockedSegment = contentRootSegment;
   try {
     // We use the safe form because we don't handle suspending here. Only error handling.
-    prepareForSegment(contentRootSegment.textEmbedding);
     renderNode(request, task, content);
-    finalizeForSegment(
+    pushSegmentFinale(
       contentRootSegment.chunks,
-      contentRootSegment.textEmbedding,
+      request.responseState,
+      contentRootSegment.lastPushedText,
+      contentRootSegment.textEmbedded,
     );
     contentRootSegment.status = COMPLETED;
     queueCompletedSegment(newBoundary, contentRootSegment);
@@ -585,15 +596,18 @@ function renderHostElement(
     request.responseState,
     segment.formatContext,
   );
+  segment.lastPushedText = false;
   const prevContext = segment.formatContext;
   segment.formatContext = getChildFormatContext(prevContext, type, props);
   // We use the non-destructive form because if something suspends, we still
   // need to pop back up and finish this subtree of HTML.
   renderNode(request, task, children);
+
   // We expect that errors will fatal the whole task and that we don't need
   // the correct context. Therefore this is not in a finally.
   segment.formatContext = prevContext;
   pushEndInstance(segment.chunks, type, props);
+  segment.lastPushedText = false;
   popComponentStackInDEV(task);
 }
 
@@ -1240,15 +1254,23 @@ function renderNodeDestructive(
   }
 
   if (typeof node === 'string') {
-    pushTextInstance(task.blockedSegment.chunks, node, request.responseState);
+    let segment = task.blockedSegment;
+    segment.lastPushedText = pushTextInstance(
+      task.blockedSegment.chunks,
+      node,
+      request.responseState,
+      segment.lastPushedText,
+    );
     return;
   }
 
   if (typeof node === 'number') {
-    pushTextInstance(
+    let segment = task.blockedSegment;
+    segment.lastPushedText = pushTextInstance(
       task.blockedSegment.chunks,
       '' + node,
       request.responseState,
+      segment.lastPushedText,
     );
     return;
   }
@@ -1287,15 +1309,19 @@ function spawnNewSuspendedTask(
   // Something suspended, we'll need to create a new segment and resolve it later.
   const segment = task.blockedSegment;
   const insertionIndex = segment.chunks.length;
-  const textEmbedding = textEmbeddingForSegment();
   const newSegment = createPendingSegment(
     request,
     insertionIndex,
     null,
     segment.formatContext,
-    textEmbedding,
+    // Adopt the parent segment's leading text embed
+    segment.lastPushedText,
+    // Assume we are text embedded at the trailing edge
+    true,
   );
   segment.children.push(newSegment);
+  // Reset lastPushedText for current Segment since the new Segment "consumed" it
+  segment.lastPushedText = false;
   const newTask = createTask(
     request,
     task.node,
@@ -1570,9 +1596,13 @@ function retryTask(request: Request, task: Task): void {
   try {
     // 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.
-    prepareForSegment(segment.textEmbedding);
     renderNodeDestructive(request, task, task.node);
-    finalizeForSegment(segment.chunks, segment.textEmbedding);
+    pushSegmentFinale(
+      segment.chunks,
+      request.responseState,
+      segment.lastPushedText,
+      segment.textEmbedded,
+    );
 
     task.abortSet.delete(task);
     segment.status = COMPLETED;
@@ -1653,7 +1683,9 @@ function flushSubtree(
       // We're emitting a placeholder for this segment to be filled in later.
       // Therefore we'll need to assign it an ID - to refer to it by.
       const segmentID = (segment.id = request.nextSegmentId++);
-      segment.textEmbedding = textEmbeddingForDelayedSegment();
+      // When this segment finally completes it won't be embedded in text since it will flush separately
+      segment.lastPushedText = false;
+      segment.textEmbedded = false;
       return writePlaceholder(destination, request.responseState, segmentID);
     }
     case COMPLETED: {

packages/react-server/src/forks/ReactServerFormatConfig.custom.js

@@ -43,13 +43,7 @@ export const pushStartCompletedSuspenseBoundary =
   $$$hostConfig.pushStartCompletedSuspenseBoundary;
 export const pushEndCompletedSuspenseBoundary =
   $$$hostConfig.pushEndCompletedSuspenseBoundary;
-export const textEmbeddingForBoundarySegment =
-  $$$hostConfig.textEmbeddingForBoundarySegment;
-export const textEmbeddingForDelayedSegment =
-  $$$hostConfig.textEmbeddingForDelayedSegment;
-export const textEmbeddingForSegment = $$$hostConfig.textEmbeddingForSegment;
-export const prepareForSegment = $$$hostConfig.prepareForSegment;
-export const finalizeForSegment = $$$hostConfig.finalizeForSegment;
+export const pushSegmentFinale = $$$hostConfig.pushSegmentFinale;
 export const writeCompletedRoot = $$$hostConfig.writeCompletedRoot;
 export const writePlaceholder = $$$hostConfig.writePlaceholder;
 export const writeStartCompletedSuspenseBoundary =