From 086fa8ee2f80f0dc34b7d145be72f9843fca975d Mon Sep 17 00:00:00 2001 From: Josh Story Date: Mon, 28 Feb 2022 15:07:39 -0800 Subject: [PATCH] re-trigger load events for img elements on commit (#23316) early load events will be missed by onLoad handlers if they trigger before the tree is committed to avoid this we reset the src property on the img element to cause the browser to re-load the img. Co-authored-by: Josh Story --- .../ReactDOMImageLoad-test.internal.js | 576 ++++++++++++++++++ .../src/client/ReactDOMHostConfig.js | 48 +- 2 files changed, 606 insertions(+), 18 deletions(-) create mode 100644 packages/react-dom/src/__tests__/ReactDOMImageLoad-test.internal.js diff --git a/packages/react-dom/src/__tests__/ReactDOMImageLoad-test.internal.js b/packages/react-dom/src/__tests__/ReactDOMImageLoad-test.internal.js new file mode 100644 index 0000000000000..bc7316349196a --- /dev/null +++ b/packages/react-dom/src/__tests__/ReactDOMImageLoad-test.internal.js @@ -0,0 +1,576 @@ +/** + * Copyright (c) Facebook, Inc. and its affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + * @emails react-core + */ + +'use strict'; + +let React; +let Scheduler; +// let ReactCache; +let ReactDOM; +// let Suspense; +let originalCreateElement; +// let TextResource; +// let textResourceShouldFail; + +let images = []; +let onLoadSpy = null; +let actualLoadSpy = null; + +function PhaseMarkers({children}) { + Scheduler.unstable_yieldValue('render start'); + React.useLayoutEffect(() => { + Scheduler.unstable_yieldValue('last layout'); + }); + React.useEffect(() => { + Scheduler.unstable_yieldValue('last passive'); + }); + return children; +} + +function last(arr) { + if (Array.isArray(arr)) { + if (arr.length) { + return arr[arr.length - 1]; + } + return undefined; + } + throw new Error('last was passed something that was not an array'); +} + +function Text(props) { + Scheduler.unstable_yieldValue(props.text); + return props.text; +} + +// function AsyncText(props) { +// const text = props.text; +// try { +// TextResource.read([props.text, props.ms]); +// Scheduler.unstable_yieldValue(text); +// return text; +// } catch (promise) { +// if (typeof promise.then === 'function') { +// Scheduler.unstable_yieldValue(`Suspend! [${text}]`); +// } else { +// Scheduler.unstable_yieldValue(`Error! [${text}]`); +// } +// throw promise; +// } +// } + +function Img({src: maybeSrc, onLoad, useImageLoader, ref}) { + const src = maybeSrc || 'default'; + Scheduler.unstable_yieldValue('Img ' + src); + return ; +} + +function Yield() { + Scheduler.unstable_yieldValue('Yield'); + Scheduler.unstable_requestPaint(); + return null; +} + +function loadImage(element) { + const event = new Event('load'); + element.__needsDispatch = false; + element.dispatchEvent(event); +} + +describe('ReactDOMImageLoad', () => { + beforeEach(() => { + jest.resetModules(); + React = require('react'); + Scheduler = require('scheduler'); + // ReactCache = require('react-cache'); + ReactDOM = require('react-dom'); + // Suspense = React.Suspense; + + onLoadSpy = jest.fn(reactEvent => { + const src = reactEvent.target.getAttribute('src'); + Scheduler.unstable_yieldValue('onLoadSpy [' + src + ']'); + }); + + actualLoadSpy = jest.fn(nativeEvent => { + const src = nativeEvent.target.getAttribute('src'); + Scheduler.unstable_yieldValue('actualLoadSpy [' + src + ']'); + nativeEvent.__originalDispatch = false; + }); + + // TextResource = ReactCache.unstable_createResource( + // ([text, ms = 0]) => { + // let listeners = null; + // let status = 'pending'; + // let value = null; + // return { + // then(resolve, reject) { + // switch (status) { + // case 'pending': { + // if (listeners === null) { + // listeners = [{resolve, reject}]; + // setTimeout(() => { + // if (textResourceShouldFail) { + // Scheduler.unstable_yieldValue( + // `Promise rejected [${text}]`, + // ); + // status = 'rejected'; + // value = new Error('Failed to load: ' + text); + // listeners.forEach(listener => listener.reject(value)); + // } else { + // Scheduler.unstable_yieldValue( + // `Promise resolved [${text}]`, + // ); + // status = 'resolved'; + // value = text; + // listeners.forEach(listener => listener.resolve(value)); + // } + // }, ms); + // } else { + // listeners.push({resolve, reject}); + // } + // break; + // } + // case 'resolved': { + // resolve(value); + // break; + // } + // case 'rejected': { + // reject(value); + // break; + // } + // } + // }, + // }; + // }, + // ([text, ms]) => text, + // ); + // textResourceShouldFail = false; + + images = []; + + originalCreateElement = document.createElement; + document.createElement = function createElement(tagName, options) { + const element = originalCreateElement.call(document, tagName, options); + if (tagName === 'img') { + element.addEventListener('load', actualLoadSpy); + images.push(element); + } + return element; + }; + + Object.defineProperty(HTMLImageElement.prototype, 'src', { + get() { + return this.getAttribute('src'); + }, + set(value) { + Scheduler.unstable_yieldValue('load triggered'); + this.__needsDispatch = true; + this.setAttribute('src', value); + }, + }); + }); + + afterEach(() => { + document.createElement = originalCreateElement; + }); + + it('captures the load event if it happens before commit phase and replays it between layout and passive effects', async function() { + const container = document.createElement('div'); + const root = ReactDOM.createRoot(container); + + React.startTransition(() => + root.render( + + + + + , + ), + ); + + expect(Scheduler).toFlushAndYieldThrough([ + 'render start', + 'Img default', + 'Yield', + ]); + const img = last(images); + loadImage(img); + expect(Scheduler).toHaveYielded([ + 'actualLoadSpy [default]', + // no onLoadSpy since we have not completed render + ]); + expect(Scheduler).toFlushAndYield([ + 'a', + 'load triggered', + 'last layout', + 'last passive', + ]); + expect(img.__needsDispatch).toBe(true); + loadImage(img); + expect(Scheduler).toHaveYielded([ + 'actualLoadSpy [default]', // the browser reloading of the image causes this to yield again + 'onLoadSpy [default]', + ]); + expect(onLoadSpy).toHaveBeenCalled(); + }); + + it('captures the load event if it happens after commit phase and replays it', async function() { + const container = document.createElement('div'); + const root = ReactDOM.createRoot(container); + + React.startTransition(() => + root.render( + + + , + ), + ); + + expect(Scheduler).toFlushAndYieldThrough([ + 'render start', + 'Img default', + 'load triggered', + 'last layout', + ]); + Scheduler.unstable_requestPaint(); + const img = last(images); + loadImage(img); + expect(Scheduler).toHaveYielded([ + 'actualLoadSpy [default]', + 'onLoadSpy [default]', + ]); + expect(Scheduler).toFlushAndYield(['last passive']); + expect(img.__needsDispatch).toBe(false); + expect(onLoadSpy).toHaveBeenCalledTimes(1); + }); + + it('it replays the last load event when more than one fire before the end of the layout phase completes', async function() { + const container = document.createElement('div'); + const root = ReactDOM.createRoot(container); + + function Base() { + const [src, setSrc] = React.useState('a'); + return ( + + + + + + ); + } + + function UpdateSrc({setSrc}) { + React.useLayoutEffect(() => { + setSrc('b'); + }, [setSrc]); + return null; + } + + React.startTransition(() => root.render()); + + expect(Scheduler).toFlushAndYieldThrough([ + 'render start', + 'Img a', + 'Yield', + ]); + const img = last(images); + loadImage(img); + expect(Scheduler).toHaveYielded(['actualLoadSpy [a]']); + + expect(Scheduler).toFlushAndYieldThrough([ + 'load triggered', + 'last layout', + // the update in layout causes a passive effects flush before a sync render + 'last passive', + 'render start', + 'Img b', + 'Yield', + // yield is ignored becasue we are sync rendering + 'last layout', + 'last passive', + ]); + expect(images.length).toBe(1); + loadImage(img); + expect(Scheduler).toHaveYielded(['actualLoadSpy [b]', 'onLoadSpy [b]']); + expect(onLoadSpy).toHaveBeenCalledTimes(1); + }); + + it('replays load events that happen in passive phase after the passive phase.', async function() { + const container = document.createElement('div'); + const root = ReactDOM.createRoot(container); + + root.render( + + + , + ); + + expect(Scheduler).toFlushAndYield([ + 'render start', + 'Img default', + 'load triggered', + 'last layout', + 'last passive', + ]); + const img = last(images); + loadImage(img); + expect(Scheduler).toHaveYielded([ + 'actualLoadSpy [default]', + 'onLoadSpy [default]', + ]); + expect(onLoadSpy).toHaveBeenCalledTimes(1); + }); + + it('captures and suppresses the load event if it happens before passive effects and a cascading update causes the img to be removed', async function() { + const container = document.createElement('div'); + const root = ReactDOM.createRoot(container); + + function ChildSuppressing({children}) { + const [showChildren, update] = React.useState(true); + React.useLayoutEffect(() => { + if (showChildren) { + update(false); + } + }, [showChildren]); + return showChildren ? children : null; + } + + React.startTransition(() => + root.render( + + + + + + + , + ), + ); + + expect(Scheduler).toFlushAndYieldThrough([ + 'render start', + 'Img default', + 'Yield', + ]); + const img = last(images); + loadImage(img); + expect(Scheduler).toHaveYielded(['actualLoadSpy [default]']); + expect(Scheduler).toFlushAndYield([ + 'a', + 'load triggered', + 'last layout', + 'last passive', + ]); + expect(img.__needsDispatch).toBe(true); + loadImage(img); + // we expect the browser to load the image again but since we are no longer rendering + // the img there will be no onLoad called + expect(Scheduler).toHaveYielded(['actualLoadSpy [default]']); + expect(Scheduler).toFlushWithoutYielding(); + expect(onLoadSpy).not.toHaveBeenCalled(); + }); + + it('captures and suppresses the load event if it happens before passive effects and a cascading update causes the img to be removed, alternate', async function() { + const container = document.createElement('div'); + const root = ReactDOM.createRoot(container); + + function Switch({children}) { + const [shouldShow, updateShow] = React.useState(true); + return children(shouldShow, updateShow); + } + + function UpdateSwitchInLayout({updateShow}) { + React.useLayoutEffect(() => { + updateShow(false); + }, []); + return null; + } + + React.startTransition(() => + root.render( + + {(shouldShow, updateShow) => ( + + <> + {shouldShow === true ? ( + <> + + + + + ) : null} + , + + + + )} + , + ), + ); + + expect(Scheduler).toFlushAndYieldThrough([ + // initial render + 'render start', + 'Img default', + 'Yield', + ]); + const img = last(images); + loadImage(img); + expect(Scheduler).toHaveYielded(['actualLoadSpy [default]']); + expect(Scheduler).toFlushAndYield([ + 'a', + 'load triggered', + // img is present at first + 'last layout', + 'last passive', + // sync re-render where the img is suppressed + 'render start', + 'last layout', + 'last passive', + ]); + expect(img.__needsDispatch).toBe(true); + loadImage(img); + // we expect the browser to load the image again but since we are no longer rendering + // the img there will be no onLoad called + expect(Scheduler).toHaveYielded(['actualLoadSpy [default]']); + expect(Scheduler).toFlushWithoutYielding(); + expect(onLoadSpy).not.toHaveBeenCalled(); + }); + + // it('captures the load event if it happens in a suspended subtree and replays it between layout and passive effects on resumption', async function() { + // function SuspendingWithImage() { + // Scheduler.unstable_yieldValue('SuspendingWithImage'); + // return ( + // }> + // + // + // + // + // + // ); + // } + + // const container = document.createElement('div'); + // const root = ReactDOM.createRoot(container); + + // React.startTransition(() => root.render()); + + // expect(Scheduler).toFlushAndYield([ + // 'SuspendingWithImage', + // 'Suspend! [A]', + // 'render start', + // 'Img default', + // 'Loading...', + // ]); + // let img = last(images); + // loadImage(img); + // expect(Scheduler).toHaveYielded(['actualLoadSpy [default]']); + // expect(onLoadSpy).not.toHaveBeenCalled(); + + // // Flush some of the time + // jest.advanceTimersByTime(50); + // // Still nothing... + // expect(Scheduler).toFlushWithoutYielding(); + + // // Flush the promise completely + // jest.advanceTimersByTime(50); + // // Renders successfully + // expect(Scheduler).toHaveYielded(['Promise resolved [A]']); + + // expect(Scheduler).toFlushAndYieldThrough([ + // 'A', + // // img was recreated on unsuspended tree causing new load event + // 'render start', + // 'Img default', + // 'last layout', + // ]); + + // expect(images.length).toBe(2); + // img = last(images); + // expect(img.__needsDispatch).toBe(true); + // loadImage(img); + // expect(Scheduler).toHaveYielded([ + // 'actualLoadSpy [default]', + // 'onLoadSpy [default]', + // ]); + + // expect(Scheduler).toFlushAndYield(['last passive']); + + // expect(onLoadSpy).toHaveBeenCalledTimes(1); + // }); + + it('correctly replays the last img load even when a yield + update causes the host element to change', async function() { + let externalSetSrc = null; + let externalSetSrcAlt = null; + + function Base() { + const [src, setSrc] = React.useState(null); + const [srcAlt, setSrcAlt] = React.useState(null); + externalSetSrc = setSrc; + externalSetSrcAlt = setSrcAlt; + return srcAlt || src ? : null; + } + + function YieldingWithImage({src}) { + Scheduler.unstable_yieldValue('YieldingWithImage'); + React.useEffect(() => { + Scheduler.unstable_yieldValue('Committed'); + }); + return ( + <> + + + + + ); + } + + const container = document.createElement('div'); + const root = ReactDOM.createRoot(container); + + root.render(); + + expect(Scheduler).toFlushWithoutYielding(); + + React.startTransition(() => externalSetSrc('a')); + + expect(Scheduler).toFlushAndYieldThrough([ + 'YieldingWithImage', + 'Img a', + 'Yield', + ]); + let img = last(images); + loadImage(img); + expect(Scheduler).toHaveYielded(['actualLoadSpy [a]']); + + ReactDOM.flushSync(() => externalSetSrcAlt('b')); + + expect(Scheduler).toHaveYielded([ + 'YieldingWithImage', + 'Img b', + 'Yield', + 'b', + 'load triggered', + 'Committed', + ]); + expect(images.length).toBe(2); + img = last(images); + expect(img.__needsDispatch).toBe(true); + loadImage(img); + + expect(Scheduler).toHaveYielded(['actualLoadSpy [b]', 'onLoadSpy [b]']); + // why is there another update here? + expect(Scheduler).toFlushAndYield([ + 'YieldingWithImage', + 'Img b', + 'Yield', + 'b', + 'Committed', + ]); + }); +}); diff --git a/packages/react-dom/src/client/ReactDOMHostConfig.js b/packages/react-dom/src/client/ReactDOMHostConfig.js index 97eb494ecba2a..fbd04da485fe3 100644 --- a/packages/react-dom/src/client/ReactDOMHostConfig.js +++ b/packages/react-dom/src/client/ReactDOMHostConfig.js @@ -146,17 +146,6 @@ const STYLE = 'style'; let eventsEnabled: ?boolean = null; let selectionInformation: null | SelectionInformation = null; -function shouldAutoFocusHostComponent(type: string, props: Props): boolean { - switch (type) { - case 'button': - case 'input': - case 'select': - case 'textarea': - return !!props.autoFocus; - } - return false; -} - export * from 'react-reconciler/src/ReactFiberHostConfigWithNoPersistence'; export function getRootHostContext( @@ -307,7 +296,17 @@ export function finalizeInitialChildren( hostContext: HostContext, ): boolean { setInitialProperties(domElement, type, props, rootContainerInstance); - return shouldAutoFocusHostComponent(type, props); + switch (type) { + case 'button': + case 'input': + case 'select': + case 'textarea': + return !!props.autoFocus; + case 'img': + return true; + default: + return false; + } } export function prepareUpdate( @@ -428,12 +427,25 @@ export function commitMount( // does to implement the `autoFocus` attribute on the client). But // there are also other cases when this might happen (such as patching // up text content during hydration mismatch). So we'll check this again. - if (shouldAutoFocusHostComponent(type, newProps)) { - ((domElement: any): - | HTMLButtonElement - | HTMLInputElement - | HTMLSelectElement - | HTMLTextAreaElement).focus(); + switch (type) { + case 'button': + case 'input': + case 'select': + case 'textarea': + if (newProps.autoFocus) { + ((domElement: any): + | HTMLButtonElement + | HTMLInputElement + | HTMLSelectElement + | HTMLTextAreaElement).focus(); + } + return; + case 'img': { + if ((newProps: any).src) { + ((domElement: any): HTMLImageElement).src = ((domElement: any): HTMLImageElement).src; + } + return; + } } }