diff --git a/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js b/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js index 6e884c0b1f15f..94698a8d527a9 100644 --- a/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js +++ b/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js @@ -105,7 +105,6 @@ import { } from 'react-reconciler/src/ReactWorkTags'; import {listenToAllSupportedEvents} from '../events/DOMPluginEventSystem'; import { - validatePreloadArguments, validatePreinitArguments, validateLinkPropsForStyleResource, getValueDescriptorExpectingObjectForWarning, @@ -2016,7 +2015,7 @@ type ScriptProps = { type PreloadProps = { rel: 'preload', - href: string, + href: ?string, [string]: mixed, }; @@ -2167,7 +2166,29 @@ function preload(href: string, options: PreloadOptions) { return; } if (__DEV__) { - validatePreloadArguments(href, options); + // TODO move this to ReactDOMFloat and expose a stricter function interface or possibly + // typed functions (preloadImage, preloadStyle, ...) + let encountered = ''; + if (typeof href !== 'string' || !href) { + encountered += `The \`href\` argument encountered was ${getValueDescriptorExpectingObjectForWarning( + href, + )}.`; + } + if (options == null || typeof options !== 'object') { + encountered += `The \`options\` argument encountered was ${getValueDescriptorExpectingObjectForWarning( + options, + )}.`; + } else if (typeof options.as !== 'string' || !options.as) { + encountered += `The \`as\` option encountered was ${getValueDescriptorExpectingObjectForWarning( + options.as, + )}.`; + } + if (encountered) { + console.error( + 'ReactDOM.preload(): Expected two arguments, a non-empty `href` string and an `options` object with an `as` property valid for a `` tag. %s', + encountered, + ); + } } const ownerDocument = getDocumentForImperativeFloatMethods(); if ( @@ -2175,13 +2196,35 @@ function preload(href: string, options: PreloadOptions) { href && typeof options === 'object' && options !== null && + typeof options.as === 'string' && + options.as && ownerDocument ) { const as = options.as; - const limitedEscapedHref = - escapeSelectorAttributeValueInsideDoubleQuotes(href); - const preloadSelector = `link[rel="preload"][as="${as}"][href="${limitedEscapedHref}"]`; - + let preloadSelector = `link[rel="preload"][as="${escapeSelectorAttributeValueInsideDoubleQuotes( + as, + )}"]`; + if (as === 'image') { + const {imageSrcSet, imageSizes} = options; + if (typeof imageSrcSet === 'string' && imageSrcSet !== '') { + preloadSelector += `[imagesrcset="${escapeSelectorAttributeValueInsideDoubleQuotes( + imageSrcSet, + )}"]`; + if (typeof imageSizes === 'string') { + preloadSelector += `[imagesizes="${escapeSelectorAttributeValueInsideDoubleQuotes( + imageSizes, + )}"]`; + } + } else { + preloadSelector += `[href="${escapeSelectorAttributeValueInsideDoubleQuotes( + href, + )}"]`; + } + } else { + preloadSelector += `[href="${escapeSelectorAttributeValueInsideDoubleQuotes( + href, + )}"]`; + } // Some preloads are keyed under their selector. This happens when the preload is for // an arbitrary type. Other preloads are keyed under the resource key they represent a preload for. // Here we figure out which key to use to determine if we have a preload already. @@ -2227,14 +2270,20 @@ function preloadPropsFromPreloadOptions( options: PreloadOptions, ): PreloadProps { return { - href, rel: 'preload', as, + // 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: as === 'image' && options.imageSrcSet ? undefined : href, crossOrigin: as === 'font' ? '' : options.crossOrigin, integrity: options.integrity, type: options.type, nonce: options.nonce, fetchPriority: options.fetchPriority, + imageSrcSet: options.imageSrcSet, + imageSizes: options.imageSizes, }; } diff --git a/packages/react-dom-bindings/src/server/ReactFizzConfigDOM.js b/packages/react-dom-bindings/src/server/ReactFizzConfigDOM.js index 4d23f0ff1afb1..b6e1e90507d5b 100644 --- a/packages/react-dom-bindings/src/server/ReactFizzConfigDOM.js +++ b/packages/react-dom-bindings/src/server/ReactFizzConfigDOM.js @@ -4819,12 +4819,12 @@ type PreconnectResource = TResource<'preconnect', null>; type PreloadAsProps = { rel: 'preload', as: string, - href: string, + href: ?string, [string]: mixed, }; type PreloadModuleProps = { rel: 'modulepreload', - href: string, + href: ?string, [string]: mixed, }; type PreloadProps = PreloadAsProps | PreloadModuleProps; @@ -5063,20 +5063,25 @@ export function preload(href: string, options: PreloadOptions) { } const resources = getResources(request); if (__DEV__) { + let encountered = ''; if (typeof href !== 'string' || !href) { + encountered += ` The \`href\` argument encountered was ${getValueDescriptorExpectingObjectForWarning( + href, + )}.`; + } + if (options == null || typeof options !== 'object') { + encountered += ` The \`options\` argument encountered was ${getValueDescriptorExpectingObjectForWarning( + options, + )}.`; + } else if (typeof options.as !== 'string' || !options.as) { + encountered += ` The \`as\` option encountered was ${getValueDescriptorExpectingObjectForWarning( + options.as, + )}.`; + } + if (encountered) { console.error( - 'ReactDOM.preload(): Expected the `href` argument (first) to be a non-empty string but encountered %s instead.', - getValueDescriptorExpectingObjectForWarning(href), - ); - } else if (options == null || typeof options !== 'object') { - console.error( - 'ReactDOM.preload(): Expected the `options` argument (second) to be an object with an `as` property describing the type of resource to be preloaded but encountered %s instead.', - getValueDescriptorExpectingEnumForWarning(options), - ); - } else if (typeof options.as !== 'string') { - console.error( - 'ReactDOM.preload(): Expected the `as` property in the `options` argument (second) to contain a string value describing the type of resource to be preloaded but encountered %s instead. Values that are valid in for the `as` attribute of a `` tag are valid here.', - getValueDescriptorExpectingEnumForWarning(options.as), + 'ReactDOM.preload(): Expected two arguments, a non-empty `href` string and an `options` object with an `as` property valid for a `` tag.%s', + encountered, ); } } @@ -5085,10 +5090,29 @@ export function preload(href: string, options: PreloadOptions) { href && typeof options === 'object' && options !== null && - typeof options.as === 'string' + typeof options.as === 'string' && + options.as ) { const as = options.as; - const key = getResourceKey(as, href); + let key: string; + if (as === 'image') { + // For image preloads the key contains either the imageSrcSet + imageSizes or the href but not + // 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); + } else { + key = getResourceKey(as, href); + } let resource = resources.preloadsMap.get(key); if (__DEV__) { const devResource = getAsResourceDEV(resource); @@ -5528,12 +5552,18 @@ function preloadPropsFromPreloadOptions( return { rel: 'preload', as, - href, + // 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: as === 'image' && options.imageSrcSet ? undefined : href, crossOrigin: as === 'font' ? '' : options.crossOrigin, integrity: options.integrity, type: options.type, nonce: options.nonce, fetchPriority: options.fetchPriority, + imageSrcSet: options.imageSrcSet, + imageSizes: options.imageSizes, }; } diff --git a/packages/react-dom-bindings/src/shared/ReactDOMResourceValidation.js b/packages/react-dom-bindings/src/shared/ReactDOMResourceValidation.js index 71f48665bb4cc..20367f428355c 100644 --- a/packages/react-dom-bindings/src/shared/ReactDOMResourceValidation.js +++ b/packages/react-dom-bindings/src/shared/ReactDOMResourceValidation.js @@ -62,60 +62,6 @@ function propNamesListJoin( } } -export function validatePreloadArguments(href: mixed, options: mixed) { - if (__DEV__) { - if (!href || typeof href !== 'string') { - const typeOfArg = getValueDescriptorExpectingObjectForWarning(href); - console.error( - 'ReactDOM.preload() expected the first argument to be a string representing an href but found %s instead.', - typeOfArg, - ); - } else if (typeof options !== 'object' || options === null) { - const typeOfArg = getValueDescriptorExpectingObjectForWarning(options); - console.error( - 'ReactDOM.preload() expected the second argument to be an options argument containing at least an "as" property' + - ' specifying the Resource type. It found %s instead. The href for the preload call where this warning originated is "%s".', - typeOfArg, - href, - ); - } else { - const as = options.as; - switch (as) { - // Font specific validation of options - case 'font': { - if (options.crossOrigin === 'use-credentials') { - console.error( - 'ReactDOM.preload() was called with an "as" type of "font" and with a "crossOrigin" option of "use-credentials".' + - ' Fonts preloading must use crossOrigin "anonymous" to be functional. Please update your font preload to omit' + - ' the crossOrigin option or change it to any other value than "use-credentials" (Browsers default all other values' + - ' to anonymous mode). The href for the preload call where this warning originated is "%s"', - href, - ); - } - break; - } - case 'script': - case 'style': { - break; - } - - // We have an invalid as type and need to warn - default: { - const typeOfAs = getValueDescriptorExpectingEnumForWarning(as); - console.error( - 'ReactDOM.preload() expected a valid "as" type in the options (second) argument but found %s instead.' + - ' Please use one of the following valid values instead: %s. The href for the preload call where this' + - ' warning originated is "%s".', - typeOfAs, - '"style", "font", or "script"', - href, - ); - } - } - } - } -} - export function validatePreinitArguments(href: mixed, options: mixed) { if (__DEV__) { if (!href || typeof href !== 'string') { diff --git a/packages/react-dom/src/__tests__/ReactDOMFloat-test.js b/packages/react-dom/src/__tests__/ReactDOMFloat-test.js index b73bc3f41e13a..40b3a61e8e730 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFloat-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFloat-test.js @@ -3545,6 +3545,147 @@ body { ); }); + it('uses imageSrcSet and imageSizes when keying image preloads', async () => { + function App({isClient}) { + // Will key off href in absense of imageSrcSet + ReactDOM.preload('foo', {as: 'image'}); + ReactDOM.preload('foo', {as: 'image'}); + + // Will key off imageSrcSet + imageSizes + ReactDOM.preload('foo', {as: 'image', imageSrcSet: 'fooset'}); + ReactDOM.preload('foo2', {as: 'image', imageSrcSet: 'fooset'}); + + // Will key off imageSrcSet + imageSizes + ReactDOM.preload('foo', { + as: 'image', + imageSrcSet: 'fooset', + imageSizes: 'foosizes', + }); + ReactDOM.preload('foo2', { + as: 'image', + imageSrcSet: 'fooset', + imageSizes: 'foosizes', + }); + + // Will key off href in absense of imageSrcSet, imageSizes is ignored. these should match the + // first preloads not not emit a new preload tag + ReactDOM.preload('foo', {as: 'image', imageSizes: 'foosizes'}); + ReactDOM.preload('foo', {as: 'image', imageSizes: 'foosizes'}); + + // These preloads are for something that isn't an image + // They should all key off the href + ReactDOM.preload('bar', {as: 'somethingelse'}); + ReactDOM.preload('bar', { + as: 'somethingelse', + imageSrcSet: 'makes no sense', + }); + ReactDOM.preload('bar', { + as: 'somethingelse', + imageSrcSet: 'makes no sense', + imageSizes: 'makes no sense', + }); + + if (isClient) { + // Will key off href in absense of imageSrcSet + ReactDOM.preload('client', {as: 'image'}); + ReactDOM.preload('client', {as: 'image'}); + + // Will key off imageSrcSet + imageSizes + ReactDOM.preload('client', {as: 'image', imageSrcSet: 'clientset'}); + ReactDOM.preload('client2', {as: 'image', imageSrcSet: 'clientset'}); + + // Will key off imageSrcSet + imageSizes + ReactDOM.preload('client', { + as: 'image', + imageSrcSet: 'clientset', + imageSizes: 'clientsizes', + }); + ReactDOM.preload('client2', { + as: 'image', + imageSrcSet: 'clientset', + imageSizes: 'clientsizes', + }); + + // Will key off href in absense of imageSrcSet, imageSizes is ignored. these should match the + // first preloads not not emit a new preload tag + ReactDOM.preload('client', {as: 'image', imageSizes: 'clientsizes'}); + ReactDOM.preload('client', {as: 'image', imageSizes: 'clientsizes'}); + } + + return ( + + hello + + ); + } + + await act(() => { + renderToPipeableStream().pipe(writable); + }); + expect(getMeaningfulChildren(document)).toEqual( + + + + + + + + hello + , + ); + + const root = ReactDOMClient.hydrateRoot(document, ); + await waitForAll([]); + expect(getMeaningfulChildren(document)).toEqual( + + + + + + + + hello + , + ); + + root.render(); + await waitForAll([]); + expect(getMeaningfulChildren(document)).toEqual( + + + + + + + + + + + hello + , + ); + }); + describe('ReactDOM.prefetchDNS(href)', () => { it('creates a dns-prefetch resource when called', async () => { function App({url}) { @@ -3834,10 +3975,10 @@ body { renderToPipeableStream().pipe(writable); }); }).toErrorDev([ - 'ReactDOM.preload(): Expected the `href` argument (first) to be a non-empty string but encountered `undefined` instead.', - 'ReactDOM.preload(): Expected the `href` argument (first) to be a non-empty string but encountered an empty string instead.', - 'ReactDOM.preload(): Expected the `options` argument (second) to be an object with an `as` property describing the type of resource to be preloaded but encountered `null` instead.', - 'ReactDOM.preload(): Expected the `as` property in the `options` argument (second) to contain a string value describing the type of resource to be preloaded but encountered `undefined` instead. Values that are valid in for the `as` attribute of a `` tag are valid here.', + 'ReactDOM.preload(): Expected two arguments, a non-empty `href` string and an `options` object with an `as` property valid for a `` tag. The `href` argument encountered was `undefined`. The `options` argument encountered was `undefined`.', + 'ReactDOM.preload(): Expected two arguments, a non-empty `href` string and an `options` object with an `as` property valid for a `` tag. The `href` argument encountered was an empty string. The `options` argument encountered was `undefined`.', + 'ReactDOM.preload(): Expected two arguments, a non-empty `href` string and an `options` object with an `as` property valid for a `` tag. The `options` argument encountered was `null`.', + 'ReactDOM.preload(): Expected two arguments, a non-empty `href` string and an `options` object with an `as` property valid for a `` tag. The `as` option encountered was `undefined`.', ]); }); diff --git a/packages/react-dom/src/shared/ReactDOMTypes.js b/packages/react-dom/src/shared/ReactDOMTypes.js index 61d220bf11b52..7c6e9a49ff518 100644 --- a/packages/react-dom/src/shared/ReactDOMTypes.js +++ b/packages/react-dom/src/shared/ReactDOMTypes.js @@ -16,6 +16,8 @@ export type PreloadOptions = { type?: string, nonce?: string, fetchPriority?: 'high' | 'low' | 'auto', + imageSrcSet?: string, + imageSizes?: string, }; export type PreinitOptions = { as: string, diff --git a/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOM-test.js b/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOM-test.js index 32224491e4aff..8254a25c1a071 100644 --- a/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOM-test.js +++ b/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOM-test.js @@ -1190,8 +1190,8 @@ describe('ReactFlightDOM', () => { root.render(); }); expect(document.head.innerHTML).toBe( - '' + - '', + '' + + '', ); expect(container.innerHTML).toBe('

hello world

'); }); diff --git a/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOMBrowser-test.js b/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOMBrowser-test.js index b1e41f2a232f7..b2cb66f5bb817 100644 --- a/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOMBrowser-test.js +++ b/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOMBrowser-test.js @@ -1101,7 +1101,7 @@ describe('ReactFlightDOMBrowser', () => { root.render(); }); expect(document.head.innerHTML).toBe( - '', + '', ); expect(container.innerHTML).toBe('

hello world

'); });