Add separator comment between text nodes (facebook#21099)

This is needed to avoid mutating the DOM during hydration. This *always* adds it even when it's just text children. We need to avoid this overhead but it's a somewhat tricky problem to solve so we defer the optimization to later.
koto · Jun 15, 2021 · c4213f7 · c4213f7
1 parent 3aa29ae
commit c4213f7
Show file tree

Hide file tree

Showing 3 changed files with 19 additions and 8 deletions.
diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzServerBrowser-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzServerBrowser-test.js
@@ -55,7 +55,7 @@ describe('ReactDOMFizzServer', () => {
       <div>hello world</div>,
     );
     const result = await readResult(stream);
-    expect(result).toBe('<div>hello world</div>');
+    expect(result).toMatchInlineSnapshot(`"<div>hello world<!-- --></div>"`);
   });
 
   // @gate experimental
@@ -93,7 +93,9 @@ describe('ReactDOMFizzServer', () => {
     expect(isComplete).toBe(true);
 
     const result = await readResult(stream);
-    expect(result).toBe('<div><!--$-->Done<!--/$--></div>');
+    expect(result).toMatchInlineSnapshot(
+      `"<div><!--$-->Done<!-- --><!--/$--></div>"`,
+    );
   });
 
   // @gate experimental

diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzServerNode-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzServerNode-test.js
@@ -65,7 +65,9 @@ describe('ReactDOMFizzServer', () => {
     );
     startWriting();
     jest.runAllTimers();
-    expect(output.result).toBe('<div>hello world</div>');
+    expect(output.result).toMatchInlineSnapshot(
+      `"<div>hello world<!-- --></div>"`,
+    );
   });
 
   // @gate experimental
@@ -81,8 +83,8 @@ describe('ReactDOMFizzServer', () => {
       '<!doctype html><html><head><title>test</title><head><body>';
     // Then React starts writing.
     startWriting();
-    expect(output.result).toBe(
-      '<!doctype html><html><head><title>test</title><head><body><div>hello world</div>',
+    expect(output.result).toMatchInlineSnapshot(
+      `"<!doctype html><html><head><title>test</title><head><body><div>hello world<!-- --></div>"`,
     );
   });
 
@@ -129,8 +131,8 @@ describe('ReactDOMFizzServer', () => {
       '<!doctype html><html><head><title>test</title><head><body>';
     // Then React starts writing.
     startWriting();
-    expect(output.result).toBe(
-      '<!doctype html><html><head><title>test</title><head><body><div><!--$-->Done<!--/$--></div>',
+    expect(output.result).toMatchInlineSnapshot(
+      `"<!doctype html><html><head><title>test</title><head><body><div><!--$-->Done<!-- --><!--/$--></div>"`,
     );
   });
 

diff --git a/packages/react-dom/src/server/ReactDOMServerFormatConfig.js b/packages/react-dom/src/server/ReactDOMServerFormatConfig.js
@@ -104,6 +104,8 @@ export function pushEmpty(
   }
 }
 
+const textSeparator = stringToPrecomputedChunk('<!-- -->');
+
 export function pushTextInstance(
   target: Array<Chunk | PrecomputedChunk>,
   text: string,
@@ -113,7 +115,12 @@ export function pushTextInstance(
   if (assignID !== null) {
     pushDummyNodeWithID(target, responseState, assignID);
   }
-  target.push(stringToChunk(encodeHTMLTextNode(text)));
+  if (text === '') {
+    // 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);
 }
 
 const startTag1 = stringToPrecomputedChunk('<');