Skip to content

Commit

Permalink
Fix next/image usage of onError() (#36305)
Browse files Browse the repository at this point in the history
## History
In PR #35889, `onError` prop was added explicitly on the `Image` component, but it was ommitted from the `ImageElement` component. In result, the callback didn't work at all.

## Now
This PR deletes the `onError` prop from explicitly defined props. Explicitly adding `onError` on the `const imgElementArgs` is considered, but deleting the defined prop from `Image` is more reasonable because the prop isn't used in `Image` component.


## Bug

- [x] Related issues linked using `fixes #number`
- [ ] Integration tests added
- [ ] Errors have helpful link attached, see `contributing.md`

Co-authored-by: Steven <229881+styfle@users.noreply.github.com>
  • Loading branch information
shinkj11 and styfle committed Apr 22, 2022
1 parent 94faeec commit 42c01dd
Show file tree
Hide file tree
Showing 3 changed files with 66 additions and 1 deletion.
1 change: 0 additions & 1 deletion packages/next/client/image.tsx
Expand Up @@ -368,7 +368,6 @@ export default function Image({
objectFit,
objectPosition,
onLoadingComplete,
onError,
placeholder = 'empty',
blurDataURL,
...all
Expand Down
44 changes: 44 additions & 0 deletions test/integration/image-component/default/pages/on-error.js
@@ -0,0 +1,44 @@
import { useState } from 'react'
import Image from 'next/image'

const Page = () => {
return (
<div>
<h1>Test onError</h1>
<p>
If error occured while loading image, native onError should be called.
</p>
<ImageWithMessage id="1" src="/test.png" layout="fill" />

<ImageWithMessage id="2" src="/nonexistent-img.png" layout="fill" />
<div id="footer" />
</div>
)
}

function ImageWithMessage({ id, ...props }) {
const [msg, setMsg] = useState('no error occured')
const style =
props.layout === 'fill'
? { position: 'relative', width: '64px', height: '64px' }
: {}

return (
<>
<div className="wrap" style={style}>
<Image
id={`img${id}`}
onError={(e) => {
const msg = `error occured while loading ${e.target.id}`
setMsg(msg)
}}
{...props}
/>
</div>
<p id={`msg${id}`}>{msg}</p>
<hr />
</>
)
}

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

it('should callback native onError when error occured while loading image', async () => {
let browser = await webdriver(appPort, '/on-error')

await check(
() => browser.eval(`document.getElementById("img1").currentSrc`),
/test\.png/
)
await check(
() => browser.eval(`document.getElementById("img2").currentSrc`),
//This is an empty data url
/nonexistent-img\.png/
)
await check(
() => browser.eval(`document.getElementById("msg1").textContent`),
'no error occured'
)
await check(
() => browser.eval(`document.getElementById("msg2").textContent`),
'error occured while loading img2'
)
})

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

0 comments on commit 42c01dd

Please sign in to comment.