Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Fizz][Float] stop preloading stylesheets that are not stylesheet resources #26873

Merged
merged 1 commit into from
Jun 1, 2023
Merged

[Fizz][Float] stop preloading stylesheets that are not stylesheet resources #26873

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 0 additions & 47 deletions packages/react-dom-bindings/src/server/ReactFizzConfigDOM.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1965,21 +1965,6 @@ function pushLink(
}
}
}
let resource = resources.preloadsMap.get(key);
if (!resource) {
resource = {
type: 'preload',
chunks: ([]: Array<Chunk | PrecomputedChunk>),
state: NoState,
props: preloadAsStylePropsFromProps(href, props),
};
resources.preloadsMap.set(key, resource);
if (__DEV__) {
markAsImplicitResourceDEV(resource, props, resource.props);
}
}
pushLinkImpl(resource.chunks, resource.props);
resources.usedStylesheets.set(key, resource);
return pushLinkImpl(target, props);
} else {
// This stylesheet refers to a Resource and we create a new one if necessary
Expand Down Expand Up @@ -4251,21 +4236,6 @@ export function writePreamble(
// Flush unblocked stylesheets by precedence
resources.precedences.forEach(flushAllStylesInPreamble, destination);

resources.usedStylesheets.forEach((resource, key) => {
if (resources.stylesMap.has(key)) {
// The underlying stylesheet is represented both as a used stylesheet
// (a regular component we will attempt to preload) and as a StylesheetResource.
// We don't want to emit two preloads for the same href so we defer
// the preload rules of the StylesheetResource when there is a conflict
} else {
const chunks = resource.chunks;
for (i = 0; i < chunks.length; i++) {
writeChunk(destination, chunks[i]);
}
}
});
resources.usedStylesheets.clear();

resources.scripts.forEach(flushResourceInPreamble, destination);
resources.scripts.clear();

Expand Down Expand Up @@ -4346,21 +4316,6 @@ export function writeHoistables(
// but we want to kick off preloading as soon as possible
resources.precedences.forEach(preloadLateStyles, destination);

resources.usedStylesheets.forEach((resource, key) => {
if (resources.stylesMap.has(key)) {
// The underlying stylesheet is represented both as a used stylesheet
// (a regular component we will attempt to preload) and as a StylesheetResource.
// We don't want to emit two preloads for the same href so we defer
// the preload rules of the StylesheetResource when there is a conflict
} else {
const chunks = resource.chunks;
for (i = 0; i < chunks.length; i++) {
writeChunk(destination, chunks[i]);
}
}
});
resources.usedStylesheets.clear();

resources.scripts.forEach(flushResourceLate, destination);
resources.scripts.clear();

Expand Down Expand Up @@ -4917,7 +4872,6 @@ export type Resources = {
// usedImagePreloads: Set<PreloadResource>,
precedences: Map<string, Set<StyleResource>>,
stylePrecedences: Map<string, StyleTagResource>,
usedStylesheets: Map<string, PreloadResource>,
scripts: Set<ScriptResource>,
usedScripts: Set<PreloadResource>,
explicitStylesheetPreloads: Set<PreloadResource>,
Expand Down Expand Up @@ -4945,7 +4899,6 @@ export function createResources(): Resources {
// usedImagePreloads: new Set(),
precedences: new Map(),
stylePrecedences: new Map(),
usedStylesheets: new Map(),
scripts: new Set(),
usedScripts: new Set(),
explicitStylesheetPreloads: new Set(),
Expand Down
95 changes: 0 additions & 95 deletions packages/react-dom/src/__tests__/ReactDOMFloat-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3881,36 +3881,6 @@ body {
]);
});

// @gate enableFloat
it('warns if you pass incompatible options to two `ReactDOM.preload(...)` when an implicit preload already exists with the same href', async () => {
function Component() {
ReactDOM.preload('foo', {
as: 'style',
crossOrigin: 'use-credentials',
});
}

await expect(async () => {
await act(() => {
renderToPipeableStream(
<html>
<body>
<link
rel="stylesheet"
href="foo"
integrity="some hash"
media="print"
/>
<Component />
</body>
</html>,
);
});
}).toErrorDev([
'ReactDOM.preload(): For `href` "foo", The options provided conflict with props on a matching <link rel="stylesheet" ... /> element. When the preload options disagree with the underlying resource it usually means the browser will not be able to use the preload when the resource is fetched, negating any benefit the preload would provide. React will preload the resource using props derived from the resource instead and ignore the options provided to the `ReactDOM.preload()` call. In general, preloading is useful when you expect to render a resource soon but have not yet done so. In this case since the underlying resource was already rendered the preload call may be extraneous. Try removing the call, otherwise try adjusting both the props on the <link rel="stylesheet" ... /> and the options passed to `ReactDOM.preload()` to agree.\n "integrity" missing from options, underlying prop value: "some hash"\n "media" missing from options, underlying prop value: "print"\n "crossOrigin" option value: "use-credentials", missing from underlying props',
]);
});

it('supports fetchPriority', async () => {
function Component({isServer}) {
ReactDOM.preload(isServer ? 'highserver' : 'highclient', {
Expand Down Expand Up @@ -4765,71 +4735,6 @@ body {
);
});

// @gate enableFloat
it('preloads stylesheets without a precedence prop when server rendering', async () => {
await act(() => {
const {pipe} = renderToPipeableStream(
<html>
<head />
<body>
<link rel="stylesheet" href="notaresource" />
<div>hello world</div>
</body>
</html>,
);
pipe(writable);
});

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

// @gate enableFloat
it('respects attributes defined on the stylesheet element when preloading stylesheets during server rendering', async () => {
await act(() => {
const {pipe} = renderToPipeableStream(
<html>
<head>
<link rel="stylesheet" fetchPriority="high" href="foo" />
</head>
<body>
<link rel="stylesheet" fetchPriority="low" href="notaresource" />
<div>hello world</div>
</body>
</html>,
);
pipe(writable);
});

expect(getMeaningfulChildren(document)).toEqual(
<html>
<head>
<link rel="preload" as="style" fetchpriority="high" href="foo" />
<link
rel="preload"
as="style"
fetchpriority="low"
href="notaresource"
/>
<link rel="stylesheet" fetchpriority="high" href="foo" />
</head>
<body>
<link rel="stylesheet" fetchpriority="low" href="notaresource" />
<div>hello world</div>
</body>
</html>,
);
});

// @gate enableFloat
it('hoists stylesheet resources to the correct precedence', async () => {
await act(() => {
Expand Down
8 changes: 1 addition & 7 deletions packages/react-dom/src/__tests__/ReactDOMSingletonComponents-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -245,9 +245,6 @@ describe('ReactDOM HostSingleton', () => {
expect(getVisibleChildren(document)).toEqual(
<html data-foo="foo">
<head data-bar="bar">
<link rel="preload" href="resource" as="style" />
<link rel="preload" href="3rdparty" as="style" />
<link rel="preload" href="3rdparty2" as="style" />
<title>a server title</title>
<link rel="stylesheet" href="resource" />
<link rel="stylesheet" href="3rdparty" />
Expand Down Expand Up @@ -842,10 +839,7 @@ describe('ReactDOM HostSingleton', () => {
await waitForAll([]);
expect(getVisibleChildren(document)).toEqual(
<html>
<head>
<link rel="preload" as="style" href="before" />
<link rel="preload" as="style" href="after" />
</head>
<head />
<body>
<link rel="stylesheet" href="before" />
<link rel="stylesheet" href="after" />
Expand Down