fix duplicate closing tags on non-void resources

facebook · Oct 22, 2022 · 252fb3b · 252fb3b
1 parent 9341775
commit 252fb3b
Show file tree

Hide file tree

Showing 7 changed files with 58 additions and 11 deletions.
diff --git a/packages/react-dom-bindings/src/server/ReactDOMServerFormatConfig.js b/packages/react-dom-bindings/src/server/ReactDOMServerFormatConfig.js
@@ -374,6 +374,11 @@ function encodeHTMLTextNode(text: string): string {
   return escapeTextForBrowser(text);
 }
 
+// This const is returned when a push is fully consumed as a Resource. The runtime
+// well compare it against the returned children and avoid pushing the end tag
+opaque type ResourceSentinel = mixed;
+export const RESOURCE_SENTINAL: ResourceSentinel = {};
+
 const textSeparator = stringToPrecomputedChunk('<!-- -->');
 
 export function pushTextInstance(
@@ -1155,7 +1160,7 @@ function pushMeta(
   props: Object,
   responseState: ResponseState,
   textEmbedded: boolean,
-): ReactNodeList {
+): ReactNodeList | ResourceSentinel {
   if (enableFloat && resourcesFromElement('meta', props)) {
     if (textEmbedded) {
       // This link follows text but we aren't writing a tag. while not as efficient as possible we need
@@ -1164,7 +1169,7 @@ function pushMeta(
     }
     // We have converted this link exclusively to a resource and no longer
     // need to emit it
-    return null;
+    return RESOURCE_SENTINAL;
   }
 
   return pushSelfClosing(target, props, 'meta', responseState);
@@ -1175,7 +1180,7 @@ function pushLink(
   props: Object,
   responseState: ResponseState,
   textEmbedded: boolean,
-): ReactNodeList {
+): ReactNodeList | ResourceSentinel {
   if (enableFloat && resourcesFromLink(props)) {
     if (textEmbedded) {
       // This link follows text but we aren't writing a tag. while not as efficient as possible we need
@@ -1184,7 +1189,7 @@ function pushLink(
     }
     // We have converted this link exclusively to a resource and no longer
     // need to emit it
-    return null;
+    return RESOURCE_SENTINAL;
   }
 
   return pushLinkImpl(target, props, responseState);
@@ -1298,11 +1303,11 @@ function pushStartTitle(
   target: Array<Chunk | PrecomputedChunk>,
   props: Object,
   responseState: ResponseState,
-): ReactNodeList {
+): ReactNodeList | ResourceSentinel {
   if (enableFloat && resourcesFromElement('title', props)) {
     // We have converted this link exclusively to a resource and no longer
     // need to emit it
-    return null;
+    return RESOURCE_SENTINAL;
   }
 
   return pushStartTitleImpl(target, props, responseState);
@@ -1415,7 +1420,7 @@ function pushStartScript(
   props: Object,
   responseState: ResponseState,
   textEmbedded: boolean,
-): ReactNodeList {
+): ReactNodeList | ResourceSentinel {
   if (enableFloat && resourcesFromScript(props)) {
     if (textEmbedded) {
       // This link follows text but we aren't writing a tag. while not as efficient as possible we need
@@ -1424,7 +1429,7 @@ function pushStartScript(
     }
     // We have converted this link exclusively to a resource and no longer
     // need to emit it
-    return null;
+    return RESOURCE_SENTINAL;
   }
 
   return pushStartGenericElement(target, props, 'script', responseState);
@@ -1652,7 +1657,7 @@ export function pushStartInstance(
   responseState: ResponseState,
   formatContext: FormatContext,
   textEmbedded: boolean,
-): ReactNodeList {
+): ReactNodeList | ResourceSentinel {
   if (__DEV__) {
     validateARIAProperties(type, props);
     validateInputProperties(type, props);
@@ -1767,6 +1772,10 @@ export function pushStartInstance(
   }
 }
 
+// function pushEndTitle(target: Array<Chunk | PrecomputedChunk>): void {
+//   if (enableFloat)
+// }
+
 const endTag1 = stringToPrecomputedChunk('</');
 const endTag2 = stringToPrecomputedChunk('>');
 

diff --git a/packages/react-dom-bindings/src/server/ReactDOMServerLegacyFormatConfig.js b/packages/react-dom-bindings/src/server/ReactDOMServerLegacyFormatConfig.js
@@ -87,6 +87,7 @@ export {
   UNINITIALIZED_SUSPENSE_BOUNDARY_ID,
   assignSuspenseBoundaryID,
   makeId,
+  RESOURCE_SENTINAL,
   pushStartInstance,
   pushEndInstance,
   pushStartCompletedSuspenseBoundary,

diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzServerBrowser-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzServerBrowser-test.js
@@ -499,7 +499,7 @@ describe('ReactDOMFizzServerBrowser', () => {
 
     const result = await readResult(stream);
     expect(result).toEqual(
-      '<!DOCTYPE html><html><head><title>foo</title></title></head><body>bar</body></html>',
+      '<!DOCTYPE html><html><head><title>foo</title></head><body>bar</body></html>',
     );
   });
 });
diff --git a/packages/react-dom/src/__tests__/ReactDOMFloat-test.js b/packages/react-dom/src/__tests__/ReactDOMFloat-test.js
@@ -382,6 +382,32 @@ describe('ReactDOMFloat', () => {
     );
   });
 
+  // @gate enableFloat
+  it('does not emit closing tags in out of order position when rendering a non-void resource type', async () => {
+    const chunks = [];
+
+    writable.on('data', chunk => {
+      chunks.push(chunk);
+    });
+
+    await actIntoEmptyDocument(() => {
+      const {pipe} = ReactDOMFizzServer.renderToPipeableStream(
+        <>
+          <title>foo</title>
+          <html>
+            <body>bar</body>
+          </html>
+          <script async={true} src="foo" />
+        </>,
+      );
+      pipe(writable);
+    });
+    expect(chunks).toEqual([
+      '<!DOCTYPE html><html><script async="" src="foo"></script><title>foo</title><body>bar',
+      '</body></html>',
+    ]);
+  });
+
   describe('HostResource', () => {
     // @gate enableFloat
     it('warns when you update props to an invalid type', async () => {

diff --git a/packages/react-native-renderer/src/server/ReactNativeServerFormatConfig.js b/packages/react-native-renderer/src/server/ReactNativeServerFormatConfig.js
@@ -121,6 +121,8 @@ export function makeId(
 
 const RAW_TEXT = stringToPrecomputedChunk('RCTRawText');
 
+export const RESOURCE_SENTINAL: mixed = {};
+
 export function pushTextInstance(
   target: Array<Chunk | PrecomputedChunk>,
   text: string,

diff --git a/packages/react-server/src/ReactFizzServer.js b/packages/react-server/src/ReactFizzServer.js
@@ -74,6 +74,7 @@ import {
   setCurrentlyRenderingBoundaryResourcesTarget,
   createResources,
   createBoundaryResources,
+  RESOURCE_SENTINAL,
 } from './ReactServerFormatConfig';
 import {
   constructClassInstance,
@@ -694,7 +695,7 @@ function renderHostElement(
   pushBuiltInComponentStackInDEV(task, type);
   const segment = task.blockedSegment;
 
-  const children = pushStartInstance(
+  const childrenOrResource = pushStartInstance(
     segment.chunks,
     request.preamble,
     type,
@@ -703,6 +704,13 @@ function renderHostElement(
     segment.formatContext,
     segment.lastPushedText,
   );
+  if (enableFloat && childrenOrResource === RESOURCE_SENTINAL) {
+    // this push did not actually write to segment chunks because the element
+    // was a Resource. We pop the stack and return early.
+    popComponentStackInDEV(task);
+    return;
+  }
+  const children: ReactNodeList = (childrenOrResource: any);
   segment.lastPushedText = false;
   const prevContext = segment.formatContext;
   segment.formatContext = getChildFormatContext(prevContext, type, props);

diff --git a/packages/react-server/src/forks/ReactServerFormatConfig.custom.js b/packages/react-server/src/forks/ReactServerFormatConfig.custom.js
@@ -82,3 +82,4 @@ export const createResources = $$$hostConfig.createResources;
 export const createBoundaryResources = $$$hostConfig.createBoundaryResources;
 export const setCurrentlyRenderingBoundaryResourcesTarget =
   $$$hostConfig.setCurrentlyRenderingBoundaryResourcesTarget;
+export const RESOURCE_SENTINAL = $$$hostConfig.RESOURCE_SENTINAL;