Fix double preload (#26729)
I found a couple scenarios where preloads were issued too aggressively 1. During SSR, if you render a new stylesheet after the preamble flushed it will flush a preload even if the resource was already preloaded 2. During Client render, if you call `ReactDOM.preload()` it will only check if a preload exists in the Document before inserting a new one. It should check for an underlying resource such as a stylesheet link or script if the preload is for a recognized asset type
Josh Story committed
Apr 25, 2023 at 15:10 UTC
ec5e9c2a75749b0a470b7148738cb85bbb035958
3 files changed
+194
-11
packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js
+20
-3
@@ -2138,8 +2138,12 @@ function preload(href: string, options: PreloadOptions) {
2138
const as = options.as;
2139
const limitedEscapedHref =
2140
escapeSelectorAttributeValueInsideDoubleQuotes(href);
2141
- const preloadKey = `link[rel="preload"][as="${as}"][href="${limitedEscapedHref}"]`;
2142
- let key = preloadKey;
2141
+ const preloadSelector = `link[rel="preload"][as="${as}"][href="${limitedEscapedHref}"]`;
2142
+
2143
+ // Some preloads are keyed under their selector. This happens when the preload is for
2144
+ // an arbitrary type. Other preloads are keyed under the resource key they represent a preload for.
2145
+ // Here we figure out which key to use to determine if we have a preload already.
2146
+ let key = preloadSelector;
2147
switch (as) {
2148
case 'style':
2149
key = getStyleKey(href);
@@ -2152,7 +2156,20 @@ function preload(href: string, options: PreloadOptions) {
2156
const preloadProps = preloadPropsFromPreloadOptions(href, as, options);
2157
preloadPropsMap.set(key, preloadProps);
2158
2155
- if (null === ownerDocument.querySelector(preloadKey)) {
2159
+ if (null === ownerDocument.querySelector(preloadSelector)) {
2160
+ if (
2161
+ as === 'style' &&
2162
+ ownerDocument.querySelector(getStylesheetSelectorFromKey(key))
2163
+ ) {
2164
+ // We already have a stylesheet for this key. We don't need to preload it.
2165
+ return;
2166
+ } else if (
2167
+ as === 'script' &&
2168
+ ownerDocument.querySelector(getScriptSelectorFromKey(key))
2169
+ ) {
2170
+ // We already have a stylesheet for this key. We don't need to preload it.
2171
+ return;
2172
+ }
2173
const instance = ownerDocument.createElement('link');
2174
setInitialProperties(instance, 'link', preloadProps);
2175
markNodeAsHoistable(instance);
packages/react-dom-bindings/src/server/ReactFizzConfigDOM.js
+14
-8
@@ -1900,6 +1900,7 @@ function pushLink(
1900
if (!resource) {
1901
const resourceProps = stylesheetPropsFromRawProps(props);
1902
const preloadResource = resources.preloadsMap.get(key);
1903
+ let state = NoState;
1904
if (preloadResource) {
1905
// If we already had a preload we don't want that resource to flush directly.
1906
// We let the newly created resource govern flushing.
@@ -1908,11 +1909,14 @@ function pushLink(
1909
resourceProps,
1910
preloadResource.props,
1911
);
1912
+ if (preloadResource.state & Flushed) {
1913
+ state = PreloadFlushed;
1914
+ }
1915
}
1916
resource = {
1917
type: 'stylesheet',
1918
chunks: ([]: Array<Chunk | PrecomputedChunk>),
1915
- state: NoState,
1919
+ state,
1920
props: resourceProps,
1921
};
1922
resources.stylesMap.set(key, resource);
@@ -4004,12 +4008,9 @@ function flushAllStylesInPreamble(
4008
}
4009
4010
function preloadLateStyle(this: Destination, resource: StyleResource) {
4007
- if (__DEV__) {
4008
- if (resource.state & PreloadFlushed) {
4009
- console.error(
4010
- 'React encountered a Stylesheet Resource that already flushed a Preload when it was not expected to. This is a bug in React.',
4011
- );
4012
- }
4011
+ if (resource.state & PreloadFlushed) {
4012
+ // This resource has already had a preload flushed
4013
+ return;
4014
}
4015
4016
if (resource.type === 'style') {
@@ -5209,10 +5210,15 @@ function preinit(href: string, options: PreinitOptions): void {
5210
}
5211
}
5212
if (!resource) {
5213
+ let state = NoState;
5214
+ const preloadResource = resources.preloadsMap.get(key);
5215
+ if (preloadResource && preloadResource.state & Flushed) {
5216
+ state = PreloadFlushed;
5217
+ }
5218
resource = {
5219
type: 'stylesheet',
5220
chunks: ([]: Array<Chunk | PrecomputedChunk>),
5215
- state: NoState,
5221
+ state,
5222
props: stylesheetPropsFromPreinitOptions(href, precedence, options),
5223
};
5224
resources.stylesMap.set(key, resource);
packages/react-dom/src/__tests__/ReactDOMFloat-test.js
+160
@@ -3391,6 +3391,166 @@ body {
3391
);
3392
});
3393
3394
+ it('will not flush a preload for a new rendered Stylesheet Resource if one was already flushed', async () => {
3395
+ function Component() {
3396
+ ReactDOM.preload('foo', {as: 'style'});
3397
+ return (
3398
+ <div>
3399
+ <Suspense fallback="loading...">
3400
+ <BlockedOn value="blocked">
3401
+ <link rel="stylesheet" href="foo" precedence="default" />
3402
+ hello
3403
+ </BlockedOn>
3404
+ </Suspense>
3405
+ </div>
3406
+ );
3407
+ }
3408
+ await act(() => {
3409
+ renderToPipeableStream(
3410
+ <html>
3411
+ <body>
3412
+ <Component />
3413
+ </body>
3414
+ </html>,
3415
+ ).pipe(writable);
3416
+ });
3417
+
3418
+ expect(getMeaningfulChildren(document)).toEqual(
3419
+ <html>
3420
+ <head>
3421
+ <link rel="preload" as="style" href="foo" />
3422
+ </head>
3423
+ <body>
3424
+ <div>loading...</div>
3425
+ </body>
3426
+ </html>,
3427
+ );
3428
+ await act(() => {
3429
+ resolveText('blocked');
3430
+ });
3431
+ await act(loadStylesheets);
3432
+ assertLog(['load stylesheet: foo']);
3433
+ expect(getMeaningfulChildren(document)).toEqual(
3434
+ <html>
3435
+ <head>
3436
+ <link rel="stylesheet" href="foo" data-precedence="default" />
3437
+ <link rel="preload" as="style" href="foo" />
3438
+ </head>
3439
+ <body>
3440
+ <div>hello</div>
3441
+ </body>
3442
+ </html>,
3443
+ );
3444
+ });
3445
+
3446
+ it('will not flush a preload for a new preinitialized Stylesheet Resource if one was already flushed', async () => {
3447
+ function Component() {
3448
+ ReactDOM.preload('foo', {as: 'style'});
3449
+ return (
3450
+ <div>
3451
+ <Suspense fallback="loading...">
3452
+ <BlockedOn value="blocked">
3453
+ <Preinit />
3454
+ hello
3455
+ </BlockedOn>
3456
+ </Suspense>
3457
+ </div>
3458
+ );
3459
+ }
3460
+
3461
+ function Preinit() {
3462
+ ReactDOM.preinit('foo', {as: 'style'});
3463
+ }
3464
+ await act(() => {
3465
+ renderToPipeableStream(
3466
+ <html>
3467
+ <body>
3468
+ <Component />
3469
+ </body>
3470
+ </html>,
3471
+ ).pipe(writable);
3472
+ });
3473
+
3474
+ expect(getMeaningfulChildren(document)).toEqual(
3475
+ <html>
3476
+ <head>
3477
+ <link rel="preload" as="style" href="foo" />
3478
+ </head>
3479
+ <body>
3480
+ <div>loading...</div>
3481
+ </body>
3482
+ </html>,
3483
+ );
3484
+ await act(() => {
3485
+ resolveText('blocked');
3486
+ });
3487
+ expect(getMeaningfulChildren(document)).toEqual(
3488
+ <html>
3489
+ <head>
3490
+ <link rel="preload" as="style" href="foo" />
3491
+ </head>
3492
+ <body>
3493
+ <div>hello</div>
3494
+ </body>
3495
+ </html>,
3496
+ );
3497
+ });
3498
+
3499
+ it('will not insert a preload if the underlying resource already exists in the Document', async () => {
3500
+ await act(() => {
3501
+ renderToPipeableStream(
3502
+ <html>
3503
+ <head>
3504
+ <link rel="stylesheet" href="foo" precedence="default" />
3505
+ <script async={true} src="bar" />
3506
+ <link rel="preload" href="baz" as="font" />
3507
+ </head>
3508
+ <body>
3509
+ <div id="container" />
3510
+ </body>
3511
+ </html>,
3512
+ ).pipe(writable);
3513
+ });
3514
+
3515
+ expect(getMeaningfulChildren(document)).toEqual(
3516
+ <html>
3517
+ <head>
3518
+ <link rel="stylesheet" href="foo" data-precedence="default" />
3519
+ <script async="" src="bar" />
3520
+ <link rel="preload" href="baz" as="font" />
3521
+ </head>
3522
+ <body>
3523
+ <div id="container" />
3524
+ </body>
3525
+ </html>,
3526
+ );
3527
+
3528
+ container = document.getElementById('container');
3529
+
3530
+ function ClientApp() {
3531
+ ReactDOM.preload('foo', {as: 'style'});
3532
+ ReactDOM.preload('bar', {as: 'script'});
3533
+ ReactDOM.preload('baz', {as: 'font'});
3534
+ return 'foo';
3535
+ }
3536
+
3537
+ const root = ReactDOMClient.createRoot(container);
3538
+
3539
+ await clientAct(() => root.render(<ClientApp />));
3540
+ expect(getMeaningfulChildren(document)).toEqual(
3541
+ <html>
3542
+ <head>
3543
+ <link rel="stylesheet" href="foo" data-precedence="default" />
3544
+ <script async="" src="bar" />
3545
+ <link rel="preload" href="baz" as="font" />
3546
+ </head>
3547
+ <body>
3548
+ <div id="container">foo</div>
3549
+ </body>
3550
+ </html>,
3551
+ );
3552
+ });
3553
+
3554
describe('ReactDOM.prefetchDNS(href)', () => {
3555
it('creates a dns-prefetch resource when called', async () => {
3556
function App({url}) {