From e1ad4aa3615333009d76f947ff05ddeff01039c6 Mon Sep 17 00:00:00 2001 From: Josh Story Date: Thu, 1 Jun 2023 13:34:36 -0700 Subject: [PATCH] [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. --- .../src/client/ReactFiberConfigDOM.js | 32 +-- .../src/server/ReactFizzConfigDOM.js | 226 ++++++------------ .../src/__tests__/ReactDOMFloat-test.js | 59 ++--- .../src/ReactFiberConfigWithNoHydration.js | 1 - .../src/ReactFiberHydrationContext.js | 11 - .../src/forks/ReactFiberConfig.custom.js | 1 - 6 files changed, 102 insertions(+), 228 deletions(-) diff --git a/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js b/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js index bab0772ca57cd..d75afafcff8e1 100644 --- a/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js +++ b/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; } diff --git a/packages/react-dom-bindings/src/server/ReactFizzConfigDOM.js b/packages/react-dom-bindings/src/server/ReactFizzConfigDOM.js index 9a05521eac3ed..788b201566aa4 100644 --- a/packages/react-dom-bindings/src/server/ReactFizzConfigDOM.js +++ b/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