Skip to content

Commit

Permalink
[Fiber] Ensure srcset is last on img instances
Browse files Browse the repository at this point in the history
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
  • Loading branch information
gnoff committed Jul 15, 2024
1 parent 8b08e99 commit 512ff02
Showing 1 changed file with 47 additions and 1 deletion.
48 changes: 47 additions & 1 deletion packages/react-dom-bindings/src/client/ReactDOMComponent.js
Original file line number Diff line number Diff line change
Expand Up @@ -1031,6 +1031,53 @@ export function setInitialProperties(
// Fast track the most common tag types
break;
}
// img tags previously were implemented as void elements with non delegated events however Safari (and possibly Firefox)
// begin fetching the image as soon as the `src` or `srcSet` property is set and if we set these before other properties
// that can modify the request (such as crossorigin) or the resource fetch (such as sizes) then the browser will load
// the wrong thing or load more than one thing. This implementation ensures src and srcSet are set on the instance last
case 'img': {
listenToNonDelegatedEvent('error', domElement);
listenToNonDelegatedEvent('load', domElement);
// Mostlye a port of Void Element logic with special casing to ensure srcset and src are set last
let hasSrc = false;
let hasSrcSet = false;
for (const propKey in props) {
if (!props.hasOwnProperty(propKey)) {
continue;
}
const propValue = props[propKey];
if (propValue == null) {
continue;
}
switch (propKey) {
case 'src':
hasSrc = true;
break;
case 'srcSet':
hasSrcSet = true;
break;
case 'children':
case 'dangerouslySetInnerHTML': {
// TODO: Can we make this a DEV warning to avoid this deny list?
throw new Error(
`${tag} is a void element tag and must neither have \`children\` nor ` +
'use `dangerouslySetInnerHTML`.',
);
}
// defaultChecked and defaultValue are ignored by setProp
default: {
setProp(domElement, tag, propKey, propValue, props, null);
}
}
}
if (hasSrc) {
setProp(domElement, tag, 'src', props.src, props, null);
}
if (hasSrcSet) {
setProp(domElement, tag, 'srcSet', props.srcSet, props, null);
}
return;
}
case 'input': {
if (__DEV__) {
checkControlledValueProps('input', props);
Expand Down Expand Up @@ -1269,7 +1316,6 @@ export function setInitialProperties(
}
case 'embed':
case 'source':
case 'img':
case 'link': {
// These are void elements that also need delegated events.
listenToNonDelegatedEvent('error', domElement);
Expand Down

0 comments on commit 512ff02

Please sign in to comment.