@samitouri / QOS-React-2 / commits / 0117239720

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

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 There are likely other combinations or property order assignment that can cause problems such as setting crossorigin after assigning src or srcset. Conceptually we should withold the src and srcSet from the Image instance until last so all relevant other properties can be assigned before actually initiating any network activity. This is an unforunate amount of code for what is realistically a bug in Browsers but it should allow us to avoid weird regressions depending on prop object order. I didn't change the preload prop order because I don't believe preload links have the same issue (they are not fetched as eagerly I believe). One nice benefit of this change though is the img case can move higher in the switch which is likely optimal given it's a relatively common tag. Previously it was as low as it was because it was part of the void element set so it couldn't be elevated without elevating less common tags --------- Co-authored-by: Jan Kassens <jan@kassens.net>

Josh Story committed Jul 16, 2024 at 08:25 UTC 0117239720b6ea830376f4f8605957ccae8b3735
1 file changed +47 -1
packages/react-dom-bindings/src/client/ReactDOMComponent.js
+47 -1
@@ -1031,6 +1031,53 @@ export function setInitialProperties(
1031 // Fast track the most common tag types
1032 break;
1033 }
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 + // Mostly 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 + }
1081 case 'input': {
1082 if (__DEV__) {
1083 checkControlledValueProps('input', props);
@@ -1269,7 +1316,6 @@ export function setInitialProperties(
1316 }
1317 case 'embed':
1318 case 'source':
1272 - case 'img':
1319 case 'link': {
1320 // These are void elements that also need delegated events.
1321 listenToNonDelegatedEvent('error', domElement);