Niels-IO / next-image-export-optimizer

Use Next.js advanced <Image/> component with the static export functionality. Optimizes all static images in an additional step after the Next.js static export.
MIT License
460 stars 56 forks source link

Image not working as expected #102

Closed PeterKottas closed 1 year ago

PeterKottas commented 1 year ago

This might be connected to https://github.com/Niels-IO/next-image-export-optimizer/issues/45 but I am not sure.

I noticed that when I load an image with priority, it won't load optimized images anymore. Here's a picture without priority: images2 Here's one with: images Notice there's no webp and all images are the same. I wonder if the way I load it might be causing it. This is my setup:

import poster from "assets/images/home/lead/main-marketing-video-poster.jpg";
const placeholder: ExportedImageProps = {
  src: poster,
  alt: "GuestBell",
  sizes: posterSizes,
  priority: true,
  placeholder: "empty",
};

This is then passed to another component which is the one that renders the actual ExportedImage component. Might this be the reason? Naturally, it should still work, but I am thinking the fact that I load the image in a different file than where I use it could be messing up something in the build process.

PeterKottas commented 1 year ago

A couple of new findings:

  1. This behavior is non-deterministic so it might be difficult to replicate
  2. It's apparently caused (or rather announced) by a few things a) You will see this error popup: Image with src "/_next/static/media/main-marketing-video-poster.157bf3ec.jpg" has a "loader" property that does not implement width. Please implement it or use the "unoptimized" property instead. b) If you pop into the networks tab, you will notice that the loading of the image was canceled: canceled
  3. As a result, I assume the loading state of the image changes to error here or here
  4. That will cause the loader to switch to the fallback loader here
  5. As a result, only the original version is loaded

So the culprit as it seems is the canceled loading of the image, why this happens I assume is the question here. Also, we could possibly be more lenient when determining the onError? Maybe let canceled through? Although still, not sure what happens then. Will it actually load the correct image after the canceled one? Does this maybe have something to do with layout movement? Like ... it first wants to load some resolution, then it changes its mind mid-way, and decides to load a different one. Cancels the prior ... maybe that's what's going on? Maybe that's what's making it flaky.

Sidenote: I am using next-pwa which might have some effect on this. But then again it might not, who knows.

PeterKottas commented 1 year ago

Ok so for now, I forked and disabled all that error handling, and the code is now working as expected. Weirdly I don't even see the canceled requests anymore. I wonder if it's actually caused by that naturalWidth check? Maybe that's how it gets canceled. Maybe that error fallback can be an opt-in feature. Or at least there could be an opt-out. Cause frankly, as long as the optimizer logic is working (the images are generated), that error should basically never happen. And if it does, chances are the original image is gone too. I am not sure here though.

Niels-IO commented 1 year ago

Removing the error handling as it is currently would be very dangerous. Right now, it always falls back to the original src, which is good behavior.

Have you noticed the bug also in production or only in the dev mode?

PeterKottas commented 1 year ago

Yeah, removing it certainly isn't an option. Just did that for us for the time being as we can pretty much guarantee the ok images are there. Struggling to find a way to tell fake errors apart from real ones. Yes production too.

Niels-IO commented 1 year ago

I can't reproduce the error right now, so I'm going to close this for the time being. If you figure out how to reproduce it, please give me a heads-up!

PeterKottas commented 1 year ago

Yeah, I still see this issue with the current version. I am using my fork where the error handling is disabled. No idea what this is about. Maybe somebody in the future will find this and be the wiser. The problem is people might be getting this, but never realizing it as it's not reliable. I only got it ~10% time, and only noticed when Lighthouse reported bad results for image compression. Subsequently, I've also replicated directly by testing on my machine. That's all I can give you so far. Feel free to keep it closed, I'll reopen if I find something. What I would say though, it might not be a terrible idea to add a flag that disables this fallback.