From f359f9b41ac1a8127f5ba505e0c04675eee0d310 Mon Sep 17 00:00:00 2001 From: Josh Story Date: Thu, 10 Aug 2023 15:39:19 -0700 Subject: [PATCH] [Fizz] Preload "suspensey" images (#27191) Eventually we will treat images without `loading="lazy"` as suspensey meaning we will coordinate the reveal of boundaries when these images have loaded and ideally decoded. As a step in that direction this change prioritizes these images for preloading to ensure the highest chance that they are loaded before boundaries reveal (or initial paint). every img rendered that is non lazy loading will emit a preload just behind fonts. This change implements a new resource queue for high priority image preloads There are a number of scenarios where we end up putting a preload in this queue 1. If you render a non-lazy image and there are fewer than 10 high priority image preloads 2. if you render a non-lazy image with fetchPriority "high" 3. if you preload as "image" with fetchPriority "high" This means that by default we won't overrsaturate this queue with every img rendered on the page but the earlier encountered ones will go first. Essentially this is React's own implementation of fetchPriority="auto". If however you specify that the fetchPriority is higher then in theory an unlimited number of images can preload in this queue. This gives users some control over queuing while still providing a good default that does not require any opting into Additionally we use fetchPriority "low" as a signal that an image does not require preloading. This may end up being pointless if not using lazy (which also opts out of preloading) because it might delay initial paint but we'll start with this hueristic and consider changes in the future when we have more information --- .../src/server/ReactFizzConfigDOM.js | 111 +++++++++++-- .../src/__tests__/ReactDOMFloat-test.js | 157 ++++++++++++++++++ 2 files changed, 250 insertions(+), 18 deletions(-) diff --git a/packages/react-dom-bindings/src/server/ReactFizzConfigDOM.js b/packages/react-dom-bindings/src/server/ReactFizzConfigDOM.js index 45bec2931d67..e335dda9aa59 100644 --- a/packages/react-dom-bindings/src/server/ReactFizzConfigDOM.js +++ b/packages/react-dom-bindings/src/server/ReactFizzConfigDOM.js @@ -2370,6 +2370,78 @@ function pushStyleContents( return; } +function getImagePreloadKey( + href: string, + imageSrcSet: ?string, + imageSizes: ?string, +) { + let uniquePart = ''; + if (typeof imageSrcSet === 'string' && imageSrcSet !== '') { + uniquePart += '[' + imageSrcSet + ']'; + if (typeof imageSizes === 'string') { + uniquePart += '[' + imageSizes + ']'; + } + } else { + uniquePart += '[][]' + href; + } + return getResourceKey('image', uniquePart); +} + +function pushImg( + target: Array, + props: Object, + resources: Resources, +): null { + if ( + props.loading !== 'lazy' && + typeof props.src === 'string' && + props.fetchPriority !== 'low' + ) { + // We have a suspensey image and ought to preload it to optimize the loading of display blocking + // resources. + const {src, imageSrcSet, imageSizes} = props; + const key = getImagePreloadKey(src, imageSrcSet, imageSizes); + let resource = resources.preloadsMap.get(key); + if (!resource) { + resource = { + type: 'preload', + chunks: [], + state: NoState, + props: { + rel: 'preload', + as: 'image', + // There is a bug in Safari where imageSrcSet is not respected on preload links + // so we omit the href here if we have imageSrcSet b/c safari will load the wrong image. + // This harms older browers that do not support imageSrcSet by making their preloads not work + // but this population is shrinking fast and is already small so we accept this tradeoff. + href: imageSrcSet ? undefined : src, + imageSrcSet, + imageSizes, + crossOrigin: props.crossOrigin, + integrity: props.integrity, + type: props.type, + fetchPriority: props.fetchPriority, + referrerPolicy: props.referrerPolicy, + }, + }; + resources.preloadsMap.set(key, resource); + if (__DEV__) { + markAsRenderedResourceDEV(resource, props); + } + pushLinkImpl(resource.chunks, resource.props); + } + if ( + props.fetchPriority === 'high' || + resources.highImagePreloads.size < 10 + ) { + resources.highImagePreloads.add(resource); + } else { + resources.bulkPreloads.add(resource); + } + } + return pushSelfClosing(target, props, 'img'); +} + function pushSelfClosing( target: Array, props: Object, @@ -3172,6 +3244,9 @@ export function pushStartInstance( case 'pre': { return pushStartPreformattedElement(target, props, type); } + case 'img': { + return pushImg(target, props, resources); + } // Omitted close tags case 'base': case 'area': @@ -3179,7 +3254,6 @@ export function pushStartInstance( case 'col': case 'embed': case 'hr': - case 'img': case 'keygen': case 'param': case 'source': @@ -4242,6 +4316,9 @@ export function writePreamble( resources.fontPreloads.forEach(flushResourceInPreamble, destination); resources.fontPreloads.clear(); + resources.highImagePreloads.forEach(flushResourceInPreamble, destination); + resources.highImagePreloads.clear(); + // Flush unblocked stylesheets by precedence resources.precedences.forEach(flushAllStylesInPreamble, destination); @@ -4250,8 +4327,8 @@ export function writePreamble( resources.scripts.forEach(flushResourceInPreamble, destination); resources.scripts.clear(); - resources.explicitPreloads.forEach(flushResourceInPreamble, destination); - resources.explicitPreloads.clear(); + resources.bulkPreloads.forEach(flushResourceInPreamble, destination); + resources.bulkPreloads.clear(); // Write embedding preloadChunks const preloadChunks = responseState.preloadChunks; @@ -4308,6 +4385,9 @@ export function writeHoistables( resources.fontPreloads.forEach(flushResourceLate, destination); resources.fontPreloads.clear(); + resources.highImagePreloads.forEach(flushResourceInPreamble, destination); + resources.highImagePreloads.clear(); + // Preload any stylesheets. these will emit in a render instruction that follows this // but we want to kick off preloading as soon as possible resources.precedences.forEach(preloadLateStyles, destination); @@ -4318,8 +4398,8 @@ export function writeHoistables( resources.scripts.forEach(flushResourceLate, destination); resources.scripts.clear(); - resources.explicitPreloads.forEach(flushResourceLate, destination); - resources.explicitPreloads.clear(); + resources.bulkPreloads.forEach(flushResourceLate, destination); + resources.bulkPreloads.clear(); // Write embedding preloadChunks const preloadChunks = responseState.preloadChunks; @@ -4859,12 +4939,13 @@ export type Resources = { // Flushing queues for Resource dependencies preconnects: Set, fontPreloads: Set, + highImagePreloads: Set, // usedImagePreloads: Set, precedences: Map>, stylePrecedences: Map, bootstrapScripts: Set, scripts: Set, - explicitPreloads: Set, + bulkPreloads: Set, // Module-global-like reference for current boundary resources boundaryResources: ?BoundaryResources, @@ -4883,12 +4964,13 @@ export function createResources(): Resources { // cleared on flush preconnects: new Set(), fontPreloads: new Set(), + highImagePreloads: new Set(), // usedImagePreloads: new Set(), precedences: new Map(), stylePrecedences: new Map(), bootstrapScripts: new Set(), scripts: new Set(), - explicitPreloads: new Set(), + bulkPreloads: new Set(), // like a module global for currently rendering boundary boundaryResources: null, @@ -5086,16 +5168,7 @@ export function preload(href: string, options: PreloadOptions) { // both. This is to prevent identical calls with the same srcSet and sizes to be duplicated // by varying the href. this is an edge case but it is the most correct behavior. const {imageSrcSet, imageSizes} = options; - let uniquePart = ''; - if (typeof imageSrcSet === 'string' && imageSrcSet !== '') { - uniquePart += '[' + imageSrcSet + ']'; - if (typeof imageSizes === 'string') { - uniquePart += '[' + imageSizes + ']'; - } - } else { - uniquePart += '[][]' + href; - } - key = getResourceKey(as, uniquePart); + key = getImagePreloadKey(href, imageSrcSet, imageSizes); } else { key = getResourceKey(as, href); } @@ -5177,8 +5250,10 @@ export function preload(href: string, options: PreloadOptions) { } if (as === 'font') { resources.fontPreloads.add(resource); + } else if (as === 'image' && options.fetchPriority === 'high') { + resources.highImagePreloads.add(resource); } else { - resources.explicitPreloads.add(resource); + resources.bulkPreloads.add(resource); } flushResources(request); } diff --git a/packages/react-dom/src/__tests__/ReactDOMFloat-test.js b/packages/react-dom/src/__tests__/ReactDOMFloat-test.js index fd8a183f248c..f48741a5e8d4 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFloat-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFloat-test.js @@ -3771,6 +3771,163 @@ body { ); }); + it('can emit preloads for non-lazy images that are rendered', async () => { + function App() { + ReactDOM.preload('script', {as: 'script'}); + ReactDOM.preload('a', {as: 'image'}); + ReactDOM.preload('b', {as: 'image'}); + return ( + + + + + + + + + + + ); + } + + await act(() => { + renderToPipeableStream().pipe(writable); + }); + + // non-lazy images are first, then arbitrary preloads like for the script and lazy images + expect(getMeaningfulChildren(document)).toEqual( + + + + + + + + + + + + + + + + + + , + ); + }); + + it('Does not preload lazy images', async () => { + function App() { + ReactDOM.preload('a', {as: 'image'}); + return ( + + + + + + + ); + } + await act(() => { + renderToPipeableStream().pipe(writable); + }); + + expect(getMeaningfulChildren(document)).toEqual( + + + + + + + + + , + ); + }); + + it('preloads up to 10 suspensey images as high priority when fetchPriority is not specified', async () => { + function App() { + ReactDOM.preload('1', {as: 'image', fetchPriority: 'high'}); + ReactDOM.preload('auto', {as: 'image'}); + ReactDOM.preload('low', {as: 'image', fetchPriority: 'low'}); + ReactDOM.preload('9', {as: 'image', fetchPriority: 'high'}); + ReactDOM.preload('10', {as: 'image', fetchPriority: 'high'}); + return ( + + + {/* skipping 1 */} + + + + + + + + + + {/* skipping 10 */} + + + + + ); + } + await act(() => { + renderToPipeableStream().pipe(writable); + }); + + expect(getMeaningfulChildren(document)).toEqual( + + + {/* First we see the preloads calls that made it to the high priority image queue */} + + + + {/* Next we see up to 7 more images qualify for high priority image queue */} + + + + + + + + {/* Next we see images that are explicitly high priority and thus make it to the high priority image queue */} + + {/* Next we see the remaining preloads that did not make it to the high priority image queue */} + + + + + + {/* skipping 1 */} + + + + + + + + + + {/* skipping 10 */} + + + + , + ); + }); + describe('ReactDOM.prefetchDNS(href)', () => { it('creates a dns-prefetch resource when called', async () => { function App({url}) {