Skip to content

Commit f0e6dbd

Browse files
committed
extends image preloading to accept imagesrcset and imagesizes to handle 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.
1 parent 86acc10 commit f0e6dbd

File tree

7 files changed

+264
-86
lines changed

7 files changed

+264
-86
lines changed

packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js

Lines changed: 62 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,6 @@ import {
105105
} from 'react-reconciler/src/ReactWorkTags';
106106
import {listenToAllSupportedEvents} from '../events/DOMPluginEventSystem';
107107
import {
108-
validatePreloadArguments,
109108
validatePreinitArguments,
110109
validateLinkPropsForStyleResource,
111110
getValueDescriptorExpectingObjectForWarning,
@@ -2016,7 +2015,7 @@ type ScriptProps = {
20162015

20172016
type PreloadProps = {
20182017
rel: 'preload',
2019-
href: string,
2018+
href: ?string,
20202019
[string]: mixed,
20212020
};
20222021

@@ -2167,21 +2166,70 @@ function preload(href: string, options: PreloadOptions) {
21672166
return;
21682167
}
21692168
if (__DEV__) {
2170-
validatePreloadArguments(href, options);
2169+
// TODO move this to ReactDOMFloat and expose a stricter function interface or possibly
2170+
// typed functions (preloadImage, preloadStyle, ...)
2171+
let encountered = '';
2172+
if (typeof href !== 'string' || !href) {
2173+
encountered += `The \`href\` argument encountered was ${getValueDescriptorExpectingObjectForWarning(
2174+
href,
2175+
)}.`;
2176+
}
2177+
if (options == null || typeof options !== 'object') {
2178+
encountered += `The \`options\` argument encountered was ${getValueDescriptorExpectingObjectForWarning(
2179+
options,
2180+
)}.`;
2181+
} else if (typeof options.as !== 'string' || !options.as) {
2182+
encountered += `The \`as\` option encountered was ${getValueDescriptorExpectingObjectForWarning(
2183+
options.as,
2184+
)}.`;
2185+
}
2186+
if (encountered) {
2187+
console.error(
2188+
'ReactDOM.preload(): Expected two arguments, a non-empty `href` string and an `options` object with an `as` property valid for a `<link rel="preload" as="..." />` tag. %s',
2189+
encountered,
2190+
);
2191+
}
21712192
}
21722193
const ownerDocument = getDocumentForImperativeFloatMethods();
21732194
if (
21742195
typeof href === 'string' &&
21752196
href &&
21762197
typeof options === 'object' &&
21772198
options !== null &&
2199+
typeof options.as === 'string' &&
2200+
options.as &&
21782201
ownerDocument
21792202
) {
21802203
const as = options.as;
2181-
const limitedEscapedHref =
2182-
escapeSelectorAttributeValueInsideDoubleQuotes(href);
2183-
const preloadSelector = `link[rel="preload"][as="${as}"][href="${limitedEscapedHref}"]`;
2184-
2204+
let preloadSelector = `link[rel="preload"][as="${escapeSelectorAttributeValueInsideDoubleQuotes(
2205+
as,
2206+
)}"]`;
2207+
if (as === 'image') {
2208+
const {imageSrcSet, imageSizes} = options;
2209+
if (typeof imageSrcSet === 'string' && imageSrcSet !== '') {
2210+
preloadSelector += `[imagesrcset="${escapeSelectorAttributeValueInsideDoubleQuotes(
2211+
imageSrcSet,
2212+
)}"]`;
2213+
if (typeof imageSizes === 'string') {
2214+
preloadSelector += `[imagesizes="${escapeSelectorAttributeValueInsideDoubleQuotes(
2215+
imageSizes,
2216+
)}"]`;
2217+
}
2218+
} else {
2219+
preloadSelector += `[href="${escapeSelectorAttributeValueInsideDoubleQuotes(
2220+
href,
2221+
)}"]`;
2222+
}
2223+
} else {
2224+
if (!href) {
2225+
// This call has no href and its type does not specify an alternate preloadabl
2226+
// resources so we noop
2227+
return;
2228+
}
2229+
preloadSelector += `[href="${escapeSelectorAttributeValueInsideDoubleQuotes(
2230+
href,
2231+
)}"]`;
2232+
}
21852233
// Some preloads are keyed under their selector. This happens when the preload is for
21862234
// an arbitrary type. Other preloads are keyed under the resource key they represent a preload for.
21872235
// Here we figure out which key to use to determine if we have a preload already.
@@ -2227,14 +2275,20 @@ function preloadPropsFromPreloadOptions(
22272275
options: PreloadOptions,
22282276
): PreloadProps {
22292277
return {
2230-
href,
22312278
rel: 'preload',
22322279
as,
2280+
// There is a bug in Safari where imageSrcSet is not respected on preload links
2281+
// so we omit the href here if we have imageSrcSet b/c safari will load the wrong image.
2282+
// This harms older browers that do not support imageSrcSet by making their preloads not work
2283+
// but this population is shrinking fast and is already small so we accept this tradeoff.
2284+
href: as === 'image' && options.imageSrcSet ? undefined : href,
22332285
crossOrigin: as === 'font' ? '' : options.crossOrigin,
22342286
integrity: options.integrity,
22352287
type: options.type,
22362288
nonce: options.nonce,
22372289
fetchPriority: options.fetchPriority,
2290+
imageSrcSet: options.imageSrcSet,
2291+
imageSizes: options.imageSizes,
22382292
};
22392293
}
22402294

packages/react-dom-bindings/src/server/ReactFizzConfigDOM.js

Lines changed: 52 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -4819,12 +4819,12 @@ type PreconnectResource = TResource<'preconnect', null>;
48194819
type PreloadAsProps = {
48204820
rel: 'preload',
48214821
as: string,
4822-
href: string,
4822+
href: ?string,
48234823
[string]: mixed,
48244824
};
48254825
type PreloadModuleProps = {
48264826
rel: 'modulepreload',
4827-
href: string,
4827+
href: ?string,
48284828
[string]: mixed,
48294829
};
48304830
type PreloadProps = PreloadAsProps | PreloadModuleProps;
@@ -5063,20 +5063,25 @@ export function preload(href: string, options: PreloadOptions) {
50635063
}
50645064
const resources = getResources(request);
50655065
if (__DEV__) {
5066+
let encountered = '';
50665067
if (typeof href !== 'string' || !href) {
5068+
encountered += ` The \`href\` argument encountered was ${getValueDescriptorExpectingObjectForWarning(
5069+
href,
5070+
)}.`;
5071+
}
5072+
if (options == null || typeof options !== 'object') {
5073+
encountered += ` The \`options\` argument encountered was ${getValueDescriptorExpectingObjectForWarning(
5074+
options,
5075+
)}.`;
5076+
} else if (typeof options.as !== 'string' || !options.as) {
5077+
encountered += ` The \`as\` option encountered was ${getValueDescriptorExpectingObjectForWarning(
5078+
options.as,
5079+
)}.`;
5080+
}
5081+
if (encountered) {
50675082
console.error(
5068-
'ReactDOM.preload(): Expected the `href` argument (first) to be a non-empty string but encountered %s instead.',
5069-
getValueDescriptorExpectingObjectForWarning(href),
5070-
);
5071-
} else if (options == null || typeof options !== 'object') {
5072-
console.error(
5073-
'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.',
5074-
getValueDescriptorExpectingEnumForWarning(options),
5075-
);
5076-
} else if (typeof options.as !== 'string') {
5077-
console.error(
5078-
'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.',
5079-
getValueDescriptorExpectingEnumForWarning(options.as),
5083+
'ReactDOM.preload(): Expected two arguments, a non-empty `href` string and an `options` object with an `as` property valid for a `<link rel="preload" as="..." />` tag.%s',
5084+
encountered,
50805085
);
50815086
}
50825087
}
@@ -5085,10 +5090,34 @@ export function preload(href: string, options: PreloadOptions) {
50855090
href &&
50865091
typeof options === 'object' &&
50875092
options !== null &&
5088-
typeof options.as === 'string'
5093+
typeof options.as === 'string' &&
5094+
options.as
50895095
) {
50905096
const as = options.as;
5091-
const key = getResourceKey(as, href);
5097+
let key: string;
5098+
if (as === 'image') {
5099+
// For image preloads the key contains either the imageSrcSet + imageSizes or the href but not
5100+
// both. This is to prevent identical calls with the same srcSet and sizes to be duplicated
5101+
// by varying the href. this is an edge case but it is the most correct behavior.
5102+
const {imageSrcSet, imageSizes} = options;
5103+
let uniquePart = '';
5104+
if (typeof imageSrcSet === 'string' && imageSrcSet !== '') {
5105+
uniquePart += '[' + imageSrcSet + ']';
5106+
if (typeof imageSizes === 'string') {
5107+
uniquePart += '[' + imageSizes + ']';
5108+
}
5109+
} else {
5110+
uniquePart += '[][]' + href;
5111+
}
5112+
key = getResourceKey(as, uniquePart);
5113+
} else {
5114+
if (!href) {
5115+
// This call has no href and its type does not specify an alternate preloadabl
5116+
// resources so we noop
5117+
return;
5118+
}
5119+
key = getResourceKey(as, href);
5120+
}
50925121
let resource = resources.preloadsMap.get(key);
50935122
if (__DEV__) {
50945123
const devResource = getAsResourceDEV(resource);
@@ -5528,12 +5557,18 @@ function preloadPropsFromPreloadOptions(
55285557
return {
55295558
rel: 'preload',
55305559
as,
5531-
href,
5560+
// There is a bug in Safari where imageSrcSet is not respected on preload links
5561+
// so we omit the href here if we have imageSrcSet b/c safari will load the wrong image.
5562+
// This harms older browers that do not support imageSrcSet by making their preloads not work
5563+
// but this population is shrinking fast and is already small so we accept this tradeoff.
5564+
href: as === 'image' && options.imageSrcSet ? undefined : href,
55325565
crossOrigin: as === 'font' ? '' : options.crossOrigin,
55335566
integrity: options.integrity,
55345567
type: options.type,
55355568
nonce: options.nonce,
55365569
fetchPriority: options.fetchPriority,
5570+
imageSrcSet: options.imageSrcSet,
5571+
imageSizes: options.imageSizes,
55375572
};
55385573
}
55395574

packages/react-dom-bindings/src/shared/ReactDOMResourceValidation.js

Lines changed: 0 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -62,60 +62,6 @@ function propNamesListJoin(
6262
}
6363
}
6464

65-
export function validatePreloadArguments(href: mixed, options: mixed) {
66-
if (__DEV__) {
67-
if (!href || typeof href !== 'string') {
68-
const typeOfArg = getValueDescriptorExpectingObjectForWarning(href);
69-
console.error(
70-
'ReactDOM.preload() expected the first argument to be a string representing an href but found %s instead.',
71-
typeOfArg,
72-
);
73-
} else if (typeof options !== 'object' || options === null) {
74-
const typeOfArg = getValueDescriptorExpectingObjectForWarning(options);
75-
console.error(
76-
'ReactDOM.preload() expected the second argument to be an options argument containing at least an "as" property' +
77-
' specifying the Resource type. It found %s instead. The href for the preload call where this warning originated is "%s".',
78-
typeOfArg,
79-
href,
80-
);
81-
} else {
82-
const as = options.as;
83-
switch (as) {
84-
// Font specific validation of options
85-
case 'font': {
86-
if (options.crossOrigin === 'use-credentials') {
87-
console.error(
88-
'ReactDOM.preload() was called with an "as" type of "font" and with a "crossOrigin" option of "use-credentials".' +
89-
' Fonts preloading must use crossOrigin "anonymous" to be functional. Please update your font preload to omit' +
90-
' the crossOrigin option or change it to any other value than "use-credentials" (Browsers default all other values' +
91-
' to anonymous mode). The href for the preload call where this warning originated is "%s"',
92-
href,
93-
);
94-
}
95-
break;
96-
}
97-
case 'script':
98-
case 'style': {
99-
break;
100-
}
101-
102-
// We have an invalid as type and need to warn
103-
default: {
104-
const typeOfAs = getValueDescriptorExpectingEnumForWarning(as);
105-
console.error(
106-
'ReactDOM.preload() expected a valid "as" type in the options (second) argument but found %s instead.' +
107-
' Please use one of the following valid values instead: %s. The href for the preload call where this' +
108-
' warning originated is "%s".',
109-
typeOfAs,
110-
'"style", "font", or "script"',
111-
href,
112-
);
113-
}
114-
}
115-
}
116-
}
117-
}
118-
11965
export function validatePreinitArguments(href: mixed, options: mixed) {
12066
if (__DEV__) {
12167
if (!href || typeof href !== 'string') {

0 commit comments

Comments
 (0)