[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/ReactDOMLegacyServerStreamConfig.js

@@ -24,7 +24,6 @@ export function flushBuffered(destination: Destination) {}
 
 export function beginWriting(destination: Destination) {}
 
-let prevWasCommentSegmenter = false;
 export function writeChunk(
   destination: Destination,
   chunk: Chunk | PrecomputedChunk,
@@ -36,16 +35,6 @@ export function writeChunkAndReturn(
   destination: Destination,
   chunk: Chunk | PrecomputedChunk,
 ): boolean {
-  if (prevWasCommentSegmenter) {
-    prevWasCommentSegmenter = false;
-    if (chunk[0] !== '<') {
-      destination.push('<!-- -->');
-    }
-  }
-  if (chunk === '<!-- -->') {
-    prevWasCommentSegmenter = true;
-    return true;
-  }
   return destination.push(chunk);
 }
 

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

@@ -284,13 +284,30 @@ 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;
+  }
+  if (textEmbedded) {
+    target.push(textSeparator);
+  }
+  target.push(stringToChunk(encodeHTMLTextNode(text)));
+  return true;
+}
+
+// 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>,
+  responseState: ResponseState,
+  lastPushedText: boolean,
+  textEmbedded: boolean,
+): void {
+  if (lastPushedText && textEmbedded) {
+    target.push(textSeparator);
   }
-  // TODO: Avoid adding a text separator in common cases.
-  target.push(stringToChunk(encodeHTMLTextNode(text)), textSeparator);
 }
 
 const styleNameCache: Map<string, PrecomputedChunk> = new Map();

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

@@ -12,6 +12,7 @@ import type {FormatContext} from './ReactDOMServerFormatConfig';
 import {
   createResponseState as createResponseStateImpl,
   pushTextInstance as pushTextInstanceImpl,
+  pushSegmentFinale as pushSegmentFinaleImpl,
   writeStartCompletedSuspenseBoundary as writeStartCompletedSuspenseBoundaryImpl,
   writeStartClientRenderedSuspenseBoundary as writeStartClientRenderedSuspenseBoundaryImpl,
   writeEndCompletedSuspenseBoundary as writeEndCompletedSuspenseBoundaryImpl,
@@ -105,11 +106,31 @@ 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 {
+    return pushTextInstanceImpl(target, text, responseState, textEmbedded);
+  }
+}
+
+export function pushSegmentFinale(
+  target: Array<Chunk | PrecomputedChunk>,
+  responseState: ResponseState,
+  lastPushedText: boolean,
+  textEmbedded: boolean,
+): void {
+  if (responseState.generateStaticMarkup) {
+    return;
   } else {
-    pushTextInstanceImpl(target, text, responseState);
+    return pushSegmentFinaleImpl(
+      target,
+      responseState,
+      lastPushedText,
+      textEmbedded,
+    );
   }
 }
 

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

@@ -122,14 +122,17 @@ export function pushTextInstance(
   target: Array<Chunk | PrecomputedChunk>,
   text: string,
   responseState: ResponseState,
-): void {
+  // This Renderer does not use this argument
+  textEmbedded: boolean,
+): boolean {
   target.push(
     INSTANCE,
     RAW_TEXT, // Type
     END, // Null terminated type string
     // TODO: props { text: text }
     END, // End of children
   );
+  return false;
 }
 
 export function pushStartInstance(
@@ -156,6 +159,14 @@ export function pushEndInstance(
   target.push(END);
 }
 
+// In this Renderer this is a noop
+export function pushSegmentFinale(
+  target: Array<Chunk | PrecomputedChunk>,
+  responseState: ResponseState,
+  lastPushedText: boolean,
+  textEmbedded: boolean,
+): void {}
+
 export function writeCompletedRoot(
   destination: Destination,
   responseState: ResponseState,

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

@@ -98,12 +98,18 @@ const ReactNoopServer = ReactFizzServer({
     return null;
   },
 
-  pushTextInstance(target: Array<Uint8Array>, text: string): void {
+  pushTextInstance(
+    target: Array<Uint8Array>,
+    text: string,
+    responseState: ResponseState,
+    textEmbedded: boolean,
+  ): boolean {
     const textInstance: TextInstance = {
       text,
       hidden: false,
     };
     target.push(Buffer.from(JSON.stringify(textInstance), 'utf8'), POP);
+    return false;
   },
   pushStartInstance(
     target: Array<Uint8Array>,
@@ -128,6 +134,14 @@ const ReactNoopServer = ReactFizzServer({
     target.push(POP);
   },
 
+  // This is a noop in ReactNoop
+  pushSegmentFinale(
+    target: Array<Uint8Array>,
+    responseState: ResponseState,
+    lastPushedText: boolean,
+    textEmbedded: boolean,
+  ): void {},
+
   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', () => {