Skip to content

Commit

Permalink
Fix onError handling in next/future/image (#39824)
Browse files Browse the repository at this point in the history
This PR fixes a race condition where the `onError` handler was never called because the error happened before react attached the error handler.
  • Loading branch information
styfle committed Aug 22, 2022
1 parent 1f5da04 commit 7e9f9bf
Show file tree
Hide file tree
Showing 4 changed files with 46 additions and 6 deletions.
5 changes: 1 addition & 4 deletions docs/api-reference/next/future/image.md
Expand Up @@ -43,7 +43,6 @@ Compared to `next/image`, the new `next/future/image` component has the followin
- Removes `lazyBoundary` prop since there is no native equivalent
- Removes `lazyRoot` prop since there is no native equivalent
- Removes `loader` config in favor of [`loader`](#loader) prop
- Note: the [`onError`](#onerror) prop might behave differently

## Known Browser Bugs

Expand Down Expand Up @@ -316,16 +315,14 @@ The callback function will be called with one argument, an object with the follo

A callback function that is invoked when the image is loaded.

Note that the load event might occur before client-side hydration completes, so this callback might not be invoked in that case.
Note that the load event might occur before the placeholder is removed and the image is fully decoded.

Instead, use [`onLoadingComplete`](#onloadingcomplete).

### onError

A callback function that is invoked if the image fails to load.

Note that the error might occur before client-side hydration completes, so this callback might not be invoked in that case.

### loading

> **Attention**: This property is only meant for advanced usage. Switching an
Expand Down
17 changes: 15 additions & 2 deletions packages/next/client/future/image.tsx
Expand Up @@ -358,7 +358,14 @@ const ImageElement = ({
loading={loading}
style={{ ...imgStyle, ...blurStyle }}
ref={useCallback(
(img: ImgElementWithDataProp) => {
(img: ImgElementWithDataProp | null) => {
if (img && onError) {
// If the image has an error before react hydrates, then the error is lost.
// The workaround is to wait until the image is mounted which is after hydration,
// then we set the src again to trigger the error handler (if there was an error).
// eslint-disable-next-line no-self-assign
img.src = img.src
}
if (process.env.NODE_ENV !== 'production') {
if (img && !srcString) {
console.error(`Image is missing required "src" property:`, img)
Expand All @@ -374,7 +381,13 @@ const ImageElement = ({
)
}
},
[srcString, placeholder, onLoadingCompleteRef, setBlurComplete]
[
srcString,
placeholder,
onLoadingCompleteRef,
setBlurComplete,
onError,
]
)}
onLoad={(event) => {
const img = event.currentTarget as ImgElementWithDataProp
Expand Down
@@ -0,0 +1,22 @@
import { useState } from 'react'
import Image from 'next/future/image'

const Page = () => {
const [msg, setMsg] = useState(`default state`)
return (
<div>
<Image
id="img"
src="/404.jpg"
width="200"
height="200"
onError={() => {
setMsg(`error state`)
}}
/>
<p id="msg">{msg}</p>
</div>
)
}

export default Page
8 changes: 8 additions & 0 deletions test/integration/image-future/default/test/index.test.ts
Expand Up @@ -387,6 +387,14 @@ function runTests(mode) {
)
})

it('should callback native onError even when error before hydration', async () => {
let browser = await webdriver(appPort, '/on-error-before-hydration')
await check(
() => browser.eval(`document.getElementById("msg").textContent`),
'error state'
)
})

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

0 comments on commit 7e9f9bf

Please sign in to comment.