Add separator comment between text nodes (#21099)

sebmarkbage · web-flow · commit 6a589ad711db · 2021-03-25T08:48:39.000-07:00
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.
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('<');

Original file line number	Diff line number	Diff line change
`@@ -104,6 +104,8 @@ export function pushEmpty(`
`104`	`104`	`}`
`105`	`105`	`}`
`106`	`106`
	`107`	`+const textSeparator = stringToPrecomputedChunk('<!-- -->');`
	`108`	`+`
`107`	`109`	`export function pushTextInstance(`
`108`	`110`	`target: Array<Chunk \| PrecomputedChunk>,`
`109`	`111`	`text: string,`
`@@ -113,7 +115,12 @@ export function pushTextInstance(`
`113`	`115`	`if (assignID !== null) {`
`114`	`116`	`pushDummyNodeWithID(target, responseState, assignID);`
`115`	`117`	`}`
`116`		`- target.push(stringToChunk(encodeHTMLTextNode(text)));`
	`118`	`+ if (text === '') {`
	`119`	`+ // Empty text doesn't have a DOM node representation and the hydration is aware of this.`
	`120`	`+ return;`
	`121`	`+ }`
	`122`	`+ // TODO: Avoid adding a text separator in common cases.`
	`123`	`+ target.push(stringToChunk(encodeHTMLTextNode(text)), textSeparator);`
`117`	`124`	`}`
`118`	`125`
`119`	`126`	`const startTag1 = stringToPrecomputedChunk('<');`