Skip to content

Commit

Permalink
extends image preloading to accept imagesrcset and imagesizes to hand…
Browse files Browse the repository at this point in the history 
…le cases where srcset and sizes are used over src. Normally we key off an href however in the case of responsive images it is possible that no src attribute is desired. Right now this happens most prominently with Safari which does understand srcset for the img tag but not for the link preload tag. Because of this it can be detrimental to preload with a fallback src since Safari will end up downloading the wrong image 100% of the time.

The solution to this is to allow the user to omit the href if they provide imagesrcset (and optionally) imagesizes. Effectively the key for image preloads will become the union of src (href), imagesrcset, and imagesizes.
  • Loading branch information
@gnoff
gnoff committed Jun 13, 2023
1 parent 8ed2797 commit 8d8bcde
Show file tree
Hide file tree
Showing 5 changed files with 311 additions and 85 deletions.
92 changes: 86 additions & 6 deletions packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js
View file
Expand Up @@ -105,7 +105,6 @@ import {
} from 'react-reconciler/src/ReactWorkTags';
import {listenToAllSupportedEvents} from '../events/DOMPluginEventSystem';
import {
validatePreloadArguments,
validatePreinitArguments,
validateLinkPropsForStyleResource,
getValueDescriptorExpectingObjectForWarning,
Expand Down Expand Up @@ -2167,21 +2166,102 @@ 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, ...)if (__DEV__) {
switch (undefined) {
default: {
let encountered = '';
if (typeof href !== 'string') {
encountered += `The \`href\` argument encountered was ${getValueDescriptorExpectingObjectForWarning(
href,
)}.`;
}
if (options == null || typeof options !== 'object') {
encountered += `The \`options\` argument encountered was ${getValueDescriptorExpectingObjectForWarning(
options,
)}.`;
if (typeof href === 'string') {
encountered += ` Try fixing the options argument for the call with \`href\` "${href}".`;
}
} 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, an `href` string and an `options` object with an `as` property valid for a `<link rel="preload" as="..." />` tag. %s',
encountered,
);
break;
}

const as = options.as;
if (as === 'image') {
if (
!href &&
(!options.imageSrcSet || typeof options.imageSrcSet !== 'string')
) {
console.error(
'ReactDOM.preload(): When preloading as an "image" expected either the `href` argument to be a non-empty string or the `imageSrcSet` option to be a non-empty string or both. The `href` encountered was %s and the `imageSrcSet` encountered was %s.',
getValueDescriptorExpectingObjectForWarning(href),
getValueDescriptorExpectingObjectForWarning(options.imageSrcSet),
);
}
} else {
if (!href) {
console.error(
'ReactDOM.preload(): When preloading as %s expected the `href` argument to be a non-empty string. The `href` encountered was %s.',
getValueDescriptorExpectingObjectForWarning(options.as),
getValueDescriptorExpectingObjectForWarning(href),
);
}
}
}
}
}
const ownerDocument = getDocumentForImperativeFloatMethods();
if (
typeof href === 'string' &&
href &&
typeof options === 'object' &&
options !== null &&
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,
)}"]`;
} else if (!href) {
// This call has neither a imageSrcSet nor an href so we can't preload anything.
// In dev we should have already warned above. We noop now.
return;
} else {
preloadSelector += `[href="${escapeSelectorAttributeValueInsideDoubleQuotes(
href,
)}"]`;
}

if (typeof imageSizes === 'string' && imageSizes !== '') {
preloadSelector += `[imagesizes="${escapeSelectorAttributeValueInsideDoubleQuotes(
imageSizes,
)}"]`;
}
} else {
if (!href) {
// This call has no href and its type does not specify an alternate preloadabl
// resources so we noop
return;
}
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.
Expand Down
113 changes: 92 additions & 21 deletions packages/react-dom-bindings/src/server/ReactFizzConfigDOM.js
View file
Expand Up @@ -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;
Expand Down Expand Up @@ -5063,32 +5063,97 @@ export function preload(href: string, options: PreloadOptions) {
}
const resources = getResources(request);
if (__DEV__) {
if (typeof href !== 'string' || !href) {
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 `<link rel="preload" as="..." />` tag are valid here.',
getValueDescriptorExpectingEnumForWarning(options.as),
);
switch (undefined) {
default: {
let encountered = '';
if (typeof href !== 'string') {
encountered += `The \`href\` argument encountered was ${getValueDescriptorExpectingObjectForWarning(
href,
)}.`;
}
if (options == null || typeof options !== 'object') {
encountered += `The \`options\` argument encountered was ${getValueDescriptorExpectingObjectForWarning(
options,
)}.`;
if (typeof href === 'string') {
encountered += ` Try fixing the options argument for the call with \`href\` "${href}".`;
}
} 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, an `href` string and an `options` object with an `as` property valid for a `<link rel="preload" as="..." />` tag. %s',
encountered,
);
break;
}

const as = options.as;
if (as === 'image') {
if (
!href &&
(!options.imageSrcSet || typeof options.imageSrcSet !== 'string')
) {
console.error(
'ReactDOM.preload(): When preloading as an "image" expected either the `href` argument to be a non-empty string or the `imageSrcSet` option to be a non-empty string or both. The `href` encountered was %s and the `imageSrcSet` encountered was %s.',
getValueDescriptorExpectingObjectForWarning(href),
getValueDescriptorExpectingObjectForWarning(options.imageSrcSet),
);
}
} else {
if (!href) {
console.error(
'ReactDOM.preload(): When preloading as %s expected the `href` argument to be a non-empty string. The `href` encountered was %s.',
getValueDescriptorExpectingObjectForWarning(options.as),
getValueDescriptorExpectingObjectForWarning(href),
);
}
}
}
}
}
if (
typeof href === 'string' &&
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') {
const {imageSrcSet, imageSizes} = options;
let uniquePart = '';
if (typeof imageSrcSet === 'string' && imageSrcSet !== '') {
uniquePart += '[' + imageSrcSet + ']';
} else if (!href) {
// This call has neither a imageSrcSet nor an href so we can't preload anything.
// In dev we should have already warned above. We noop now.
return;
} else {
uniquePart += '[]';
}

if (typeof imageSizes === 'string' && imageSizes !== '') {
uniquePart += '[' + imageSizes + ']';
} else {
uniquePart += '[]';
}

uniquePart += href;

key = getResourceKey(as, uniquePart);
} else {
if (!href) {
// This call has no href and its type does not specify an alternate preloadabl
// resources so we noop
return;
}
key = getResourceKey(as, href);
}
let resource = resources.preloadsMap.get(key);
if (__DEV__) {
const devResource = getAsResourceDEV(resource);
Expand Down Expand Up @@ -5528,12 +5593,18 @@ function preloadPropsFromPreloadOptions(
return {
rel: 'preload',
as,
href,
// When preloading with imageSrcSet and imageSizes sometimes it
// can make sense to not have a default href. We validate that
// you are omitting it elsewhere but here we just mark it undefined
// if it is falsey so that when it renders it omits the href
href: href ? href : undefined,
crossOrigin: as === 'font' ? '' : options.crossOrigin,
integrity: options.integrity,
type: options.type,
nonce: options.nonce,
fetchPriority: options.fetchPriority,
imageSrcSet: options.imageSrcSet,
imageSizes: options.imageSizes,
};
}

Expand Down
54 changes: 0 additions & 54 deletions packages/react-dom-bindings/src/shared/ReactDOMResourceValidation.js
View file
Expand Up @@ -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') {
Expand Down

0 comments on commit 8d8bcde

Please sign in to comment.