Skip to content

Commit

Permalink
Fix next/image usage in most cases of onLoad() (#36176)
Browse files Browse the repository at this point in the history
Even though we never documented it, the `onLoad` prop used to work (in most cases) in Next.js 12.1.4 and broke in 12.1.5 once the code was refactored in PR #35889.

We still shouldn't document it because `onLoad` doesn't work in all cases so [`onLoadingComplete`](https://nextjs.org/docs/api-reference/next/image#onloadingcomplete) is preferred, however tests were added for the cases that do work.

Use cases that don't work with `onLoad` are Data URLs as well as `loading="eager"`.
  • Loading branch information
styfle committed Apr 15, 2022
1 parent cbb64ee commit 4c872cb
Show file tree
Hide file tree
Showing 3 changed files with 179 additions and 0 deletions.
4 changes: 4 additions & 0 deletions packages/next/client/image.tsx
Expand Up @@ -884,6 +884,7 @@ const ImageElement = ({
onLoadingCompleteRef,
setBlurComplete,
setIntersection,
onLoad,
onError,
isVisible,
...rest
Expand Down Expand Up @@ -933,6 +934,9 @@ const ImageElement = ({
onLoadingCompleteRef,
setBlurComplete
)
if (onLoad) {
onLoad(event)
}
}}
onError={(event) => {
if (placeholder === 'blur') {
Expand Down
101 changes: 101 additions & 0 deletions test/integration/image-component/default/pages/on-load.js
@@ -0,0 +1,101 @@
import { useState } from 'react'
import Image from 'next/image'

const Page = () => {
// Hoisted state to count each image load callback
const [idToCount, setIdToCount] = useState({})
const [clicked, setClicked] = useState(false)

return (
<div>
<h1>Test onLoad</h1>
<p>
This is the native onLoad which doesn't work as many places as
onLoadingComplete
</p>
<ImageWithMessage
id="1"
src="/test.jpg"
layout="intrinsic"
width="128"
height="128"
idToCount={idToCount}
setIdToCount={setIdToCount}
/>

<ImageWithMessage
id="2"
src={require('../public/test.png')}
placeholder="blur"
layout="fixed"
idToCount={idToCount}
setIdToCount={setIdToCount}
/>

<ImageWithMessage
id="3"
src="/test.svg"
layout="responsive"
width="1200"
height="1200"
idToCount={idToCount}
setIdToCount={setIdToCount}
/>

<ImageWithMessage
id="4"
src="/test.ico"
layout="fill"
objectFit="contain"
idToCount={idToCount}
setIdToCount={setIdToCount}
/>

<ImageWithMessage
id="8"
src={clicked ? '/foo/test-rect.jpg' : '/wide.png'}
layout={clicked ? 'fixed' : 'intrinsic'}
width="500"
height="500"
idToCount={idToCount}
setIdToCount={setIdToCount}
/>

<button id="toggle" onClick={() => setClicked(!clicked)}>
Toggle
</button>
<div id="footer" />
</div>
)
}

function ImageWithMessage({ id, idToCount, setIdToCount, ...props }) {
const [msg, setMsg] = useState('[LOADING]')
const style =
props.layout === 'fill'
? { position: 'relative', width: '64px', height: '64px' }
: {}
return (
<>
<div className="wrap" style={style}>
<Image
id={`img${id}`}
onLoad={(e) => {
let count = idToCount[id] || 0
count++
idToCount[id] = count
setIdToCount(idToCount)
const msg = `loaded ${count} ${e.target.id} with native onLoad`
setMsg(msg)
console.log(msg)
}}
{...props}
/>
</div>
<p id={`msg${id}`}>{msg}</p>
<hr />
</>
)
}

export default Page
74 changes: 74 additions & 0 deletions test/integration/image-component/default/test/index.test.js
Expand Up @@ -289,6 +289,80 @@ function runTests(mode) {
}
})

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

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

await check(
() => browser.eval(`document.getElementById("img1").currentSrc`),
/test(.*)jpg/
)
await check(
() => browser.eval(`document.getElementById("img2").currentSrc`),
/test(.*).png/
)
await check(
() => browser.eval(`document.getElementById("img3").currentSrc`),
/test\.svg/
)
await check(
() => browser.eval(`document.getElementById("img4").currentSrc`),
/test(.*)ico/
)
await check(
() => browser.eval(`document.getElementById("msg1").textContent`),
'loaded 1 img1 with native onLoad'
)
await check(
() => browser.eval(`document.getElementById("msg2").textContent`),
'loaded 1 img2 with native onLoad'
)
await check(
() => browser.eval(`document.getElementById("msg3").textContent`),
'loaded 1 img3 with native onLoad'
)
await check(
() => browser.eval(`document.getElementById("msg4").textContent`),
'loaded 1 img4 with native onLoad'
)
await check(
() => browser.eval(`document.getElementById("msg8").textContent`),
'loaded 1 img8 with native onLoad'
)
await check(
() =>
browser.eval(
`document.getElementById("img8").getAttribute("data-nimg")`
),
'intrinsic'
)
await check(
() => browser.eval(`document.getElementById("img8").currentSrc`),
/wide.png/
)
await browser.eval('document.getElementById("toggle").click()')
// The normal `onLoad()` is triggered by lazy placeholder image
// so ideally this would be "2" instead of "3" count
await check(
() => browser.eval(`document.getElementById("msg8").textContent`),
'loaded 3 img8 with native onLoad'
)
await check(
() =>
browser.eval(
`document.getElementById("img8").getAttribute("data-nimg")`
),
'fixed'
)
await check(
() => browser.eval(`document.getElementById("img8").currentSrc`),
/test-rect.jpg/
)
})

it('should work with image with blob src', async () => {
let browser
try {
Expand Down

0 comments on commit 4c872cb

Please sign in to comment.