reassign src from props not the dom element src property (#23395)
The src property on the dom element will return a fully qualified name and this does not match the dom src attribute or the props provided to react. instead of reading from the element and re-assigning the property we assign the property from props which is how it was initially assigned during the render Co-authored-by: Josh Story <story@hey.com>
Josh Story committed
Mar 1, 2022 at 15:23 UTC
f0dd459e0d97081cb3c313ec52285e3e422f8dbf
2 files changed
+56
-1
packages/react-dom/src/__tests__/ReactDOMImageLoad-test.internal.js
+55
@@ -18,6 +18,7 @@ let ReactDOMClient;
18
let originalCreateElement;
19
// let TextResource;
20
// let textResourceShouldFail;
21
+let originalHTMLImageElementSrcDescriptor;
22
23
let images = [];
24
let onLoadSpy = null;
@@ -165,6 +166,11 @@ describe('ReactDOMImageLoad', () => {
166
return element;
167
};
168
169
+ originalHTMLImageElementSrcDescriptor = Object.getOwnPropertyDescriptor(
170
+ HTMLImageElement.prototype,
171
+ 'src',
172
+ );
173
+
174
Object.defineProperty(HTMLImageElement.prototype, 'src', {
175
get() {
176
return this.getAttribute('src');
@@ -179,6 +185,11 @@ describe('ReactDOMImageLoad', () => {
185
186
afterEach(() => {
187
document.createElement = originalCreateElement;
188
+ Object.defineProperty(
189
+ HTMLImageElement.prototype,
190
+ 'src',
191
+ originalHTMLImageElementSrcDescriptor,
192
+ );
193
});
194
195
it('captures the load event if it happens before commit phase and replays it between layout and passive effects', async function() {
@@ -575,4 +586,48 @@ describe('ReactDOMImageLoad', () => {
586
'Committed',
587
]);
588
});
589
+
590
+ it('preserves the src property / attribute when triggering a potential new load event', () => {
591
+ // this test covers a regression identified in https://github.com/mui/material-ui/pull/31263
592
+ // where the resetting of the src property caused the property to change from relative to fully qualified
593
+
594
+ // make sure we are not using the patched src setter
595
+ Object.defineProperty(
596
+ HTMLImageElement.prototype,
597
+ 'src',
598
+ originalHTMLImageElementSrcDescriptor,
599
+ );
600
+
601
+ const container = document.createElement('div');
602
+ const root = ReactDOMClient.createRoot(container);
603
+
604
+ React.startTransition(() =>
605
+ root.render(
606
+ <PhaseMarkers>
607
+ <Img onLoad={onLoadSpy} />
608
+ <Yield />
609
+ <Text text={'a'} />
610
+ </PhaseMarkers>,
611
+ ),
612
+ );
613
+
614
+ // render to yield to capture state of img src attribute and property before commit
615
+ expect(Scheduler).toFlushAndYieldThrough([
616
+ 'render start',
617
+ 'Img default',
618
+ 'Yield',
619
+ ]);
620
+ const img = last(images);
621
+ const renderSrcProperty = img.src;
622
+ const renderSrcAttr = img.getAttribute('src');
623
+
624
+ // finish render and commit causing the src property to be rewritten
625
+ expect(Scheduler).toFlushAndYield(['a', 'last layout', 'last passive']);
626
+ const commitSrcProperty = img.src;
627
+ const commitSrcAttr = img.getAttribute('src');
628
+
629
+ // ensure attribute and properties agree
630
+ expect(renderSrcProperty).toBe(commitSrcProperty);
631
+ expect(renderSrcAttr).toBe(commitSrcAttr);
632
+ });
633
});
packages/react-dom/src/client/ReactDOMHostConfig.js
+1
-1
@@ -442,7 +442,7 @@ export function commitMount(
442
return;
443
case 'img': {
444
if ((newProps: any).src) {
445
- ((domElement: any): HTMLImageElement).src = ((domElement: any): HTMLImageElement).src;
445
+ ((domElement: any): HTMLImageElement).src = (newProps: any).src;
446
}
447
return;
448
}