[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.
Josh Story committed
Jun 1, 2023 at 13:34 UTC
e1ad4aa3615333009d76f947ff05ddeff01039c6
6 files changed
+102
-228
packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js
+10
-22
@@ -1036,19 +1036,6 @@ export function bindInstance(
1036
1037
export const supportsHydration = true;
1038
1039
-// With Resources, some HostComponent types will never be server rendered and need to be
1040
-// inserted without breaking hydration
1041
-export function isHydratableType(type: string, props: Props): boolean {
1042
- if (enableFloat) {
1043
- if (type === 'script') {
1044
- const {async, onLoad, onError} = (props: any);
1045
- return !(async && (onLoad || onError));
1046
- }
1047
- return true;
1048
- } else {
1049
- return true;
1050
- }
1051
-}
1039
export function isHydratableText(text: string): boolean {
1040
return text !== '';
1041
}
@@ -1164,21 +1151,22 @@ export function canHydrateInstance(
1151
// if we learn it is problematic
1152
const srcAttr = element.getAttribute('src');
1153
if (
1167
- srcAttr &&
1168
- element.hasAttribute('async') &&
1169
- !element.hasAttribute('itemprop')
1170
- ) {
1171
- // This is an async script resource
1172
- break;
1173
- } else if (
1154
srcAttr !== (anyProps.src == null ? null : anyProps.src) ||
1155
element.getAttribute('type') !==
1156
(anyProps.type == null ? null : anyProps.type) ||
1157
element.getAttribute('crossorigin') !==
1158
(anyProps.crossOrigin == null ? null : anyProps.crossOrigin)
1159
) {
1180
- // This script is for a different src
1181
- break;
1160
+ // This script is for a different src/type/crossOrigin. It may be a script resource
1161
+ // or it may just be a mistmatch
1162
+ if (
1163
+ srcAttr &&
1164
+ element.hasAttribute('async') &&
1165
+ !element.hasAttribute('itemprop')
1166
+ ) {
1167
+ // This is an async script resource
1168
+ break;
1169
+ }
1170
}
1171
return element;
1172
}
packages/react-dom-bindings/src/server/ReactFizzConfigDOM.js
+78
-148
@@ -2642,128 +2642,102 @@ function pushScript(
2642
noscriptTagInScope: boolean,
2643
): null {
2644
if (enableFloat) {
2645
+ const asyncProp = props.async;
2646
if (
2647
+ typeof props.src !== 'string' ||
2648
+ !props.src ||
2649
+ !(
2650
+ asyncProp &&
2651
+ typeof asyncProp !== 'function' &&
2652
+ typeof asyncProp !== 'symbol'
2653
+ ) ||
2654
+ props.onLoad ||
2655
+ props.onError ||
2656
insertionMode === SVG_MODE ||
2657
noscriptTagInScope ||
2648
- props.itemProp != null ||
2649
- typeof props.src !== 'string' ||
2650
- !props.src
2658
+ props.itemProp != null
2659
) {
2652
- // This script will not be a resource nor can it be preloaded, we bailout early
2653
- // and emit it in place.
2660
+ // This script will not be a resource, we bailout early and emit it in place.
2661
return pushScriptImpl(target, props);
2662
}
2663
2664
const src = props.src;
2665
const key = getResourceKey('script', src);
2659
- if (props.async !== true || props.onLoad || props.onError) {
2660
- // we don't want to preload nomodule scripts
2661
- if (props.noModule !== true) {
2662
- // We can't resourcify scripts with load listeners. To avoid ambiguity with
2663
- // other Resourcified async scripts on the server we omit them from the server
2664
- // stream and expect them to be inserted during hydration on the client.
2665
- // We can still preload them however so the client can start fetching the script
2666
- // as soon as possible
2667
- let resource = resources.preloadsMap.get(key);
2668
- if (!resource) {
2669
- resource = {
2670
- type: 'preload',
2671
- chunks: [],
2672
- state: NoState,
2673
- props: preloadAsScriptPropsFromProps(props.src, props),
2674
- };
2675
- resources.preloadsMap.set(key, resource);
2676
- if (__DEV__) {
2677
- markAsImplicitResourceDEV(resource, props, resource.props);
2666
+ // We can make this <script> into a ScriptResource
2667
+ let resource = resources.scriptsMap.get(key);
2668
+ if (__DEV__) {
2669
+ const devResource = getAsResourceDEV(resource);
2670
+ if (devResource) {
2671
+ switch (devResource.__provenance) {
2672
+ case 'rendered': {
2673
+ const differenceDescription = describeDifferencesForScripts(
2674
+ // Diff the props from the JSX element, not the derived resource props
2675
+ props,
2676
+ devResource.__originalProps,
2677
+ );
2678
+ if (differenceDescription) {
2679
+ console.error(
2680
+ 'React encountered a <script async={true} src="%s" .../> that has props that conflict' +
2681
+ ' with another hoistable script with the same `src`. When rendering hoistable scripts (async scripts without any loading handlers)' +
2682
+ ' the props from the first encountered instance will be used and props from later instances will be ignored.' +
2683
+ ' Update the props on both <script async={true} .../> instance so they agree.%s',
2684
+ src,
2685
+ differenceDescription,
2686
+ );
2687
+ }
2688
+ break;
2689
}
2679
- resources.usedScripts.add(resource);
2680
- pushLinkImpl(resource.chunks, resource.props);
2681
- }
2682
- }
2683
-
2684
- if (props.async !== true) {
2685
- // This is not an async script, we can preloaded it but it still needs to
2686
- // be emitted in place since it needs to hydrate on the client
2687
- pushScriptImpl(target, props);
2688
- return null;
2689
- }
2690
- } else {
2691
- // We can make this <script> into a ScriptResource
2692
- let resource = resources.scriptsMap.get(key);
2693
- if (__DEV__) {
2694
- const devResource = getAsResourceDEV(resource);
2695
- if (devResource) {
2696
- switch (devResource.__provenance) {
2697
- case 'rendered': {
2698
- const differenceDescription = describeDifferencesForScripts(
2690
+ case 'preinit': {
2691
+ const differenceDescription =
2692
+ describeDifferencesForScriptOverPreinit(
2693
// Diff the props from the JSX element, not the derived resource props
2694
props,
2701
- devResource.__originalProps,
2695
+ devResource.__propsEquivalent,
2696
+ );
2697
+ if (differenceDescription) {
2698
+ console.error(
2699
+ 'React encountered a <script async={true} src="%s" .../> with props that conflict' +
2700
+ ' with the options provided to `ReactDOM.preinit("%s", { as: "script", ... })`. React will use the first props or preinitialization' +
2701
+ ' options encountered when rendering a hoistable script with a particular `src` and will ignore any newer props or' +
2702
+ ' options. The first instance of this script resource was created using the `ReactDOM.preinit()` function.' +
2703
+ ' Please note, `ReactDOM.preinit()` is modeled off of module import assertions capabilities and does not support' +
2704
+ ' arbitrary props. If you need to have props not included with the preinit options you will need to rely on rendering' +
2705
+ ' <script> tags only.%s',
2706
+ src,
2707
+ src,
2708
+ differenceDescription,
2709
);
2703
- if (differenceDescription) {
2704
- console.error(
2705
- 'React encountered a <script async={true} src="%s" .../> that has props that conflict' +
2706
- ' with another hoistable script with the same `src`. When rendering hoistable scripts (async scripts without any loading handlers)' +
2707
- ' the props from the first encountered instance will be used and props from later instances will be ignored.' +
2708
- ' Update the props on both <script async={true} .../> instance so they agree.%s',
2709
- src,
2710
- differenceDescription,
2711
- );
2712
- }
2713
- break;
2714
- }
2715
- case 'preinit': {
2716
- const differenceDescription =
2717
- describeDifferencesForScriptOverPreinit(
2718
- // Diff the props from the JSX element, not the derived resource props
2719
- props,
2720
- devResource.__propsEquivalent,
2721
- );
2722
- if (differenceDescription) {
2723
- console.error(
2724
- 'React encountered a <script async={true} src="%s" .../> with props that conflict' +
2725
- ' with the options provided to `ReactDOM.preinit("%s", { as: "script", ... })`. React will use the first props or preinitialization' +
2726
- ' options encountered when rendering a hoistable script with a particular `src` and will ignore any newer props or' +
2727
- ' options. The first instance of this script resource was created using the `ReactDOM.preinit()` function.' +
2728
- ' Please note, `ReactDOM.preinit()` is modeled off of module import assertions capabilities and does not support' +
2729
- ' arbitrary props. If you need to have props not included with the preinit options you will need to rely on rendering' +
2730
- ' <script> tags only.%s',
2731
- src,
2732
- src,
2733
- differenceDescription,
2734
- );
2735
- }
2736
- break;
2710
}
2711
+ break;
2712
}
2713
}
2714
}
2741
- if (!resource) {
2742
- resource = {
2743
- type: 'script',
2744
- chunks: [],
2745
- state: NoState,
2746
- props: null,
2747
- };
2748
- resources.scriptsMap.set(key, resource);
2749
- if (__DEV__) {
2750
- markAsRenderedResourceDEV(resource, props);
2751
- }
2752
- // Add to the script flushing queue
2753
- resources.scripts.add(resource);
2754
-
2755
- let scriptProps = props;
2756
- const preloadResource = resources.preloadsMap.get(key);
2757
- if (preloadResource) {
2758
- // If we already had a preload we don't want that resource to flush directly.
2759
- // We let the newly created resource govern flushing.
2760
- preloadResource.state |= Blocked;
2761
- scriptProps = {...props};
2762
- adoptPreloadPropsForScriptProps(scriptProps, preloadResource.props);
2763
- }
2764
- // encode the tag as Chunks
2765
- pushScriptImpl(resource.chunks, scriptProps);
2715
+ }
2716
+ if (!resource) {
2717
+ resource = {
2718
+ type: 'script',
2719
+ chunks: [],
2720
+ state: NoState,
2721
+ props: null,
2722
+ };
2723
+ resources.scriptsMap.set(key, resource);
2724
+ if (__DEV__) {
2725
+ markAsRenderedResourceDEV(resource, props);
2726
+ }
2727
+ // Add to the script flushing queue
2728
+ resources.scripts.add(resource);
2729
+
2730
+ let scriptProps = props;
2731
+ const preloadResource = resources.preloadsMap.get(key);
2732
+ if (preloadResource) {
2733
+ // If we already had a preload we don't want that resource to flush directly.
2734
+ // We let the newly created resource govern flushing.
2735
+ preloadResource.state |= Blocked;
2736
+ scriptProps = {...props};
2737
+ adoptPreloadPropsForScriptProps(scriptProps, preloadResource.props);
2738
}
2739
+ // encode the tag as Chunks
2740
+ pushScriptImpl(resource.chunks, scriptProps);
2741
}
2742
2743
if (textEmbedded) {
@@ -4239,9 +4213,6 @@ export function writePreamble(
4213
resources.scripts.forEach(flushResourceInPreamble, destination);
4214
resources.scripts.clear();
4215
4242
- resources.usedScripts.forEach(flushResourceInPreamble, destination);
4243
- resources.usedScripts.clear();
4244
-
4216
resources.explicitStylesheetPreloads.forEach(
4217
flushResourceInPreamble,
4218
destination,
@@ -4319,9 +4290,6 @@ export function writeHoistables(
4290
resources.scripts.forEach(flushResourceLate, destination);
4291
resources.scripts.clear();
4292
4322
- resources.usedScripts.forEach(flushResourceLate, destination);
4323
- resources.usedScripts.clear();
4324
-
4293
resources.explicitStylesheetPreloads.forEach(flushResourceLate, destination);
4294
resources.explicitStylesheetPreloads.clear();
4295
@@ -4873,7 +4841,6 @@ export type Resources = {
4841
precedences: Map<string, Set<StyleResource>>,
4842
stylePrecedences: Map<string, StyleTagResource>,
4843
scripts: Set<ScriptResource>,
4876
- usedScripts: Set<PreloadResource>,
4844
explicitStylesheetPreloads: Set<PreloadResource>,
4845
// explicitImagePreloads: Set<PreloadResource>,
4846
explicitScriptPreloads: Set<PreloadResource>,
@@ -4900,7 +4867,6 @@ export function createResources(): Resources {
4867
precedences: new Map(),
4868
stylePrecedences: new Map(),
4869
scripts: new Set(),
4903
- usedScripts: new Set(),
4870
explicitStylesheetPreloads: new Set(),
4871
// explicitImagePreloads: new Set(),
4872
explicitScriptPreloads: new Set(),
@@ -5563,19 +5529,6 @@ function preloadAsStylePropsFromProps(href: string, props: any): PreloadProps {
5529
};
5530
}
5531
5566
-function preloadAsScriptPropsFromProps(href: string, props: any): PreloadProps {
5567
- return {
5568
- rel: 'preload',
5569
- as: 'script',
5570
- href,
5571
- crossOrigin: props.crossOrigin,
5572
- fetchPriority: props.fetchPriority,
5573
- integrity: props.integrity,
5574
- nonce: props.nonce,
5575
- referrerPolicy: props.referrerPolicy,
5576
- };
5577
-}
5578
-
5532
function stylesheetPropsFromPreinitOptions(
5533
href: string,
5534
precedence: string,
@@ -5694,29 +5647,6 @@ function markAsImperativeResourceDEV(
5647
}
5648
}
5649
5697
-function markAsImplicitResourceDEV(
5698
- resource: Resource,
5699
- underlyingProps: any,
5700
- impliedProps: any,
5701
-): void {
5702
- if (__DEV__) {
5703
- const devResource: ImplicitResourceDEV = (resource: any);
5704
- if (typeof devResource.__provenance === 'string') {
5705
- console.error(
5706
- 'Resource already marked for DEV type. This is a bug in React.',
5707
- );
5708
- }
5709
- devResource.__provenance = 'implicit';
5710
- devResource.__underlyingProps = underlyingProps;
5711
- devResource.__impliedProps = impliedProps;
5712
- } else {
5713
- // eslint-disable-next-line react-internal/prod-error-codes
5714
- throw new Error(
5715
- 'markAsImplicitResourceDEV was included in a production build. This is a bug in React.',
5716
- );
5717
- }
5718
-}
5719
-
5650
function getAsResourceDEV(
5651
resource: null | void | Resource,
5652
): null | ResourceDEV {
packages/react-dom/src/__tests__/ReactDOMFloat-test.js
+14
-45
@@ -456,12 +456,12 @@ describe('ReactDOMFloat', () => {
456
expect(getMeaningfulChildren(document)).toEqual(
457
<html>
458
<head>
459
- <link rel="preload" href="foo" as="script" />
459
<meta property="foo" content="bar" />
460
<title>foo</title>
461
<link rel="foo" href="bar" />
462
<noscript><link rel="icon" href="icon"></noscript>
463
<base target="foo" href="bar" />
464
+ <script async="" src="foo" />
465
</head>
466
<body>foo</body>
467
</html>,
@@ -487,7 +487,6 @@ describe('ReactDOMFloat', () => {
487
expect(getMeaningfulChildren(document)).toEqual(
488
<html>
489
<head>
490
- <link rel="preload" href="foo" as="script" />
490
<meta property="foo" content="bar" />
491
<title>foo</title>
492
<link rel="foo" href="bar" />
@@ -2668,8 +2667,6 @@ body {
2667
{/* Hoisted Resources and elements */}
2668
<link rel="stylesheet" href="stylesheet" data-precedence="default" />
2669
<script async="" src="rendered" />
2671
- <link rel="preload" as="script" href="sync rendered" />
2672
- <link rel="preload" as="script" href="async rendered" />
2670
<link rel="foo" href="foo" />
2671
<meta name="foo" content="foo" />
2672
<title>title</title>
@@ -2678,6 +2675,7 @@ body {
2675
<link rel="stylesheet" href="stylesheet" />
2676
<script src="sync rendered" data-meaningful="" />
2677
<style>{'body { background-color: red; }'}</style>
2678
+ <script src="async rendered" async="" />
2679
<noscript><meta name="noscript" content="noscript"></noscript>
2680
<link rel="foo" href="foo" />
2681
</head>
@@ -2734,8 +2732,6 @@ body {
2732
<style>{'body { background-color: blue; }'}</style>
2733
<link rel="stylesheet" href="stylesheet" data-precedence="default" />
2734
<script async="" src="rendered" />
2737
- <link rel="preload" as="script" href="sync rendered" />
2738
- <link rel="preload" as="script" href="async rendered" />
2735
<link rel="foo" href="foo" />
2736
<meta name="foo" content="foo" />
2737
<title>title</title>
@@ -2780,8 +2776,6 @@ body {
2776
<style>{'body { background-color: blue; }'}</style>
2777
<link rel="stylesheet" href="stylesheet" data-precedence="default" />
2778
<script async="" src="rendered" />
2783
- <link rel="preload" as="script" href="sync rendered" />
2784
- <link rel="preload" as="script" href="async rendered" />
2779
<style>{'body { background-color: blue; }'}</style>
2780
<div />
2781
<script async="" src="injected" />
@@ -6061,6 +6055,7 @@ background-color: green;
6055
<script src="foo" async={true} />
6056
<script src="bar" async={true} onLoad={() => {}} />
6057
<script src="baz" data-meaningful="" />
6058
+ <script src="qux" defer={true} data-meaningful="" />
6059
hello world
6060
</body>
6061
</html>,
@@ -6076,17 +6071,17 @@ background-color: green;
6071
<html>
6072
<head>
6073
<script src="foo" async="" />
6079
- <link rel="preload" href="bar" as="script" />
6080
- <link rel="preload" href="baz" as="script" />
6074
</head>
6075
<body>
6076
+ <script src="bar" async="" />
6077
<script src="baz" data-meaningful="" />
6078
+ <script src="qux" defer="" data-meaningful="" />
6079
hello world
6080
</body>
6081
</html>,
6082
);
6083
6089
- ReactDOMClient.hydrateRoot(
6084
+ const root = ReactDOMClient.hydrateRoot(
6085
document,
6086
<html>
6087
<head />
@@ -6094,6 +6089,7 @@ background-color: green;
6089
<script src="foo" async={true} />
6090
<script src="bar" async={true} onLoad={() => {}} />
6091
<script src="baz" data-meaningful="" />
6092
+ <script src="qux" defer={true} data-meaningful="" />
6093
hello world
6094
</body>
6095
</html>,
@@ -6105,53 +6101,26 @@ background-color: green;
6101
<html>
6102
<head>
6103
<script src="foo" async="" />
6108
- <link rel="preload" href="bar" as="script" />
6109
- <link rel="preload" href="baz" as="script" />
6104
</head>
6105
<body>
6106
<script src="bar" async="" />
6107
<script src="baz" data-meaningful="" />
6108
+ <script src="qux" defer="" data-meaningful="" />
6109
hello world
6110
</body>
6111
</html>,
6112
);
6118
- });
6119
-
6120
- // @gate enableFloat
6121
- it('respects attributes defined on the script element when preloading scripts during server rendering', async () => {
6122
- await act(() => {
6123
- const {pipe} = renderToPipeableStream(
6124
- <html>
6125
- <head />
6126
- <body>
6127
- <script src="foo" fetchPriority="high" nonce="1234" />
6128
- <script src="bar" fetchPriority="low" nonce="1234" />
6129
- hello world
6130
- </body>
6131
- </html>,
6132
- );
6133
- pipe(writable);
6134
- });
6113
6114
+ root.unmount();
6115
+ // When we unmount we expect to retain singletons and any content that is not cleared within them.
6116
+ // The foo script is a resource so it sticks around. The other scripts are regular HostComponents
6117
+ // so they unmount and are removed from the DOM.
6118
expect(getMeaningfulChildren(document)).toEqual(
6119
<html>
6120
<head>
6139
- <link
6140
- rel="preload"
6141
- href="foo"
6142
- fetchpriority="high"
6143
- nonce="1234"
6144
- as="script"
6145
- />
6146
- <link
6147
- rel="preload"
6148
- href="bar"
6149
- fetchpriority="low"
6150
- nonce="1234"
6151
- as="script"
6152
- />
6121
+ <script src="foo" async="" />
6122
</head>
6154
- <body>hello world</body>
6123
+ <body />
6124
</html>,
6125
);
6126
});
packages/react-reconciler/src/ReactFiberConfigWithNoHydration.js
-1
@@ -21,7 +21,6 @@ function shim(...args: any): empty {
21
// Hydration (when unsupported)
22
export type SuspenseInstance = mixed;
23
export const supportsHydration = false;
24
-export const isHydratableType = shim;
24
export const isHydratableText = shim;
25
export const isSuspenseInstancePending = shim;
26
export const isSuspenseInstanceFallback = shim;
packages/react-reconciler/src/ReactFiberHydrationContext.js
-11
@@ -37,7 +37,6 @@ import {
37
} from './ReactFiberFlags';
38
import {
39
enableHostSingletons,
40
- enableFloat,
40
enableClientRenderFallbackOnTextMismatch,
41
diffInCommitPhase,
42
} from 'shared/ReactFeatureFlags';
@@ -77,7 +76,6 @@ import {
76
canHydrateInstance,
77
canHydrateTextInstance,
78
canHydrateSuspenseInstance,
80
- isHydratableType,
79
isHydratableText,
80
} from './ReactFiberConfig';
81
import {OffscreenLane} from './ReactFiberLane';
@@ -450,15 +448,6 @@ function tryToClaimNextHydratableInstance(fiber: Fiber): void {
448
if (!isHydrating) {
449
return;
450
}
453
- if (enableFloat) {
454
- if (!isHydratableType(fiber.type, fiber.pendingProps)) {
455
- // This fiber never hydrates from the DOM and always does an insert
456
- fiber.flags = (fiber.flags & ~Hydrating) | Placement;
457
- isHydrating = false;
458
- hydrationParentFiber = fiber;
459
- return;
460
- }
461
- }
451
const initialInstance = nextHydratableInstance;
452
const nextInstance = nextHydratableInstance;
453
if (!nextInstance) {
packages/react-reconciler/src/forks/ReactFiberConfig.custom.js
-1
@@ -135,7 +135,6 @@ export const cloneHiddenTextInstance = $$$config.cloneHiddenTextInstance;
135
// Hydration
136
// (optional)
137
// -------------------
138
-export const isHydratableType = $$$config.isHydratableType;
138
export const isHydratableText = $$$config.isHydratableText;
139
export const isSuspenseInstancePending = $$$config.isSuspenseInstancePending;
140
export const isSuspenseInstanceFallback = $$$config.isSuspenseInstanceFallback;