@samitouri / QOS-React-2 / commits / 20b6f4c0e8

[Float][Fiber] Assume stylesheets in document are already loaded (#29811)

When we made stylesheets suspend even during high priority updates we exposed a bug in the loading tracking of stylesheets that are loaded as part of the preamble. This allowed these stylesheets to put suspense boundaries into fallback mode more often than expected because cases where a stylesheet was server rendered could now cause a fallback to trigger which was never intended to happen. This fix updates resource construction to evaluate whether the instance exists in the DOM prior to construction and if so marks the resource as loaded and inserted. One ambiguity that needed to be solved still is how to tell whether a stylesheet rendered as part of a late Suspense boundary reveal is already loaded. I updated the instruction to clear out the loading promise after successfully loading. This is useful because later if we encounter this same resource again we can avoid the microtask if it is already loaded. It also means that we can concretely understand that if a stylesheet is in the DOM without this marker then it must have loaded (or errored) already.

Josh Story committed Jun 7, 2024 at 16:53 UTC 20b6f4c0e8a1f40ee61735201645e0395ff08f94

5 files changed +225 -45

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

+42 -30

@@ -2412,7 +2412,7 @@ export function getResource(

2412 if (!resource) {

2413 // We asserted this above but Flow can't figure out that the type satisfies

2414 const ownerDocument = getDocumentFromRoot(resourceRoot);

2415 - resource = {

2415 + resource = ({

2416 type: 'stylesheet',

2417 instance: null,

2418 count: 0,

@@ -2420,15 +2420,34 @@ export function getResource(

2420 loading: NotLoaded,

2421 preload: null,

2422 },

2423 - };

2423 + }: StylesheetResource);

2424 styles.set(key, resource);

2425 + const instance = ownerDocument.querySelector(

2426 + getStylesheetSelectorFromKey(key),

2427 + );

2428 + if (instance) {

2429 + const loadingState: ?Promise<mixed> = (instance: any)._p;

2430 + if (loadingState) {

2431 + // This instance is inserted as part of a boundary reveal and is not yet

2432 + // loaded

2433 + } else {

2434 + // This instance is already loaded

2435 + resource.instance = instance;

2436 + resource.state.loading = Loaded | Inserted;

2437 + }

2438 + }

2439 +

2440 if (!preloadPropsMap.has(key)) {

2426 - preloadStylesheet(

2427 - ownerDocument,

2428 - key,

2429 - preloadPropsFromStylesheet(qualifiedProps),

2430 - resource.state,

2431 - );

2441 + const preloadProps = preloadPropsFromStylesheet(qualifiedProps);

2442 + preloadPropsMap.set(key, preloadProps);

2443 + if (!instance) {

2444 + preloadStylesheet(

2445 + ownerDocument,

2446 + key,

2447 + preloadProps,

2448 + resource.state,

2449 + );

2450 + }

2451 }

2452 }

2453 if (currentProps && currentResource === null) {

@@ -2599,28 +2618,21 @@ function preloadStylesheet(

2618 preloadProps: PreloadProps,

2619 state: StylesheetState,

2620 ) {

2602 - preloadPropsMap.set(key, preloadProps);

2603 -

2604 - if (!ownerDocument.querySelector(getStylesheetSelectorFromKey(key))) {

2605 - // There is no matching stylesheet instance in the Document.

2606 - // We will insert a preload now to kick off loading because

2607 - // we expect this stylesheet to commit

2608 - const preloadEl = ownerDocument.querySelector(

2609 - getPreloadStylesheetSelectorFromKey(key),

2610 - );

2611 - if (preloadEl) {

2612 - // If we find a preload already it was SSR'd and we won't have an actual

2613 - // loading state to track. For now we will just assume it is loaded

2614 - state.loading = Loaded;

2615 - } else {

2616 - const instance = ownerDocument.createElement('link');

2617 - state.preload = instance;

2618 - instance.addEventListener('load', () => (state.loading |= Loaded));

2619 - instance.addEventListener('error', () => (state.loading |= Errored));

2620 - setInitialProperties(instance, 'link', preloadProps);

2621 - markNodeAsHoistable(instance);

2622 - (ownerDocument.head: any).appendChild(instance);

2623 - }

2621 + const preloadEl = ownerDocument.querySelector(

2622 + getPreloadStylesheetSelectorFromKey(key),

2623 + );

2624 + if (preloadEl) {

2625 + // If we find a preload already it was SSR'd and we won't have an actual

2626 + // loading state to track. For now we will just assume it is loaded

2627 + state.loading = Loaded;

2628 + } else {

2629 + const instance = ownerDocument.createElement('link');

2630 + state.preload = instance;

2631 + instance.addEventListener('load', () => (state.loading |= Loaded));

2632 + instance.addEventListener('error', () => (state.loading |= Errored));

2633 + setInitialProperties(instance, 'link', preloadProps);

2634 + markNodeAsHoistable(instance);

2635 + (ownerDocument.head: any).appendChild(instance);

2636 }

2637 }

2638

packages/react-dom-bindings/src/server/fizz-instruction-set/ReactDOMFizzInstructionSetExternalRuntime.js

+8 -7

@@ -45,6 +45,11 @@ export function completeBoundaryWithStyles(

45 const dependencies = [];

46 let href, precedence, attr, loadingState, resourceEl, media;

48 + function cleanupWith(cb) {

49 + this['_p'] = null;

50 + cb();

51 + }

52 +

53 // Sheets Mode

54 let sheetMode = true;

55 while (true) {

@@ -80,18 +85,14 @@ export function completeBoundaryWithStyles(

85 resourceEl.setAttribute(attr, stylesheetDescriptor[j++]);

86 }

87 loadingState = resourceEl['_p'] = new Promise((resolve, reject) => {

83 - resourceEl.onload = resolve;

84 - resourceEl.onerror = reject;

88 + resourceEl.onload = cleanupWith.bind(resourceEl, resolve);

89 + resourceEl.onerror = cleanupWith.bind(resourceEl, reject);

90 });

91 // Save this resource element so we can bailout if it is used again

92 resourceMap.set(href, resourceEl);

93 }

94 media = resourceEl.getAttribute('media');

90 - if (

91 - loadingState &&

92 - loadingState['s'] !== 'l' &&

93 - (!media || window['matchMedia'](media).matches)

94 - ) {

95 + if (loadingState && (!media || window['matchMedia'](media).matches)) {

96 dependencies.push(loadingState);

97 }

98 if (avoidInsert) {

packages/react-dom-bindings/src/server/fizz-instruction-set/ReactDOMFizzInstructionSetInlineCodeStrings.js

+1 -1

@@ -6,7 +6,7 @@ export const clientRenderBoundary =

6 export const completeBoundary =

7 '$RC=function(b,c,e){c=document.getElementById(c);c.parentNode.removeChild(c);var a=document.getElementById(b);if(a){b=a.previousSibling;if(e)b.data="$!",a.setAttribute("data-dgst",e);else{e=b.parentNode;a=b.nextSibling;var f=0;do{if(a&&8===a.nodeType){var d=a.data;if("/$"===d)if(0===f)break;else f--;else"$"!==d&&"$?"!==d&&"$!"!==d||f++}d=a.nextSibling;e.removeChild(a);a=d}while(a);for(;c.firstChild;)e.insertBefore(c.firstChild,a);b.data="$"}b._reactRetry&&b._reactRetry()}};';

8 export const completeBoundaryWithStyles =

9 - '$RM=new Map;\n$RR=function(r,t,w){for(var u=$RC,n=$RM,p=new Map,q=document,g,b,h=q.querySelectorAll("link[data-precedence],style[data-precedence]"),v=[],k=0;b=h[k++];)"not all"===b.getAttribute("media")?v.push(b):("LINK"===b.tagName&&n.set(b.getAttribute("href"),b),p.set(b.dataset.precedence,g=b));b=0;h=[];var l,a;for(k=!0;;){if(k){var f=w[b++];if(!f){k=!1;b=0;continue}var c=!1,m=0;var d=f[m++];if(a=n.get(d)){var e=a._p;c=!0}else{a=q.createElement("link");a.href=d;a.rel="stylesheet";for(a.dataset.precedence=\nl=f[m++];e=f[m++];)a.setAttribute(e,f[m++]);e=a._p=new Promise(function(x,y){a.onload=x;a.onerror=y});n.set(d,a)}d=a.getAttribute("media");!e||"l"===e.s||d&&!matchMedia(d).matches||h.push(e);if(c)continue}else{a=v[b++];if(!a)break;l=a.getAttribute("data-precedence");a.removeAttribute("media")}c=p.get(l)||g;c===g&&(g=a);p.set(l,a);c?c.parentNode.insertBefore(a,c.nextSibling):(c=q.head,c.insertBefore(a,c.firstChild))}Promise.all(h).then(u.bind(null,r,t,""),u.bind(null,r,t,"Resource failed to load"))};';

9 + '$RM=new Map;\n$RR=function(t,u,y){function v(n){this._p=null;n()}for(var w=$RC,p=$RM,q=new Map,r=document,g,b,h=r.querySelectorAll("link[data-precedence],style[data-precedence]"),x=[],k=0;b=h[k++];)"not all"===b.getAttribute("media")?x.push(b):("LINK"===b.tagName&&p.set(b.getAttribute("href"),b),q.set(b.dataset.precedence,g=b));b=0;h=[];var l,a;for(k=!0;;){if(k){var e=y[b++];if(!e){k=!1;b=0;continue}var c=!1,m=0;var d=e[m++];if(a=p.get(d)){var f=a._p;c=!0}else{a=r.createElement("link");a.href=\nd;a.rel="stylesheet";for(a.dataset.precedence=l=e[m++];f=e[m++];)a.setAttribute(f,e[m++]);f=a._p=new Promise(function(n,z){a.onload=v.bind(a,n);a.onerror=v.bind(a,z)});p.set(d,a)}d=a.getAttribute("media");!f||d&&!matchMedia(d).matches||h.push(f);if(c)continue}else{a=x[b++];if(!a)break;l=a.getAttribute("data-precedence");a.removeAttribute("media")}c=q.get(l)||g;c===g&&(g=a);q.set(l,a);c?c.parentNode.insertBefore(a,c.nextSibling):(c=r.head,c.insertBefore(a,c.firstChild))}Promise.all(h).then(w.bind(null,\nt,u,""),w.bind(null,t,u,"Resource failed to load"))};';

10 export const completeSegment =

11 '$RS=function(a,b){a=document.getElementById(a);b=document.getElementById(b);for(a.parentNode.removeChild(a);a.firstChild;)b.parentNode.insertBefore(a.firstChild,b);b.parentNode.removeChild(b)};';

12 export const formReplaying =

packages/react-dom-bindings/src/server/fizz-instruction-set/ReactDOMFizzInstructionSetInlineSource.js

+8 -7

@@ -47,6 +47,11 @@ export function completeBoundaryWithStyles(

47 const dependencies = [];

48 let href, precedence, attr, loadingState, resourceEl, media;

50 + function cleanupWith(cb) {

51 + this['_p'] = null;

52 + cb();

53 + }

54 +

55 // Sheets Mode

56 let sheetMode = true;

57 while (true) {

@@ -82,18 +87,14 @@ export function completeBoundaryWithStyles(

87 resourceEl.setAttribute(attr, stylesheetDescriptor[j++]);

88 }

89 loadingState = resourceEl['_p'] = new Promise((resolve, reject) => {

85 - resourceEl.onload = resolve;

86 - resourceEl.onerror = reject;

90 + resourceEl.onload = cleanupWith.bind(resourceEl, resolve);

91 + resourceEl.onerror = cleanupWith.bind(resourceEl, reject);

92 });

93 // Save this resource element so we can bailout if it is used again

94 resourceMap.set(href, resourceEl);

95 }

96 media = resourceEl.getAttribute('media');

92 - if (

93 - loadingState &&

94 - loadingState['s'] !== 'l' &&

95 - (!media || window['matchMedia'](media).matches)

96 - ) {

97 + if (loadingState && (!media || window['matchMedia'](media).matches)) {

98 dependencies.push(loadingState);

99 }

100 if (avoidInsert) {

packages/react-dom/src/__tests__/ReactDOMFloat-test.js

+166

@@ -3348,6 +3348,172 @@ body {

3348 );

3349 });

3350

3351 + it('will assume stylesheets already in the document have loaded if it cannot confirm it is not yet loaded', async () => {

3352 + await act(() => {

3353 + renderToPipeableStream(

3354 + <html>

3355 + <head>

3356 + <link rel="stylesheet" href="foo" data-precedence="default" />

3357 + </head>

3358 + <body>

3359 + <div id="foo" />

3360 + </body>

3361 + </html>,

3362 + ).pipe(writable);

3363 + });

3364 +

3365 + const root = ReactDOMClient.createRoot(document.querySelector('#foo'));

3366 +

3367 + root.render(

3368 + <div>

3369 + <Suspense fallback="loading...">

3370 + <link rel="stylesheet" href="foo" precedence="default" />

3371 + hello world

3372 + </Suspense>

3373 + </div>,

3374 + );

3375 +

3376 + await waitForAll([]);

3377 + expect(getMeaningfulChildren(document)).toEqual(

3378 + <html>

3379 + <head>

3380 + <link rel="stylesheet" href="foo" data-precedence="default" />

3381 + </head>

3382 + <body>

3383 + <div id="foo">

3384 + <div>hello world</div>

3385 + </div>

3386 + </body>

3387 + </html>,

3388 + );

3389 + });

3390 +

3391 + it('will assume wait for loading stylesheets to load before continuing', async () => {

3392 + let ssr = true;

3393 + function Component() {

3394 + if (ssr) {

3395 + return null;

3396 + } else {

3397 + return (

3398 + <>

3399 + <link rel="stylesheet" href="foo" precedence="default" />

3400 + <div>hello client</div>

3401 + </>

3402 + );

3403 + }

3404 + }

3405 +

3406 + await act(() => {

3407 + renderToPipeableStream(

3408 + <html>

3409 + <body>

3410 + <div>

3411 + <Suspense fallback="loading...">

3412 + <BlockedOn value="reveal">

3413 + <link rel="stylesheet" href="foo" precedence="default" />

3414 + <div>hello world</div>

3415 + </BlockedOn>

3416 + </Suspense>

3417 + </div>

3418 + <div>

3419 + <Suspense fallback="loading 2...">

3420 + <Component />

3421 + </Suspense>

3422 + </div>

3423 + </body>

3424 + </html>,

3425 + ).pipe(writable);

3426 + });

3427 +

3428 + expect(getMeaningfulChildren(document)).toEqual(

3429 + <html>

3430 + <head />

3431 + <body>

3432 + <div>loading...</div>

3433 + <div />

3434 + </body>

3435 + </html>,

3436 + );

3437 +

3438 + await act(() => {

3439 + resolveText('reveal');

3440 + });

3441 +

3442 + expect(getMeaningfulChildren(document)).toEqual(

3443 + <html>

3444 + <head>

3445 + <link rel="stylesheet" href="foo" data-precedence="default" />

3446 + </head>

3447 + <body>

3448 + <div>loading...</div>

3449 + <div />

3450 + <link rel="preload" href="foo" as="style" />

3451 + </body>

3452 + </html>,

3453 + );

3454 +

3455 + ssr = false;

3456 +

3457 + ReactDOMClient.hydrateRoot(

3458 + document,

3459 + <html>

3460 + <body>

3461 + <div>

3462 + <Suspense fallback="loading...">

3463 + <BlockedOn value="reveal">

3464 + <link rel="stylesheet" href="foo" precedence="default" />

3465 + <div>hello world</div>

3466 + </BlockedOn>

3467 + </Suspense>

3468 + </div>

3469 + <div>

3470 + <Suspense fallback="loading 2...">

3471 + <Component />

3472 + </Suspense>

3473 + </div>

3474 + </body>

3475 + </html>,

3476 + );

3477 + await waitForAll([]);

3478 +

3479 + expect(getMeaningfulChildren(document)).toEqual(

3480 + <html>

3481 + <head>

3482 + <link rel="stylesheet" href="foo" data-precedence="default" />

3483 + </head>

3484 + <body>

3485 + <div>loading...</div>

3486 + <div />

3487 + <link rel="preload" href="foo" as="style" />

3488 + </body>

3489 + </html>,

3490 + );

3491 +

3492 + await expect(async () => {

3493 + loadStylesheets();

3494 + }).toErrorDev([

3495 + "Hydration failed because the server rendered HTML didn't match the client.",

3496 + ]);

3497 + assertLog(['load stylesheet: foo']);

3498 +

3499 + expect(getMeaningfulChildren(document)).toEqual(

3500 + <html>

3501 + <head>

3502 + <link rel="stylesheet" href="foo" data-precedence="default" />

3503 + </head>

3504 + <body>

3505 + <div>

3506 + <div>hello world</div>

3507 + </div>

3508 + <div>

3509 + <div>hello client</div>

3510 + </div>

3511 + <link rel="preload" href="foo" as="style" />

3512 + </body>

3513 + </html>,

3514 + );

3515 + });

3516 +

3517 it('can suspend commits on more than one root for the same resource at the same time', async () => {

3518 document.body.innerHTML = '';

3519 const container1 = document.createElement('div');