vercel / next.js

The React Framework
https://nextjs.org
MIT License
125.47k stars 26.8k forks source link

Image onLoad triggers late #64458

Open theDanielJLewis opened 5 months ago

theDanielJLewis commented 5 months ago

Link to the code that reproduces this issue

https://codesandbox.io/p/devbox/hardcore-keller-xt5l7m?file=/app/TestImage.tsx:18,29

To Reproduce

  1. Start the application
  2. Reduce network speed to "Fast 3G" to better see the problem (but it's visible in not throttled, too).
  3. Observe how the red background disappears when the image is loaded, but the image remains transparent for actually longer than the red background shows before the image goes fully opaque.

Current vs. Expected behavior

I expect "onLoad" to trigger immediately when the image has loaded. Thus, any kind of placeholder, skeleton, or such will cease when the image is fully visible, and any such effect would not continue to apply on the image.

But that's not what happens.

In this simplified code demonstration, the red background represents the loading state, and that (and any styles or classes) should be removed when the image is loaded. I like to add animate-pulse from Tailwind on my loading skeletons, but I stuck with opacity: 0.25 for simplicity. That red background gets replaced with the image, but any classes or styles are still applied to the image after the image has already loaded. So in this case, the image remains at 25% opacity. Then after a few seconds (on "Fast 3G"), onLoad finally triggers and the change of state removes the styles and the image returns to full opacity.

This happens in both dev and start.

Provide environment information

Operating System:
  Platform: linux
  Arch: x64
  Version: #1 SMP PREEMPT_DYNAMIC Sun Aug  6 20:05:33 UTC 2023
  Available memory (MB): 4102
  Available CPU cores: 2
Binaries:
  Node: 20.9.0
  npm: 9.8.1
  Yarn: 1.22.19
  pnpm: 8.10.2
Relevant Packages:
  next: 14.2.0-canary.9 // There is a newer canary version (14.2.1-canary.4) available, please upgrade! 
  eslint-config-next: N/A
  react: 18.2.0
  react-dom: 18.2.0
  typescript: 5.1.3
Next.js Config:
  output: N/A

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

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

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

next dev (local), next start (local)

Additional context

I've experienced this since I started using Next.js with version 13. CodeSandbox is using 14.2.0-canary.9, but the problem still exists.

I've tried some crazy workarounds to this, but they are all unsatisfactory. I want this to just work.

theDanielJLewis commented 5 months ago

Here's a video demonstrating how this looks on my own site, after build and running start.

You first see it with "Fast 3G," and then I refresh again with no throttling.

https://github.com/vercel/next.js/assets/154936/05f2b4f2-cde4-4980-90a2-541ed98f6798

philwolstenholme commented 5 months ago

I've noticed this too, I was trying to fade in images once they had loaded by removing an opacity-0 utility class, but gave up on the idea when I noticed how much slower it had made the images appear.

michalhonc commented 5 months ago

I suspect it is due to the fact that Image component is using ref for the <img> as a way to retrieve the completed property from it, ie. not the native onload callback. This might be done due to possibility of the image being loaded before the client JS kicks in, thus missing the onLoad callback.

In a nutshell

  1. User gets HTML (with loading state instead of image)
  2. JS bundle for that image is being downloaded (still loading state for the image)
  3. The image is loaded (high chance that image size is smaller than JS bundle size) before the JS bundle (now image is shown BUT loaded state is still present)
  4. JS bundle is ready, onLoad callback is executed (image is shown and loading state is revoked)

To not over-engineer this with images having separate, smaller, high-priority bundles I suggest using plain CSS absolute element with lower z-index which gets overlapped by the image when loaded

philwolstenholme commented 5 months ago

I suspect it is due to the fact that Image component is using ref for the <img> as a way to retrieve the completed property from it, ie. not the native onload callback. This might be done due to possibility of the image being loaded before the client JS kicks in, thus missing the onLoad callback.

In a nutshell

  1. User gets HTML (with loading state instead of image)
  2. JS bundle for that image is being downloaded (still loading state for the image)
  3. The image is loaded (high chance that image size is smaller than JS bundle size) before the JS bundle (now image is shown BUT loaded state is still present)
  4. JS bundle is ready, onLoad callback is executed (image is shown and loading state is revoked)

To not over-engineer this with images having separate, smaller, high-priority bundles I suggest using plain CSS absolute element with lower z-index which gets overlapped by the image when loaded

Ah that makes a lot of sense.

I suppose people can always render an online script element and attach a standard browser onLoad event in there.

theDanielJLewis commented 3 hours ago

Does this problem still exist in Canary?

Edit by maintainer bot: Comment was automatically minimized because it was considered unhelpful. (If you think this was by mistake, let us know). Please only comment if it adds context to the issue. If you want to express that you have the same problem, use the upvote 👍 on the issue description or subscribe to the issue for updates. Thanks!