From 7a2d331e1cb8646f8667906f3ed72d20a82f5c93 Mon Sep 17 00:00:00 2001 From: Steven Date: Mon, 22 Aug 2022 11:37:35 -0400 Subject: [PATCH 1/3] Add fix for onerror hanlding --- docs/api-reference/next/future/image.md | 5 +---- packages/next/client/future/image.tsx | 6 +++++ .../pages/on-error-before-hydration.js | 22 +++++++++++++++++++ .../image-future/default/test/index.test.ts | 8 +++++++ 4 files changed, 37 insertions(+), 4 deletions(-) create mode 100644 test/integration/image-future/default/pages/on-error-before-hydration.js diff --git a/docs/api-reference/next/future/image.md b/docs/api-reference/next/future/image.md index 76e2ee33cddc..862270c60d9c 100644 --- a/docs/api-reference/next/future/image.md +++ b/docs/api-reference/next/future/image.md @@ -39,7 +39,6 @@ Compared to `next/image`, the new `next/future/image` component has the followin - Removes `layout`, `objectFit`, and `objectPosition` props in favor of `style` or `className` - Removes `IntersectionObserver` implementation in favor of [native lazy loading](https://caniuse.com/loading-lazy-attr) - Removes `loader` config in favor of [`loader`](#loader) prop -- Note: the [`onError`](#onerror) prop might behave differently ## Known Browser Bugs @@ -312,7 +311,7 @@ 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). @@ -320,8 +319,6 @@ Instead, use [`onLoadingComplete`](#onloadingcomplete). 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 diff --git a/packages/next/client/future/image.tsx b/packages/next/client/future/image.tsx index 81cc5e45d964..1d7570766104 100644 --- a/packages/next/client/future/image.tsx +++ b/packages/next/client/future/image.tsx @@ -359,6 +359,12 @@ const ImageElement = ({ style={{ ...imgStyle, ...blurStyle }} ref={useCallback( (img: ImgElementWithDataProp) => { + if (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). + img.src = img.src + } if (process.env.NODE_ENV !== 'production') { if (img && !srcString) { console.error(`Image is missing required "src" property:`, img) diff --git a/test/integration/image-future/default/pages/on-error-before-hydration.js b/test/integration/image-future/default/pages/on-error-before-hydration.js new file mode 100644 index 000000000000..ac68b482a3fc --- /dev/null +++ b/test/integration/image-future/default/pages/on-error-before-hydration.js @@ -0,0 +1,22 @@ +import { useState } from 'react' +import Image from 'next/future/image' + +const Page = () => { + const [msg, setMsg] = useState(`default state`) + return ( +
+ { + setMsg(`error state`) + }} + /> +

{msg}

+
+ ) +} + +export default Page diff --git a/test/integration/image-future/default/test/index.test.ts b/test/integration/image-future/default/test/index.test.ts index bf6a00d046e4..a21d1112da25 100644 --- a/test/integration/image-future/default/test/index.test.ts +++ b/test/integration/image-future/default/test/index.test.ts @@ -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 { From 502addfa63dacd6fa65029a00fa1477b7c903de8 Mon Sep 17 00:00:00 2001 From: Steven Date: Mon, 22 Aug 2022 11:50:51 -0400 Subject: [PATCH 2/3] Check for null ref --- packages/next/client/future/image.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/next/client/future/image.tsx b/packages/next/client/future/image.tsx index 1d7570766104..ce8d450c0bad 100644 --- a/packages/next/client/future/image.tsx +++ b/packages/next/client/future/image.tsx @@ -358,8 +358,8 @@ const ImageElement = ({ loading={loading} style={{ ...imgStyle, ...blurStyle }} ref={useCallback( - (img: ImgElementWithDataProp) => { - if (onError) { + (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). From 9bb6b28dbd7bae443441970e3828b5c1e345b442 Mon Sep 17 00:00:00 2001 From: Steven Date: Mon, 22 Aug 2022 12:07:27 -0400 Subject: [PATCH 3/3] Add onError to deps array --- packages/next/client/future/image.tsx | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/packages/next/client/future/image.tsx b/packages/next/client/future/image.tsx index ce8d450c0bad..5fe483f337fe 100644 --- a/packages/next/client/future/image.tsx +++ b/packages/next/client/future/image.tsx @@ -363,6 +363,7 @@ const ImageElement = ({ // 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') { @@ -380,7 +381,13 @@ const ImageElement = ({ ) } }, - [srcString, placeholder, onLoadingCompleteRef, setBlurComplete] + [ + srcString, + placeholder, + onLoadingCompleteRef, + setBlurComplete, + onError, + ] )} onLoad={(event) => { const img = event.currentTarget as ImgElementWithDataProp