Skip to content

Commit

Permalink
Fix onLoad prop in next/future/image (#41374)
Browse files Browse the repository at this point in the history 
Fixes a bug in `next/future/image` where `onLoad` might not be called.
This is the same workaround we added for `onError`.
  • Loading branch information
@styfle
styfle committed Oct 13, 2022
1 parent 46bdef8 commit 7fc301d
Show file tree
Hide file tree
Showing 3 changed files with 112 additions and 21 deletions.
43 changes: 39 additions & 4 deletions packages/next/client/future/image.tsx
View file
Expand Up @@ -48,7 +48,7 @@ type ImageLoaderPropsWithConfig = ImageLoaderProps & {
}

type PlaceholderValue = 'blur' | 'empty'

type OnLoad = React.ReactEventHandler<HTMLImageElement> | undefined
type OnLoadingComplete = (img: HTMLImageElement) => void

type ImgElementStyle = NonNullable<JSX.IntrinsicElements['img']['style']>
Expand Down Expand Up @@ -128,6 +128,7 @@ type ImageElementProps = Omit<ImageProps, 'src' | 'alt' | 'loader'> & {
unoptimized: boolean
loader: ImageLoaderWithConfig
placeholder: PlaceholderValue
onLoadRef: React.MutableRefObject<OnLoad | undefined>
onLoadingCompleteRef: React.MutableRefObject<OnLoadingComplete | undefined>
setBlurComplete: (b: boolean) => void
setShowAltText: (b: boolean) => void
Expand Down Expand Up @@ -245,6 +246,7 @@ function handleLoading(
img: ImgElementWithDataProp,
src: string,
placeholder: PlaceholderValue,
onLoadRef: React.MutableRefObject<OnLoad | undefined>,
onLoadingCompleteRef: React.MutableRefObject<OnLoadingComplete | undefined>,
setBlurComplete: (b: boolean) => void
) {
Expand All @@ -265,6 +267,30 @@ function handleLoading(
if (placeholder === 'blur') {
setBlurComplete(true)
}
if (onLoadRef?.current) {
const event = new Event('load')
Object.defineProperty(event, 'target', { writable: false, value: img })
let prevented = false
let stopped = false
const sytheticEvent = {
...event,
nativeEvent: event,
currentTarget: img,
target: img,
isDefaultPrevented: () => prevented,
isPropagationStopped: () => stopped,
persist: () => {},
preventDefault: () => {
prevented = true
event.preventDefault()
},
stopPropagation: () => {
stopped = true
event.stopPropagation()
},
}
onLoadRef.current(sytheticEvent)
}
if (onLoadingCompleteRef?.current) {
onLoadingCompleteRef.current(img)
}
Expand Down Expand Up @@ -331,6 +357,7 @@ const ImageElement = ({
config,
unoptimized,
loader,
onLoadRef,
onLoadingCompleteRef,
setBlurComplete,
setShowAltText,
Expand Down Expand Up @@ -397,6 +424,7 @@ const ImageElement = ({
img,
srcString,
placeholder,
onLoadRef,
onLoadingCompleteRef,
setBlurComplete
)
Expand All @@ -405,6 +433,7 @@ const ImageElement = ({
[
srcString,
placeholder,
onLoadRef,
onLoadingCompleteRef,
setBlurComplete,
onError,
Expand All @@ -416,12 +445,10 @@ const ImageElement = ({
img,
srcString,
placeholder,
onLoadRef,
onLoadingCompleteRef,
setBlurComplete
)
if (onLoad) {
onLoad(event)
}
}}
onError={(event) => {
// if the real image fails to load, this will ensure "alt" is visible
Expand Down Expand Up @@ -515,6 +542,7 @@ export default function Image({
height,
fill,
style,
onLoad,
onLoadingComplete,
placeholder = 'empty',
blurDataURL,
Expand Down Expand Up @@ -837,6 +865,12 @@ export default function Image({
crossOrigin: rest.crossOrigin,
}

const onLoadRef = useRef(onLoad)

useEffect(() => {
onLoadRef.current = onLoad
}, [onLoad])

const onLoadingCompleteRef = useRef(onLoadingComplete)

useEffect(() => {
Expand All @@ -859,6 +893,7 @@ export default function Image({
placeholder,
loader,
srcString,
onLoadRef,
onLoadingCompleteRef,
setBlurComplete,
setShowAltText,
Expand Down
41 changes: 32 additions & 9 deletions test/integration/image-future/default/pages/on-load.js
View file
Expand Up @@ -2,6 +2,7 @@ import { useState } from 'react'
import Image from 'next/future/image'

const Page = () => {
const [idToCount, setIdToCount] = useState({})
const [clicked, setClicked] = useState(false)

const red =
Expand All @@ -10,10 +11,7 @@ const Page = () => {
return (
<div>
<h1>Test onLoad</h1>
<p>
This is the native onLoad which doesn't work as many places as
onLoadingComplete
</p>
<p>This is the native onLoad</p>
<button id="toggle" onClick={() => setClicked(!clicked)}>
Toggle
</button>
Expand All @@ -23,6 +21,8 @@ const Page = () => {
src={clicked ? '/test.jpg' : red}
width="128"
height="128"
idToCount={idToCount}
setIdToCount={setIdToCount}
/>

<ImageWithMessage
Expand All @@ -31,43 +31,66 @@ const Page = () => {
placeholder={clicked ? 'blur' : 'empty'}
width="256"
height="256"
idToCount={idToCount}
setIdToCount={setIdToCount}
/>

<ImageWithMessage
id="3"
src={clicked ? '/test.svg' : red}
width="1200"
height="1200"
idToCount={idToCount}
setIdToCount={setIdToCount}
/>

<ImageWithMessage
id="4"
src={clicked ? '/test.ico' : red}
width={200}
height={200}
idToCount={idToCount}
setIdToCount={setIdToCount}
/>

<ImageWithMessage
id="5"
src={clicked ? '/wide.png' : red}
width="500"
height="500"
src="/wide.png"
width="600"
height="300"
idToCount={idToCount}
setIdToCount={setIdToCount}
/>

<ImageWithMessage
id="6"
src={red}
width="300"
height="300"
idToCount={idToCount}
setIdToCount={setIdToCount}
/>

<div id="footer" />
</div>
)
}

function ImageWithMessage({ id, ...props }) {
function ImageWithMessage({ id, idToCount, setIdToCount, ...props }) {
const [msg, setMsg] = useState('[LOADING]')

return (
<>
<div className="wrap">
<Image
id={`img${id}`}
alt={`img${id}`}
onLoad={(e) => {
setMsg(`loaded ${e.target.id} with native onLoad`)
let count = idToCount[id] || 0
count++
idToCount[id] = count
setIdToCount(idToCount)
setMsg(`loaded ${e.target.id} with native onLoad, count ${count}`)
}}
{...props}
/>
Expand Down
49 changes: 41 additions & 8 deletions test/integration/image-future/default/test/index.test.ts
View file
Expand Up @@ -300,34 +300,36 @@ function runTests(mode) {
)
})

it('should callback native onLoad in most cases', async () => {
it('should callback native onLoad with sythetic event', async () => {
let browser = await webdriver(appPort, '/on-load')

await browser.eval('document.getElementById("toggle").click()')

await browser.eval(
`document.getElementById("footer").scrollIntoView({behavior: "smooth"})`
)

await check(
() => browser.eval(`document.getElementById("msg1").textContent`),
'loaded img1 with native onLoad'
'loaded img1 with native onLoad, count 1'
)
await check(
() => browser.eval(`document.getElementById("msg2").textContent`),
'loaded img2 with native onLoad'
'loaded img2 with native onLoad, count 1'
)
await check(
() => browser.eval(`document.getElementById("msg3").textContent`),
'loaded img3 with native onLoad'
'loaded img3 with native onLoad, count 1'
)
await check(
() => browser.eval(`document.getElementById("msg4").textContent`),
'loaded img4 with native onLoad'
'loaded img4 with native onLoad, count 1'
)
await check(
() => browser.eval(`document.getElementById("msg5").textContent`),
'loaded img5 with native onLoad'
'loaded img5 with native onLoad, count 1'
)
await check(
() => browser.eval(`document.getElementById("msg6").textContent`),
'loaded img6 with native onLoad, count 1'
)
await check(
() =>
Expand All @@ -337,6 +339,33 @@ function runTests(mode) {
'future'
)

await browser.eval('document.getElementById("toggle").click()')

await check(
() => browser.eval(`document.getElementById("msg1").textContent`),
'loaded img1 with native onLoad, count 2'
)
await check(
() => browser.eval(`document.getElementById("msg2").textContent`),
'loaded img2 with native onLoad, count 2'
)
await check(
() => browser.eval(`document.getElementById("msg3").textContent`),
'loaded img3 with native onLoad, count 2'
)
await check(
() => browser.eval(`document.getElementById("msg4").textContent`),
'loaded img4 with native onLoad, count 2'
)
await check(
() => browser.eval(`document.getElementById("msg5").textContent`),
'loaded img5 with native onLoad, count 1'
)
await check(
() => browser.eval(`document.getElementById("msg6").textContent`),
'loaded img6 with native onLoad, count 1'
)

await check(
() => browser.eval(`document.getElementById("img1").currentSrc`),
/test(.*)jpg/
Expand All @@ -357,6 +386,10 @@ function runTests(mode) {
() => browser.eval(`document.getElementById("img5").currentSrc`),
/wide.png/
)
await check(
() => browser.eval(`document.getElementById("img6").currentSrc`),
'data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR42mO8ysv7HwAEngHwC+JqOgAAAABJRU5ErkJggg=='
)
})

it('should callback native onError when error occured while loading image', async () => {
Expand Down

0 comments on commit 7fc301d

Please sign in to comment.