Skip to content

Commit 01ef646

Browse files
committed
[Fiber] Ensure srcset is last on img instances
Safari has a behavior (bug) where when you consturct an Image in javascript if you set srcset before properties for `sizes` the brwoser will download the largest image size because it starts to load before you communicate the sizes information. https://x.com/OliverJAsh/status/1812408504444989588?t=CVHPqBaUiF5-6DBPGERTDA This change updates the dom fiber implementation to ensure that sizes is added before srcset
1 parent 8b08e99 commit 01ef646

File tree

1 file changed

+47
-1
lines changed

1 file changed

+47
-1
lines changed

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

Lines changed: 47 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1031,6 +1031,53 @@ export function setInitialProperties(
10311031
// Fast track the most common tag types
10321032
break;
10331033
}
1034+
// img tags previously were implemented as void elements with non delegated events however Safari (and possibly Firefox)
1035+
// begin fetching the image as soon as the `src` or `srcSet` property is set and if we set these before other properties
1036+
// that can modify the request (such as crossorigin) or the resource fetch (such as sizes) then the browser will load
1037+
// the wrong thing or load more than one thing. This implementation ensures src and srcSet are set on the instance last
1038+
case 'img': {
1039+
listenToNonDelegatedEvent('error', domElement);
1040+
listenToNonDelegatedEvent('load', domElement);
1041+
// Mostlye a port of Void Element logic with special casing to ensure srcset and src are set last
1042+
let hasSrc = false;
1043+
let hasSrcSet = false;
1044+
for (const propKey in props) {
1045+
if (!props.hasOwnProperty(propKey)) {
1046+
continue;
1047+
}
1048+
const propValue = props[propKey];
1049+
if (propValue == null) {
1050+
continue;
1051+
}
1052+
switch (propKey) {
1053+
case 'src':
1054+
hasSrc = true;
1055+
break;
1056+
case 'srcSet':
1057+
hasSrcSet = true;
1058+
break;
1059+
case 'children':
1060+
case 'dangerouslySetInnerHTML': {
1061+
// TODO: Can we make this a DEV warning to avoid this deny list?
1062+
throw new Error(
1063+
`${tag} is a void element tag and must neither have \`children\` nor ` +
1064+
'use `dangerouslySetInnerHTML`.',
1065+
);
1066+
}
1067+
// defaultChecked and defaultValue are ignored by setProp
1068+
default: {
1069+
setProp(domElement, tag, propKey, propValue, props, null);
1070+
}
1071+
}
1072+
}
1073+
if (hasSrcSet) {
1074+
setProp(domElement, tag, 'srcSet', props.srcSet, props, null);
1075+
}
1076+
if (hasSrc) {
1077+
setProp(domElement, tag, 'src', props.src, props, null);
1078+
}
1079+
return;
1080+
}
10341081
case 'input': {
10351082
if (__DEV__) {
10361083
checkControlledValueProps('input', props);
@@ -1269,7 +1316,6 @@ export function setInitialProperties(
12691316
}
12701317
case 'embed':
12711318
case 'source':
1272-
case 'img':
12731319
case 'link': {
12741320
// These are void elements that also need delegated events.
12751321
listenToNonDelegatedEvent('error', domElement);

0 commit comments

Comments
 (0)