From 5b099d81f8deeff481effc59938d5e59a287115c Mon Sep 17 00:00:00 2001 From: Josh Story Date: Tue, 30 May 2023 14:53:23 -0700 Subject: [PATCH] We previously preloaded stylesheets that were rendered in Fizz. The idea was we'd get a headstart fetching these resources since we know they are going to be rendered. However to really be effective non-float stylesheets need to rendered in the head and the preload here is not helpful and potentially hurtful to perf in a minor way. This change removes this functionality to make the code smaller and simpler --- .../src/server/ReactFizzConfigDOM.js | 49 ---------- .../src/__tests__/ReactDOMFloat-test.js | 95 ------------------- .../ReactDOMSingletonComponents-test.js | 8 +- 3 files changed, 1 insertion(+), 151 deletions(-) diff --git a/packages/react-dom-bindings/src/server/ReactFizzConfigDOM.js b/packages/react-dom-bindings/src/server/ReactFizzConfigDOM.js index 5314564242ed9..a20abb62b2201 100644 --- a/packages/react-dom-bindings/src/server/ReactFizzConfigDOM.js +++ b/packages/react-dom-bindings/src/server/ReactFizzConfigDOM.js @@ -1960,21 +1960,6 @@ function pushLink( } } } - let resource = resources.preloadsMap.get(key); - if (!resource) { - resource = { - type: 'preload', - chunks: ([]: Array), - 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.add(resource); return pushLinkImpl(target, props); } else { // This stylesheet refers to a Resource and we create a new one if necessary @@ -4246,22 +4231,6 @@ export function writePreamble( // Flush unblocked stylesheets by precedence resources.precedences.forEach(flushAllStylesInPreamble, destination); - resources.usedStylesheets.forEach(resource => { - const key = getResourceKey(resource.props.as, resource.props.href); - 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(); @@ -4342,22 +4311,6 @@ export function writeHoistables( // but we want to kick off preloading as soon as possible resources.precedences.forEach(preloadLateStyles, destination); - resources.usedStylesheets.forEach(resource => { - const key = getResourceKey(resource.props.as, resource.props.href); - 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(); @@ -4908,7 +4861,6 @@ export type Resources = { // usedImagePreloads: Set, precedences: Map>, stylePrecedences: Map, - usedStylesheets: Set, scripts: Set, usedScripts: Set, explicitStylesheetPreloads: Set, @@ -4936,7 +4888,6 @@ export function createResources(): Resources { // usedImagePreloads: new Set(), precedences: new Map(), stylePrecedences: new Map(), - usedStylesheets: new Set(), scripts: new Set(), usedScripts: new Set(), explicitStylesheetPreloads: new Set(), diff --git a/packages/react-dom/src/__tests__/ReactDOMFloat-test.js b/packages/react-dom/src/__tests__/ReactDOMFloat-test.js index b0303165d0dba..e6ff142d1a833 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFloat-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFloat-test.js @@ -3879,36 +3879,6 @@ body { 'Warning: ReactDOM.preload(): The options provided conflict with another call to `ReactDOM.preload("foo", { as: "font", ...})`. React will always use the options it first encounters when preloading a resource for a given `href` and `as` type, and any later options will be ignored if different. Try updating all calls to `ReactDOM.preload()` with the same `href` and `as` type to use the same options, or eliminate one of the calls.\n "crossOrigin" missing from options, original option value: "use-credentials"', ]); }); - - // @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( - - - - - - , - ); - }); - }).toErrorDev([ - 'ReactDOM.preload(): For `href` "foo", The options provided conflict with props on a matching 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 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', - ]); - }); }); describe('ReactDOM.preinit(href, { as: ... })', () => { @@ -4568,71 +4538,6 @@ body { ); }); - // @gate enableFloat - it('preloads stylesheets without a precedence prop when server rendering', async () => { - await act(() => { - const {pipe} = renderToPipeableStream( - - - - -
hello world
- - , - ); - pipe(writable); - }); - - expect(getMeaningfulChildren(document)).toEqual( - - - - - - -
hello world
- - , - ); - }); - - // @gate enableFloat - it('respects attributes defined on the stylesheet element when preloading stylesheets during server rendering', async () => { - await act(() => { - const {pipe} = renderToPipeableStream( - - - - - - -
hello world
- - , - ); - pipe(writable); - }); - - expect(getMeaningfulChildren(document)).toEqual( - - - - - - - - -
hello world
- - , - ); - }); - // @gate enableFloat it('hoists stylesheet resources to the correct precedence', async () => { await act(() => { diff --git a/packages/react-dom/src/__tests__/ReactDOMSingletonComponents-test.js b/packages/react-dom/src/__tests__/ReactDOMSingletonComponents-test.js index 4d34f2edb7d56..7d1bddbe661c2 100644 --- a/packages/react-dom/src/__tests__/ReactDOMSingletonComponents-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMSingletonComponents-test.js @@ -245,9 +245,6 @@ describe('ReactDOM HostSingleton', () => { expect(getVisibleChildren(document)).toEqual( - - - a server title @@ -842,10 +839,7 @@ describe('ReactDOM HostSingleton', () => { await waitForAll([]); expect(getVisibleChildren(document)).toEqual( - - - - +