[Fizz][Float] stop automatically preloading scripts that are not script resources (#26877)

Currently we preload all scripts that are not hoisted. One of the
original reasons for this is we stopped SSR rendering async scripts that
had an onLoad/onError because we needed to be able to distinguish
between Float scripts and non-Float scripts during hydration. Hydration
has been refactored a bit and we can not get around this limitation so
we can just emit the async script in place. However, sync and defer
scripts are also preloaded. While this is sometimes desirable it is not
universally so and there are issues with conveying priority properly
(see fetchpriority) so with this change we remove the automatic
preloading of non-Float scripts altogether.

For this change to make sense we also need to emit async scripts with
loading handlers during SSR. we previously only preloaded them during
SSR because it was necessary to keep async scripts as unambiguously
resources when hydrating. One ancillary benefit was that load handlers
would always fire b/c there was no chance the script would run before
hydration. With this change we go back to having the ability to have
load handlers fired before hydration. This is already a problem with
images and we don't have a generalized solution for it however our
likely approach to this sort of thing where you need to wait for a
script to load is to use something akin to `importScripts()` rather than
rendering a script with onLoad.

Author: gnoff

packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js

@@ -1036,19 +1036,6 @@ export function bindInstance(
 
 export const supportsHydration = true;
 
-// With Resources, some HostComponent types will never be server rendered and need to be
-// inserted without breaking hydration
-export function isHydratableType(type: string, props: Props): boolean {
-  if (enableFloat) {
-    if (type === 'script') {
-      const {async, onLoad, onError} = (props: any);
-      return !(async && (onLoad || onError));
-    }
-    return true;
-  } else {
-    return true;
-  }
-}
 export function isHydratableText(text: string): boolean {
   return text !== '';
 }
@@ -1164,21 +1151,22 @@ export function canHydrateInstance(
           // if we learn it is problematic
           const srcAttr = element.getAttribute('src');
           if (
-            srcAttr &&
-            element.hasAttribute('async') &&
-            !element.hasAttribute('itemprop')
-          ) {
-            // This is an async script resource
-            break;
-          } else if (
             srcAttr !== (anyProps.src == null ? null : anyProps.src) ||
             element.getAttribute('type') !==
               (anyProps.type == null ? null : anyProps.type) ||
             element.getAttribute('crossorigin') !==
               (anyProps.crossOrigin == null ? null : anyProps.crossOrigin)
           ) {
-            // This script is for a different src
-            break;
+            // This script is for a different src/type/crossOrigin. It may be a script resource
+            // or it may just be a mistmatch
+            if (
+              srcAttr &&
+              element.hasAttribute('async') &&
+              !element.hasAttribute('itemprop')
+            ) {
+              // This is an async script resource
+              break;
+            }
           }
           return element;
         }

packages/react-dom-bindings/src/server/ReactFizzConfigDOM.js

@@ -2642,128 +2642,102 @@ function pushScript(
   noscriptTagInScope: boolean,
 ): null {
   if (enableFloat) {
+    const asyncProp = props.async;
     if (
+      typeof props.src !== 'string' ||
+      !props.src ||
+      !(
+        asyncProp &&
+        typeof asyncProp !== 'function' &&
+        typeof asyncProp !== 'symbol'
+      ) ||
+      props.onLoad ||
+      props.onError ||
       insertionMode === SVG_MODE ||
       noscriptTagInScope ||
-      props.itemProp != null ||
-      typeof props.src !== 'string' ||
-      !props.src
+      props.itemProp != null
     ) {
-      // This script will not be a resource nor can it be preloaded, we bailout early
-      // and emit it in place.
+      // This script will not be a resource, we bailout early and emit it in place.
       return pushScriptImpl(target, props);
     }
 
     const src = props.src;
     const key = getResourceKey('script', src);
-    if (props.async !== true || props.onLoad || props.onError) {
-      // we don't want to preload nomodule scripts
-      if (props.noModule !== true) {
-        // We can't resourcify scripts with load listeners. To avoid ambiguity with
-        // other Resourcified async scripts on the server we omit them from the server
-        // stream and expect them to be inserted during hydration on the client.
-        // We can still preload them however so the client can start fetching the script
-        // as soon as possible
-        let resource = resources.preloadsMap.get(key);
-        if (!resource) {
-          resource = {
-            type: 'preload',
-            chunks: [],
-            state: NoState,
-            props: preloadAsScriptPropsFromProps(props.src, props),
-          };
-          resources.preloadsMap.set(key, resource);
-          if (__DEV__) {
-            markAsImplicitResourceDEV(resource, props, resource.props);
+    // We can make this <script> into a ScriptResource
+    let resource = resources.scriptsMap.get(key);
+    if (__DEV__) {
+      const devResource = getAsResourceDEV(resource);
+      if (devResource) {
+        switch (devResource.__provenance) {
+          case 'rendered': {
+            const differenceDescription = describeDifferencesForScripts(
+              // Diff the props from the JSX element, not the derived resource props
+              props,
+              devResource.__originalProps,
+            );
+            if (differenceDescription) {
+              console.error(
+                'React encountered a <script async={true} src="%s" .../> that has props that conflict' +
+                  ' with another hoistable script with the same `src`. When rendering hoistable scripts (async scripts without any loading handlers)' +
+                  ' the props from the first encountered instance will be used and props from later instances will be ignored.' +
+                  ' Update the props on both <script async={true} .../> instance so they agree.%s',
+                src,
+                differenceDescription,
+              );
+            }
+            break;
           }
-          resources.usedScripts.add(resource);
-          pushLinkImpl(resource.chunks, resource.props);
-        }
-      }
-
-      if (props.async !== true) {
-        // This is not an async script, we can preloaded it but it still needs to
-        // be emitted in place since it needs to hydrate on the client
-        pushScriptImpl(target, props);
-        return null;
-      }
-    } else {
-      // We can make this <script> into a ScriptResource
-      let resource = resources.scriptsMap.get(key);
-      if (__DEV__) {
-        const devResource = getAsResourceDEV(resource);
-        if (devResource) {
-          switch (devResource.__provenance) {
-            case 'rendered': {
-              const differenceDescription = describeDifferencesForScripts(
+          case 'preinit': {
+            const differenceDescription =
+              describeDifferencesForScriptOverPreinit(
                 // Diff the props from the JSX element, not the derived resource props
                 props,
-                devResource.__originalProps,
+                devResource.__propsEquivalent,
+              );
+            if (differenceDescription) {
+              console.error(
+                'React encountered a <script async={true} src="%s" .../> with props that conflict' +
+                  ' with the options provided to `ReactDOM.preinit("%s", { as: "script", ... })`. React will use the first props or preinitialization' +
+                  ' options encountered when rendering a hoistable script with a particular `src` and will ignore any newer props or' +
+                  ' options. The first instance of this script resource was created using the `ReactDOM.preinit()` function.' +
+                  ' Please note, `ReactDOM.preinit()` is modeled off of module import assertions capabilities and does not support' +
+                  ' arbitrary props. If you need to have props not included with the preinit options you will need to rely on rendering' +
+                  ' <script> tags only.%s',
+                src,
+                src,
+                differenceDescription,
               );
-              if (differenceDescription) {
-                console.error(
-                  'React encountered a <script async={true} src="%s" .../> that has props that conflict' +
-                    ' with another hoistable script with the same `src`. When rendering hoistable scripts (async scripts without any loading handlers)' +
-                    ' the props from the first encountered instance will be used and props from later instances will be ignored.' +
-                    ' Update the props on both <script async={true} .../> instance so they agree.%s',
-                  src,
-                  differenceDescription,
-                );
-              }
-              break;
-            }
-            case 'preinit': {
-              const differenceDescription =
-                describeDifferencesForScriptOverPreinit(
-                  // Diff the props from the JSX element, not the derived resource props
-                  props,
-                  devResource.__propsEquivalent,
-                );
-              if (differenceDescription) {
-                console.error(
-                  'React encountered a <script async={true} src="%s" .../> with props that conflict' +
-                    ' with the options provided to `ReactDOM.preinit("%s", { as: "script", ... })`. React will use the first props or preinitialization' +
-                    ' options encountered when rendering a hoistable script with a particular `src` and will ignore any newer props or' +
-                    ' options. The first instance of this script resource was created using the `ReactDOM.preinit()` function.' +
-                    ' Please note, `ReactDOM.preinit()` is modeled off of module import assertions capabilities and does not support' +
-                    ' arbitrary props. If you need to have props not included with the preinit options you will need to rely on rendering' +
-                    ' <script> tags only.%s',
-                  src,
-                  src,
-                  differenceDescription,
-                );
-              }
-              break;
             }
+            break;
           }
         }
       }
-      if (!resource) {
-        resource = {
-          type: 'script',
-          chunks: [],
-          state: NoState,
-          props: null,
-        };
-        resources.scriptsMap.set(key, resource);
-        if (__DEV__) {
-          markAsRenderedResourceDEV(resource, props);
-        }
-        // Add to the script flushing queue
-        resources.scripts.add(resource);
-
-        let scriptProps = props;
-        const preloadResource = resources.preloadsMap.get(key);
-        if (preloadResource) {
-          // If we already had a preload we don't want that resource to flush directly.
-          // We let the newly created resource govern flushing.
-          preloadResource.state |= Blocked;
-          scriptProps = {...props};
-          adoptPreloadPropsForScriptProps(scriptProps, preloadResource.props);
-        }
-        // encode the tag as Chunks
-        pushScriptImpl(resource.chunks, scriptProps);
+    }
+    if (!resource) {
+      resource = {
+        type: 'script',
+        chunks: [],
+        state: NoState,
+        props: null,
+      };
+      resources.scriptsMap.set(key, resource);
+      if (__DEV__) {
+        markAsRenderedResourceDEV(resource, props);
+      }
+      // Add to the script flushing queue
+      resources.scripts.add(resource);
+
+      let scriptProps = props;
+      const preloadResource = resources.preloadsMap.get(key);
+      if (preloadResource) {
+        // If we already had a preload we don't want that resource to flush directly.
+        // We let the newly created resource govern flushing.
+        preloadResource.state |= Blocked;
+        scriptProps = {...props};
+        adoptPreloadPropsForScriptProps(scriptProps, preloadResource.props);
       }
+      // encode the tag as Chunks
+      pushScriptImpl(resource.chunks, scriptProps);
     }
 
     if (textEmbedded) {
@@ -4239,9 +4213,6 @@ export function writePreamble(
   resources.scripts.forEach(flushResourceInPreamble, destination);
   resources.scripts.clear();
 
-  resources.usedScripts.forEach(flushResourceInPreamble, destination);
-  resources.usedScripts.clear();
-
   resources.explicitStylesheetPreloads.forEach(
     flushResourceInPreamble,
     destination,
@@ -4319,9 +4290,6 @@ export function writeHoistables(
   resources.scripts.forEach(flushResourceLate, destination);
   resources.scripts.clear();
 
-  resources.usedScripts.forEach(flushResourceLate, destination);
-  resources.usedScripts.clear();
-
   resources.explicitStylesheetPreloads.forEach(flushResourceLate, destination);
   resources.explicitStylesheetPreloads.clear();
 
@@ -4873,7 +4841,6 @@ export type Resources = {
   precedences: Map<string, Set<StyleResource>>,
   stylePrecedences: Map<string, StyleTagResource>,
   scripts: Set<ScriptResource>,
-  usedScripts: Set<PreloadResource>,
   explicitStylesheetPreloads: Set<PreloadResource>,
   // explicitImagePreloads: Set<PreloadResource>,
   explicitScriptPreloads: Set<PreloadResource>,
@@ -4900,7 +4867,6 @@ export function createResources(): Resources {
     precedences: new Map(),
     stylePrecedences: new Map(),
     scripts: new Set(),
-    usedScripts: new Set(),
     explicitStylesheetPreloads: new Set(),
     // explicitImagePreloads: new Set(),
     explicitScriptPreloads: new Set(),
@@ -5563,19 +5529,6 @@ function preloadAsStylePropsFromProps(href: string, props: any): PreloadProps {
   };
 }
 
-function preloadAsScriptPropsFromProps(href: string, props: any): PreloadProps {
-  return {
-    rel: 'preload',
-    as: 'script',
-    href,
-    crossOrigin: props.crossOrigin,
-    fetchPriority: props.fetchPriority,
-    integrity: props.integrity,
-    nonce: props.nonce,
-    referrerPolicy: props.referrerPolicy,
-  };
-}
-
 function stylesheetPropsFromPreinitOptions(
   href: string,
   precedence: string,
@@ -5694,29 +5647,6 @@ function markAsImperativeResourceDEV(
   }
 }
 
-function markAsImplicitResourceDEV(
-  resource: Resource,
-  underlyingProps: any,
-  impliedProps: any,
-): void {
-  if (__DEV__) {
-    const devResource: ImplicitResourceDEV = (resource: any);
-    if (typeof devResource.__provenance === 'string') {
-      console.error(
-        'Resource already marked for DEV type. This is a bug in React.',
-      );
-    }
-    devResource.__provenance = 'implicit';
-    devResource.__underlyingProps = underlyingProps;
-    devResource.__impliedProps = impliedProps;
-  } else {
-    // eslint-disable-next-line react-internal/prod-error-codes
-    throw new Error(
-      'markAsImplicitResourceDEV was included in a production build. This is a bug in React.',
-    );
-  }
-}
-
 function getAsResourceDEV(
   resource: null | void | Resource,
 ): null | ResourceDEV {