[Fizz][Float] <img> inside <picture> should not preload during SSR (

facebook#27346) img tags inside picture tags should not automatically be preloaded because usually the img is a fallback. We will consider a more comprehensive way of preloading picture tags which may require a technique like using an inline script to construct the image in the browser but for now we simply omit the preloads to avoid harming load times by loading fallbacks.
EdisonVan · Apr 15, 2024 · 2c3ae5e · 2c3ae5e
1 parent a364e60
commit 2c3ae5e
Show file tree

Hide file tree

Showing 2 changed files with 75 additions and 40 deletions.
diff --git a/packages/react-dom-bindings/src/server/ReactFizzConfigDOM.js b/packages/react-dom-bindings/src/server/ReactFizzConfigDOM.js
@@ -499,22 +499,26 @@ const HTML_COLGROUP_MODE = 8;
 
 type InsertionMode = 0 | 1 | 2 | 3 | 4 | 5 | 6 | 7 | 8;
 
+const NO_SCOPE = /*         */ 0b00;
+const NOSCRIPT_SCOPE = /*   */ 0b01;
+const PICTURE_SCOPE = /*    */ 0b10;
+
 // Lets us keep track of contextual state and pick it back up after suspending.
 export type FormatContext = {
   insertionMode: InsertionMode, // root/svg/html/mathml/table
   selectedValue: null | string | Array<string>, // the selected value(s) inside a <select>, or null outside <select>
-  noscriptTagInScope: boolean,
+  tagScope: number,
 };
 
 function createFormatContext(
   insertionMode: InsertionMode,
   selectedValue: null | string,
-  noscriptTagInScope: boolean,
+  tagScope: number,
 ): FormatContext {
   return {
     insertionMode,
     selectedValue,
-    noscriptTagInScope,
+    tagScope,
   };
 }
 
@@ -525,7 +529,7 @@ export function createRootFormatContext(namespaceURI?: string): FormatContext {
       : namespaceURI === 'http://www.w3.org/1998/Math/MathML'
       ? MATHML_MODE
       : ROOT_HTML_MODE;
-  return createFormatContext(insertionMode, null, false);
+  return createFormatContext(insertionMode, null, NO_SCOPE);
 }
 
 export function getChildFormatContext(
@@ -535,80 +539,70 @@ export function getChildFormatContext(
 ): FormatContext {
   switch (type) {
     case 'noscript':
-      return createFormatContext(HTML_MODE, null, true);
+      return createFormatContext(
+        HTML_MODE,
+        null,
+        parentContext.tagScope | NOSCRIPT_SCOPE,
+      );
     case 'select':
       return createFormatContext(
         HTML_MODE,
         props.value != null ? props.value : props.defaultValue,
-        parentContext.noscriptTagInScope,
+        parentContext.tagScope,
       );
     case 'svg':
+      return createFormatContext(SVG_MODE, null, parentContext.tagScope);
+    case 'picture':
       return createFormatContext(
-        SVG_MODE,
+        HTML_MODE,
         null,
-        parentContext.noscriptTagInScope,
+        parentContext.tagScope | PICTURE_SCOPE,
       );
     case 'math':
-      return createFormatContext(
-        MATHML_MODE,
-        null,
-        parentContext.noscriptTagInScope,
-      );
+      return createFormatContext(MATHML_MODE, null, parentContext.tagScope);
     case 'foreignObject':
-      return createFormatContext(
-        HTML_MODE,
-        null,
-        parentContext.noscriptTagInScope,
-      );
+      return createFormatContext(HTML_MODE, null, parentContext.tagScope);
     // Table parents are special in that their children can only be created at all if they're
     // wrapped in a table parent. So we need to encode that we're entering this mode.
     case 'table':
-      return createFormatContext(
-        HTML_TABLE_MODE,
-        null,
-        parentContext.noscriptTagInScope,
-      );
+      return createFormatContext(HTML_TABLE_MODE, null, parentContext.tagScope);
     case 'thead':
     case 'tbody':
     case 'tfoot':
       return createFormatContext(
         HTML_TABLE_BODY_MODE,
         null,
-        parentContext.noscriptTagInScope,
+        parentContext.tagScope,
       );
     case 'colgroup':
       return createFormatContext(
         HTML_COLGROUP_MODE,
         null,
-        parentContext.noscriptTagInScope,
+        parentContext.tagScope,
       );
     case 'tr':
       return createFormatContext(
         HTML_TABLE_ROW_MODE,
         null,
-        parentContext.noscriptTagInScope,
+        parentContext.tagScope,
       );
   }
   if (parentContext.insertionMode >= HTML_TABLE_MODE) {
     // Whatever tag this was, it wasn't a table parent or other special parent, so we must have
     // entered plain HTML again.
-    return createFormatContext(
-      HTML_MODE,
-      null,
-      parentContext.noscriptTagInScope,
-    );
+    return createFormatContext(HTML_MODE, null, parentContext.tagScope);
   }
   if (parentContext.insertionMode === ROOT_HTML_MODE) {
     if (type === 'html') {
       // We've emitted the root and is now in <html> mode.
-      return createFormatContext(HTML_HTML_MODE, null, false);
+      return createFormatContext(HTML_HTML_MODE, null, parentContext.tagScope);
     } else {
       // We've emitted the root and is now in plain HTML mode.
-      return createFormatContext(HTML_MODE, null, false);
+      return createFormatContext(HTML_MODE, null, parentContext.tagScope);
     }
   } else if (parentContext.insertionMode === HTML_HTML_MODE) {
     // We've emitted the document element and is now in plain HTML mode.
-    return createFormatContext(HTML_MODE, null, false);
+    return createFormatContext(HTML_MODE, null, parentContext.tagScope);
   }
   return parentContext;
 }
@@ -2457,12 +2451,14 @@ function pushImg(
   target: Array<Chunk | PrecomputedChunk>,
   props: Object,
   resumableState: ResumableState,
+  pictureTagInScope: boolean,
 ): null {
   const {src, srcSet} = props;
   if (
     props.loading !== 'lazy' &&
     (typeof src === 'string' || typeof srcSet === 'string') &&
     props.fetchPriority !== 'low' &&
+    pictureTagInScope === false &&
     // We exclude data URIs in src and srcSet since these should not be preloaded
     !(
       typeof src === 'string' &&
@@ -3230,7 +3226,7 @@ export function pushStartInstance(
             props,
             renderState,
             formatContext.insertionMode,
-            formatContext.noscriptTagInScope,
+            !!(formatContext.tagScope & NOSCRIPT_SCOPE),
           )
         : pushStartTitle(target, props);
     case 'link':
@@ -3241,7 +3237,7 @@ export function pushStartInstance(
         renderState,
         textEmbedded,
         formatContext.insertionMode,
-        formatContext.noscriptTagInScope,
+        !!(formatContext.tagScope & NOSCRIPT_SCOPE),
       );
     case 'script':
       return enableFloat
@@ -3251,7 +3247,7 @@ export function pushStartInstance(
             resumableState,
             textEmbedded,
             formatContext.insertionMode,
-            formatContext.noscriptTagInScope,
+            !!(formatContext.tagScope & NOSCRIPT_SCOPE),
           )
         : pushStartGenericElement(target, props, type);
     case 'style':
@@ -3262,7 +3258,7 @@ export function pushStartInstance(
         renderState,
         textEmbedded,
         formatContext.insertionMode,
-        formatContext.noscriptTagInScope,
+        !!(formatContext.tagScope & NOSCRIPT_SCOPE),
       );
     case 'meta':
       return pushMeta(
@@ -3271,7 +3267,7 @@ export function pushStartInstance(
         renderState,
         textEmbedded,
         formatContext.insertionMode,
-        formatContext.noscriptTagInScope,
+        !!(formatContext.tagScope & NOSCRIPT_SCOPE),
       );
     // Newline eating tags
     case 'listing':
@@ -3280,7 +3276,12 @@ export function pushStartInstance(
     }
     case 'img': {
       return enableFloat
-        ? pushImg(target, props, resumableState)
+        ? pushImg(
+            target,
+            props,
+            resumableState,
+            !!(formatContext.tagScope & PICTURE_SCOPE),
+          )
         : pushSelfClosing(target, props, type);
     }
     // Omitted close tags

diff --git a/packages/react-dom/src/__tests__/ReactDOMFloat-test.js b/packages/react-dom/src/__tests__/ReactDOMFloat-test.js
@@ -4085,6 +4085,40 @@ body {
     );
   });
 
+  // https://github.com/vercel/next.js/discussions/54799
+  it('omits preloads when an <img> is inside a <picture>', async () => {
+    await act(() => {
+      renderToPipeableStream(
+        <html>
+          <body>
+            <picture>
+              <img src="foo" />
+            </picture>
+            <picture>
+              <source type="image/webp" srcSet="webpsrc" />
+              <img src="jpg fallback" />
+            </picture>
+          </body>
+        </html>,
+      ).pipe(writable);
+    });
+
+    expect(getMeaningfulChildren(document)).toEqual(
+      <html>
+        <head />
+        <body>
+          <picture>
+            <img src="foo" />
+          </picture>
+          <picture>
+            <source type="image/webp" srcset="webpsrc" />
+            <img src="jpg fallback" />
+          </picture>
+        </body>
+      </html>,
+    );
+  });
+
   describe('ReactDOM.prefetchDNS(href)', () => {
     it('creates a dns-prefetch resource when called', async () => {
       function App({url}) {