[Fizz] Improve text separator byte efficiency

packages/react-dom/src/__tests__/ReactDOMServerIntegrationElements-test.js

@@ -101,7 +101,7 @@ describe('ReactDOMServerIntegration', () => {
         ) {
           // For plain server markup result we have comments between.
           // If we're able to hydrate, they remain.
-          expect(e.childNodes.length).toBe(render === streamRender ? 6 : 5);
+          expect(e.childNodes.length).toBe(5);
           expectTextNode(e.childNodes[0], ' ');
           expectTextNode(e.childNodes[2], ' ');
           expectTextNode(e.childNodes[4], ' ');
@@ -119,8 +119,8 @@ describe('ReactDOMServerIntegration', () => {
             Text<span>More Text</span>
           </div>,
         );
-        expect(e.childNodes.length).toBe(render === streamRender ? 3 : 2);
-        const spanNode = e.childNodes[render === streamRender ? 2 : 1];
+        expect(e.childNodes.length).toBe(2);
+        const spanNode = e.childNodes[1];
         expectTextNode(e.childNodes[0], 'Text');
         expect(spanNode.tagName).toBe('SPAN');
         expect(spanNode.childNodes.length).toBe(1);
@@ -147,19 +147,19 @@ describe('ReactDOMServerIntegration', () => {
       itRenders('a custom element with text', async render => {
         const e = await render(<custom-element>Text</custom-element>);
         expect(e.tagName).toBe('CUSTOM-ELEMENT');
-        expect(e.childNodes.length).toBe(render === streamRender ? 2 : 1);
+        expect(e.childNodes.length).toBe(1);
         expectNode(e.firstChild, TEXT_NODE_TYPE, 'Text');
       });
 
       itRenders('a leading blank child with a text sibling', async render => {
         const e = await render(<div>{''}foo</div>);
-        expect(e.childNodes.length).toBe(render === streamRender ? 2 : 1);
+        expect(e.childNodes.length).toBe(1);
         expectTextNode(e.childNodes[0], 'foo');
       });
 
       itRenders('a trailing blank child with a text sibling', async render => {
         const e = await render(<div>foo{''}</div>);
-        expect(e.childNodes.length).toBe(render === streamRender ? 2 : 1);
+        expect(e.childNodes.length).toBe(1);
         expectTextNode(e.childNodes[0], 'foo');
       });
 
@@ -176,7 +176,7 @@ describe('ReactDOMServerIntegration', () => {
           render === streamRender
         ) {
           // In the server render output there's a comment between them.
-          expect(e.childNodes.length).toBe(render === streamRender ? 4 : 3);
+          expect(e.childNodes.length).toBe(3);
           expectTextNode(e.childNodes[0], 'foo');
           expectTextNode(e.childNodes[2], 'bar');
         } else {
@@ -203,7 +203,7 @@ describe('ReactDOMServerIntegration', () => {
             render === streamRender
           ) {
             // In the server render output there's a comment between them.
-            expect(e.childNodes.length).toBe(render === streamRender ? 6 : 5);
+            expect(e.childNodes.length).toBe(5);
             expectTextNode(e.childNodes[0], 'a');
             expectTextNode(e.childNodes[2], 'b');
             expectTextNode(e.childNodes[4], 'c');
@@ -240,7 +240,11 @@ describe('ReactDOMServerIntegration', () => {
             e
           </div>,
         );
-        if (render === serverRender || render === clientRenderOnServerString) {
+        if (
+          render === serverRender ||
+          render === streamRender ||
+          render === clientRenderOnServerString
+        ) {
           // In the server render output there's comments between text nodes.
           expect(e.childNodes.length).toBe(5);
           expectTextNode(e.childNodes[0], 'a');
@@ -249,15 +253,6 @@ describe('ReactDOMServerIntegration', () => {
           expectTextNode(e.childNodes[3].childNodes[0], 'c');
           expectTextNode(e.childNodes[3].childNodes[2], 'd');
           expectTextNode(e.childNodes[4], 'e');
-        } else if (render === streamRender) {
-          // In the server render output there's comments after each text node.
-          expect(e.childNodes.length).toBe(7);
-          expectTextNode(e.childNodes[0], 'a');
-          expectTextNode(e.childNodes[2], 'b');
-          expect(e.childNodes[4].childNodes.length).toBe(4);
-          expectTextNode(e.childNodes[4].childNodes[0], 'c');
-          expectTextNode(e.childNodes[4].childNodes[2], 'd');
-          expectTextNode(e.childNodes[5], 'e');
         } else {
           expect(e.childNodes.length).toBe(4);
           expectTextNode(e.childNodes[0], 'a');
@@ -296,7 +291,7 @@ describe('ReactDOMServerIntegration', () => {
           render === streamRender
         ) {
           // In the server markup there's a comment between.
-          expect(e.childNodes.length).toBe(render === streamRender ? 4 : 3);
+          expect(e.childNodes.length).toBe(3);
           expectTextNode(e.childNodes[0], 'foo');
           expectTextNode(e.childNodes[2], '40');
         } else {
@@ -335,13 +330,13 @@ describe('ReactDOMServerIntegration', () => {
 
       itRenders('null children as blank', async render => {
         const e = await render(<div>{null}foo</div>);
-        expect(e.childNodes.length).toBe(render === streamRender ? 2 : 1);
+        expect(e.childNodes.length).toBe(1);
         expectTextNode(e.childNodes[0], 'foo');
       });
 
       itRenders('false children as blank', async render => {
         const e = await render(<div>{false}foo</div>);
-        expect(e.childNodes.length).toBe(render === streamRender ? 2 : 1);
+        expect(e.childNodes.length).toBe(1);
         expectTextNode(e.childNodes[0], 'foo');
       });
 
@@ -353,7 +348,7 @@ describe('ReactDOMServerIntegration', () => {
             {false}
           </div>,
         );
-        expect(e.childNodes.length).toBe(render === streamRender ? 2 : 1);
+        expect(e.childNodes.length).toBe(1);
         expectTextNode(e.childNodes[0], 'foo');
       });
 
@@ -740,10 +735,10 @@ describe('ReactDOMServerIntegration', () => {
             </div>,
           );
           expect(e.id).toBe('parent');
-          expect(e.childNodes.length).toBe(render === streamRender ? 4 : 3);
+          expect(e.childNodes.length).toBe(3);
           const child1 = e.childNodes[0];
           const textNode = e.childNodes[1];
-          const child2 = e.childNodes[render === streamRender ? 3 : 2];
+          const child2 = e.childNodes[2];
           expect(child1.id).toBe('child1');
           expect(child1.childNodes.length).toBe(0);
           expectTextNode(textNode, ' ');
@@ -757,10 +752,10 @@ describe('ReactDOMServerIntegration', () => {
         async render => {
           // prettier-ignore
           const e = await render(<div id="parent">  <div id="child" />   </div>); // eslint-disable-line no-multi-spaces
-          expect(e.childNodes.length).toBe(render === streamRender ? 5 : 3);
+          expect(e.childNodes.length).toBe(3);
           const textNode1 = e.childNodes[0];
-          const child = e.childNodes[render === streamRender ? 2 : 1];
-          const textNode2 = e.childNodes[render === streamRender ? 3 : 2];
+          const child = e.childNodes[1];
+          const textNode2 = e.childNodes[2];
           expect(e.id).toBe('parent');
           expectTextNode(textNode1, '  ');
           expect(child.id).toBe('child');
@@ -783,9 +778,7 @@ describe('ReactDOMServerIntegration', () => {
         ) {
           // For plain server markup result we have comments between.
           // If we're able to hydrate, they remain.
-          expect(parent.childNodes.length).toBe(
-            render === streamRender ? 6 : 5,
-          );
+          expect(parent.childNodes.length).toBe(5);
           expectTextNode(parent.childNodes[0], 'a');
           expectTextNode(parent.childNodes[2], 'b');
           expectTextNode(parent.childNodes[4], 'c');
@@ -817,7 +810,7 @@ describe('ReactDOMServerIntegration', () => {
           render === clientRenderOnServerString ||
           render === streamRender
         ) {
-          expect(e.childNodes.length).toBe(render === streamRender ? 4 : 3);
+          expect(e.childNodes.length).toBe(3);
           expectTextNode(e.childNodes[0], '<span>Text1&quot;</span>');
           expectTextNode(e.childNodes[2], '<span>Text2&quot;</span>');
         } else {
@@ -868,7 +861,7 @@ describe('ReactDOMServerIntegration', () => {
           );
           if (render === serverRender || render === streamRender) {
             // We have three nodes because there is a comment between them.
-            expect(e.childNodes.length).toBe(render === streamRender ? 4 : 3);
+            expect(e.childNodes.length).toBe(3);
             // Everything becomes LF when parsed from server HTML.
             // Null character is ignored.
             expectNode(e.childNodes[0], TEXT_NODE_TYPE, 'foo\nbar');

packages/react-dom/src/__tests__/ReactDOMUseId-test.js

@@ -343,7 +343,6 @@ describe('useId', () => {
         id="container"
       >
         :R0:, :R0H1:, :R0H2:
-        <!-- -->
       </div>
     `);
   });
@@ -369,7 +368,6 @@ describe('useId', () => {
         id="container"
       >
         :R0:
-        <!-- -->
       </div>
     `);
   });

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

@@ -279,6 +279,7 @@ function encodeHTMLTextNode(text: string): string {
 }
 
 const textSeparator = stringToPrecomputedChunk('<!-- -->');
+let lastPushedText = false;
 
 export function pushTextInstance(
   target: Array<Chunk | PrecomputedChunk>,
@@ -289,8 +290,72 @@ export function pushTextInstance(
     // Empty text doesn't have a DOM node representation and the hydration is aware of this.
     return;
   }
-  // TODO: Avoid adding a text separator in common cases.
-  target.push(stringToChunk(encodeHTMLTextNode(text)), textSeparator);
+  // 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) {
+    target.push(textSeparator);
+  }
+  lastPushedText = true;
+  target.push(stringToChunk(encodeHTMLTextNode(text)));
+}
+
+// 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(
+  target: Array<Chunk | PrecomputedChunk>,
+  textEmbedding: TextEmbedding,
+) {
+  if (lastPushedText && textEmbedding & TRAILING_SEPARATOR_NEEDED) {
+    target.push(textSeparator);
+  }
+  lastPushedText = false;
 }
 
 const styleNameCache: Map<string, PrecomputedChunk> = new Map();
@@ -1324,6 +1389,7 @@ export function pushStartInstance(
   responseState: ResponseState,
   formatContext: FormatContext,
 ): ReactNodeList {
+  lastPushedText = false;
   if (__DEV__) {
     validateARIAProperties(type, props);
     validateInputProperties(type, props);
@@ -1436,6 +1502,7 @@ 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

@@ -75,6 +75,7 @@ export function createRootFormatContext(): FormatContext {
 export type {
   FormatContext,
   SuspenseBoundaryID,
+  TextEmbedding,
 } from './ReactDOMServerFormatConfig';
 
 export {
@@ -95,6 +96,11 @@ export {
   writeEndPendingSuspenseBoundary,
   writePlaceholder,
   writeCompletedRoot,
+  textEmbeddingForBoundarySegment,
+  textEmbeddingForSegment,
+  textEmbeddingForDelayedSegment,
+  prepareForSegment,
+  finalizeForSegment,
 } from './ReactDOMServerFormatConfig';
 
 import {stringToChunk} from 'react-server/src/ReactServerStreamConfig';

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

@@ -155,6 +155,12 @@ 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 writeCompletedRoot(
   destination: Destination,

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

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

packages/react-server-dom-relay/src/__tests__/ReactDOMServerFB-test.internal.js

@@ -93,9 +93,7 @@ describe('ReactDOMServerFB', () => {
     await jest.runAllTimers();
 
     const result = readResult(stream);
-    expect(result).toMatchInlineSnapshot(
-      `"<div><!--$-->Done<!-- --><!--/$--></div>"`,
-    );
+    expect(result).toMatchInlineSnapshot(`"<div><!--$-->Done<!--/$--></div>"`);
   });
 
   it('should throw an error when an error is thrown at the root', () => {