Skip to content

Commit

Permalink
[Float][Fiber] Assume stylesheets in document are already loaded (che…
Browse files Browse the repository at this point in the history
…rrypick #29811) (#29835)

Cherrypick #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.
  • Loading branch information
gnoff authored Jun 10, 2024
1 parent 1df34bd commit 6230622
Show file tree
Hide file tree
Showing 5 changed files with 225 additions and 45 deletions.
72 changes: 42 additions & 30 deletions packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js
Original file line number Diff line number Diff line change
Expand Up @@ -2412,23 +2412,42 @@ export function getResource(
if (!resource) {
// We asserted this above but Flow can't figure out that the type satisfies
const ownerDocument = getDocumentFromRoot(resourceRoot);
resource = {
resource = ({
type: 'stylesheet',
instance: null,
count: 0,
state: {
loading: NotLoaded,
preload: null,
},
};
}: StylesheetResource);
styles.set(key, resource);
const instance = ownerDocument.querySelector(
getStylesheetSelectorFromKey(key),
);
if (instance) {
const loadingState: ?Promise<mixed> = (instance: any)._p;
if (loadingState) {
// This instance is inserted as part of a boundary reveal and is not yet
// loaded
} else {
// This instance is already loaded
resource.instance = instance;
resource.state.loading = Loaded | Inserted;
}
}

if (!preloadPropsMap.has(key)) {
preloadStylesheet(
ownerDocument,
key,
preloadPropsFromStylesheet(qualifiedProps),
resource.state,
);
const preloadProps = preloadPropsFromStylesheet(qualifiedProps);
preloadPropsMap.set(key, preloadProps);
if (!instance) {
preloadStylesheet(
ownerDocument,
key,
preloadProps,
resource.state,
);
}
}
}
if (currentProps && currentResource === null) {
Expand Down Expand Up @@ -2599,28 +2618,21 @@ function preloadStylesheet(
preloadProps: PreloadProps,
state: StylesheetState,
) {
preloadPropsMap.set(key, preloadProps);
if (!ownerDocument.querySelector(getStylesheetSelectorFromKey(key))) {
// There is no matching stylesheet instance in the Document.
// We will insert a preload now to kick off loading because
// we expect this stylesheet to commit
const preloadEl = ownerDocument.querySelector(
getPreloadStylesheetSelectorFromKey(key),
);
if (preloadEl) {
// If we find a preload already it was SSR'd and we won't have an actual
// loading state to track. For now we will just assume it is loaded
state.loading = Loaded;
} else {
const instance = ownerDocument.createElement('link');
state.preload = instance;
instance.addEventListener('load', () => (state.loading |= Loaded));
instance.addEventListener('error', () => (state.loading |= Errored));
setInitialProperties(instance, 'link', preloadProps);
markNodeAsHoistable(instance);
(ownerDocument.head: any).appendChild(instance);
}
const preloadEl = ownerDocument.querySelector(
getPreloadStylesheetSelectorFromKey(key),
);
if (preloadEl) {
// If we find a preload already it was SSR'd and we won't have an actual
// loading state to track. For now we will just assume it is loaded
state.loading = Loaded;
} else {
const instance = ownerDocument.createElement('link');
state.preload = instance;
instance.addEventListener('load', () => (state.loading |= Loaded));
instance.addEventListener('error', () => (state.loading |= Errored));
setInitialProperties(instance, 'link', preloadProps);
markNodeAsHoistable(instance);
(ownerDocument.head: any).appendChild(instance);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,11 @@ export function completeBoundaryWithStyles(
const dependencies = [];
let href, precedence, attr, loadingState, resourceEl, media;

function cleanupWith(cb) {
this['_p'] = null;
cb();
}

// Sheets Mode
let sheetMode = true;
while (true) {
Expand Down Expand Up @@ -80,18 +85,14 @@ export function completeBoundaryWithStyles(
resourceEl.setAttribute(attr, stylesheetDescriptor[j++]);
}
loadingState = resourceEl['_p'] = new Promise((resolve, reject) => {
resourceEl.onload = resolve;
resourceEl.onerror = reject;
resourceEl.onload = cleanupWith.bind(resourceEl, resolve);
resourceEl.onerror = cleanupWith.bind(resourceEl, reject);
});
// Save this resource element so we can bailout if it is used again
resourceMap.set(href, resourceEl);
}
media = resourceEl.getAttribute('media');
if (
loadingState &&
loadingState['s'] !== 'l' &&
(!media || window['matchMedia'](media).matches)
) {
if (loadingState && (!media || window['matchMedia'](media).matches)) {
dependencies.push(loadingState);
}
if (avoidInsert) {
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,11 @@ export function completeBoundaryWithStyles(
const dependencies = [];
let href, precedence, attr, loadingState, resourceEl, media;

function cleanupWith(cb) {
this['_p'] = null;
cb();
}

// Sheets Mode
let sheetMode = true;
while (true) {
Expand Down Expand Up @@ -82,18 +87,14 @@ export function completeBoundaryWithStyles(
resourceEl.setAttribute(attr, stylesheetDescriptor[j++]);
}
loadingState = resourceEl['_p'] = new Promise((resolve, reject) => {
resourceEl.onload = resolve;
resourceEl.onerror = reject;
resourceEl.onload = cleanupWith.bind(resourceEl, resolve);
resourceEl.onerror = cleanupWith.bind(resourceEl, reject);
});
// Save this resource element so we can bailout if it is used again
resourceMap.set(href, resourceEl);
}
media = resourceEl.getAttribute('media');
if (
loadingState &&
loadingState['s'] !== 'l' &&
(!media || window['matchMedia'](media).matches)
) {
if (loadingState && (!media || window['matchMedia'](media).matches)) {
dependencies.push(loadingState);
}
if (avoidInsert) {
Expand Down
166 changes: 166 additions & 0 deletions packages/react-dom/src/__tests__/ReactDOMFloat-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -3348,6 +3348,172 @@ body {
);
});

it('will assume stylesheets already in the document have loaded if it cannot confirm it is not yet loaded', async () => {
await act(() => {
renderToPipeableStream(
<html>
<head>
<link rel="stylesheet" href="foo" data-precedence="default" />
</head>
<body>
<div id="foo" />
</body>
</html>,
).pipe(writable);
});

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

root.render(
<div>
<Suspense fallback="loading...">
<link rel="stylesheet" href="foo" precedence="default" />
hello world
</Suspense>
</div>,
);

await waitForAll([]);
expect(getMeaningfulChildren(document)).toEqual(
<html>
<head>
<link rel="stylesheet" href="foo" data-precedence="default" />
</head>
<body>
<div id="foo">
<div>hello world</div>
</div>
</body>
</html>,
);
});

it('will assume wait for loading stylesheets to load before continuing', async () => {
let ssr = true;
function Component() {
if (ssr) {
return null;
} else {
return (
<>
<link rel="stylesheet" href="foo" precedence="default" />
<div>hello client</div>
</>
);
}
}

await act(() => {
renderToPipeableStream(
<html>
<body>
<div>
<Suspense fallback="loading...">
<BlockedOn value="reveal">
<link rel="stylesheet" href="foo" precedence="default" />
<div>hello world</div>
</BlockedOn>
</Suspense>
</div>
<div>
<Suspense fallback="loading 2...">
<Component />
</Suspense>
</div>
</body>
</html>,
).pipe(writable);
});

expect(getMeaningfulChildren(document)).toEqual(
<html>
<head />
<body>
<div>loading...</div>
<div />
</body>
</html>,
);

await act(() => {
resolveText('reveal');
});

expect(getMeaningfulChildren(document)).toEqual(
<html>
<head>
<link rel="stylesheet" href="foo" data-precedence="default" />
</head>
<body>
<div>loading...</div>
<div />
<link rel="preload" href="foo" as="style" />
</body>
</html>,
);

ssr = false;

ReactDOMClient.hydrateRoot(
document,
<html>
<body>
<div>
<Suspense fallback="loading...">
<BlockedOn value="reveal">
<link rel="stylesheet" href="foo" precedence="default" />
<div>hello world</div>
</BlockedOn>
</Suspense>
</div>
<div>
<Suspense fallback="loading 2...">
<Component />
</Suspense>
</div>
</body>
</html>,
);
await waitForAll([]);

expect(getMeaningfulChildren(document)).toEqual(
<html>
<head>
<link rel="stylesheet" href="foo" data-precedence="default" />
</head>
<body>
<div>loading...</div>
<div />
<link rel="preload" href="foo" as="style" />
</body>
</html>,
);

await expect(async () => {
loadStylesheets();
}).toErrorDev([
"Hydration failed because the server rendered HTML didn't match the client.",
]);
assertLog(['load stylesheet: foo']);

expect(getMeaningfulChildren(document)).toEqual(
<html>
<head>
<link rel="stylesheet" href="foo" data-precedence="default" />
</head>
<body>
<div>
<div>hello world</div>
</div>
<div>
<div>hello client</div>
</div>
<link rel="preload" href="foo" as="style" />
</body>
</html>,
);
});

it('can suspend commits on more than one root for the same resource at the same time', async () => {
document.body.innerHTML = '';
const container1 = document.createElement('div');
Expand Down

0 comments on commit 6230622

Please sign in to comment.