next/image: Setting onError needlessly resets src on rerenders

[SheepTester]

Link to the code that reproduces this issue

https://codesandbox.io/p/devbox/boring-wright-cf45dc

To Reproduce

1. Start the application
2. Observe how the left gif (which has onError set) restarts every second, when there's a rerender due to a state change, while the right gif plays normally.
3. Comment out line 41 onError={() => {}}
4. The left gif no longer restarts every second

Current vs. Expected behavior

I expect the image to behave the same regardless of whether onError is set or not. However, due to the img.src = img.src line linked below, setting onError has several side-effects (because img.src = img.src is not pure)

1. The image loads again, which fires onLoad on every re-render
2. Because the image is loaded again, gifs restart on every re-render
3. Frequent state changes, such as dragging an image around, will look very choppy as the image flashes between unloaded and loaded

Verify canary release

• [X] I verified that the issue exists in the latest Next.js canary release

Provide environment information

Operating System:
  Platform: linux
  Arch: x64
  Version: #1 SMP PREEMPT_DYNAMIC Sun Aug  6 20:05:33 UTC 2023
Binaries:
  Node: 20.9.0
  npm: 9.8.1
  Yarn: 1.22.19
  pnpm: 8.10.2
Relevant Packages:
  next: 14.0.5-canary.32
  eslint-config-next: 14.0.3
  react: 18.2.0
  react-dom: 18.2.0
  typescript: 5.3.2
Next.js Config:
  output: N/A

Which area(s) are affected? (Select all that apply)

Image optimization (next/image, next/legacy/image)

Additional context

I believe this is due to the following lines, introduced in #39824

https://github.com/vercel/next.js/blob/fa32e32df47774c60a731553c447e1d4880bdd4b/packages/next/src/client/image-component.tsx#L241-L247

[Avi98]

@SheepTester can you try sending a memoized callback for error? this (useCallback)[https://github.com/vercel/next.js/blob/fa32e32df47774c60a731553c447e1d4880bdd4b/packages/next/src/client/image-component.tsx#L229] has the onError dependency.

[SheepTester]

Ah, memoizing the callback fixes it, thanks! This fixes the issue I had in my project.

I suppose for such an easy fix, this issue could be closed, unless there's still interest in improving this?

[colbyfayock]

i noticed this happening to myself in dev mode only in a library i'm working on: https://github.com/cloudinary-community/next-cloudinary/blob/main/next-cloudinary/src/components/CldImage/CldImage.tsx#L156

its not as much of a concern because it seems to only be reproducible in dev mode, however, i'm still seeing duplicate image requests come through, yet when similarly commenting out onError, it doesn't happen anymore

im currently using a callback (though certainly could be doing something wrong)

ive even tried simply passing back the simple instance from SheepTester's example above:

const handleError = useCallback(() => {}, []);

  return (
    <ResolvedImage
      key={imgKey}
      {...imageProps}
      loader={loader}
      onError={handleOnError}
      ref={ref}
    />
  );

yet still see the duplicate request

for sake of testing things, i commented out pretty much all other props (including the key) and only onError impacts this

one thing of note is the wrapper component only renders "once"

With onError

Without onError

[imansalhi]

i think this is a bug ihave same issue with onError: import NextImage from "next/image"; import placeholderImg from "@/assets/imgs/placeholder.png"; import { useCallback, useMemo } from "react"; export default function Image({ lazy, src, ...props }) { const memoizedSrc = useMemo(() => src, [src]); const handleError = useCallback((e) => { const img = e.target; if (img.src === placeholderImg.src) { return; } img.style = "object-fit: none; background: var(--color-light-3); max-height: 100%;"; img.src = placeholderImg.src; }, []); return ( <NextImage {...props} src={memoizedSrc} onError={handleError} priority={!lazy} loading={lazy ? "lazy" : "eager"} /> ); }

@colbyfayock