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(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(hello world
'); });