Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Fiber] Ensure srcset and src are assigned last on img instances #30340

Merged
merged 2 commits into from
Jul 16, 2024
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Safari truly is the new IE.

// 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
gnoff marked this conversation as resolved.
Show resolved Hide resolved
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 (hasSrcSet) {
setProp(domElement, tag, 'srcSet', props.srcSet, props, null);
}
if (hasSrc) {
setProp(domElement, tag, 'src', props.src, 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