-
Notifications
You must be signed in to change notification settings - Fork 27k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
srcSet ignores sizes provided #27547
Comments
Ideally, there would also be a way to limit the number of Imagine I have a large hero image, which I'm displaying at Does this suggestion belong here or should I create a new issue/feature request for it? Thanks! |
Hey @markplewis, thanks for reminding me. I forgot to mention: ideally Nextjs wouldn’t attempt to scale up the image above native resolution ever, because this will always be inferior to allowing the browser to scale up. Usually a Lancszos or bicubic algorithm will look worse than simply doing nothing to the image. The only exception: if it used DLSS (deep learning super-scaling) to res up the imagine beyond its native resolution. I’ve used several of these cloud-based and desktop DLSS imaging solitons, and they can usually handle up to 400% up-res without compromising quality in most cases. |
If it’s working properly, the values you pass into The idea of using a consistent set of device sizes (shared across all |
Thanks @samuelgoff, all of that makes sense. I'm working on my first NextJs project and I was surprised that every image uses the same, global set of sizes to generates its
|
Hey @markplewis, bear in mind the browser has the luxury of using both If you look at dppx on caniuse, this doesn't really have great coverage. At least in the past, the media queries supported by If it were me, I would write the sizes rules to use (regular) |
Hey @markplewis, Going back to your original use case, if the largest resolution you want something to appear in the Regardless, if you provide your own |
Not sure if this is related but for some reason if you provide a query with calc, like this:
Probably this is caused by the same bug |
This could be unrelated, but the example image you use here reminds me of a separate set of issues: in general, a properly implemented SVG will tend to be smaller than PNG or WebP. For this reason, I've often inlined my SVGs instead of using the That said, it is entirely possible to encounter highly-detailed (or poorly-crafted) vector images (think distressed or grunge textures). I've seen examples where it's 50-150KB as a PNG, but over 1.4MB as an SVG. Ideally, |
The only reason I used svg was because it was a standard boilerplate demo. To be honest svg should not require srcset since the weight is size invariant (even if it embeds bitmap there is no way to optimize for most image cdns) |
This also happens with the new Image component in Next.js 13 |
I'm encountering one of the issues mentioned above where next.js is adding image sizes that are greater than the native image to the srcset (causing the image to get scaled up) when I'd really just like to serve the native image in that instance. If you use tools like https://ausi.github.io/respimagelint/ to validate you responsive images they will complain about this saying something like |
That's truly disappointing. I was hopeful they would've addressed this issue in the rewrite, because it creates unnecessary bloat & slowdown when building/deploying -- and even makes the rendered HTML larger than it needs to be -- to assume that every image is going to be used full width. I think that's a huge & false assumption. |
To Vercel's credit, they've added support for webp & avif, which makes the overall file size smaller (avif is roughly 50% smaller than JPEG, all things being equal), but they take significantly longer to encode. So encoding unnecessarily large images at full resolution (& down) actually makes the build/deploy performance issues I identified with v11 significantly worse. |
From the docs:
From my
If I put
So that part of the documentation seems correct, it won't include sizes that are so small they would never be served based on But if I do something like So yes, to the original point a bunch of extra srcSets are placed in the HTML which definitely adds some needless bloat. But here is my confusion: Does next.js actually generate all of these images as soon as a single one is requested, or does it only generate the srcSet needed when requested. I suspect it's the latter, in which case your statement of:
probably isn't true, since srcSets that are never requested are never generated. Anyway, interesting discussion and for sure shipping less html would be a nice optimization. |
This is a build-time generated set of images, which is why they need to be statically imported. There may be some lazy generation in dev mode, but whenever you do a production build (e.g. deploy), this is definitely doing a dependency analysis & deciding which images & sizes to generate, and then generating them all at that time, so they can be included with the deployment. Think about it: the build process generates a set of files that can be uploaded to a number of different JAMstack hosting providers; that set of files MUST include the images, because there is no opportunity to access the source repo after the build process (& upload) is complete. Vercel's Edge Image Optimization feature is a completely different animal, and that is doing optimizations on-demand, then caches in CDN for up to 30 days without being accessed before being expired. If it's accessed within a shorter timeframe, then it is retained in cache. |
Ah good point, I am talking about Edge Image Optimization (or equivalent, we're self hosting). In either case, having srcSets that will never be applicable is a waste, for sure. |
I don't know how Edge Image Optimization would interact with self-hosted. Are these dynamic images? If so, I would imagine the build-time generation of |
The image urls come through as part of the API we query in getStaticProps for page data. HTML is generated with the srcSets. The images are not generated though at this point, next.js does not generate images that aren't statically referenced in code until request time. Because we're self hosting, we install the Not sure if Vercel's version of this is doing something different, but we're using the out-of-the-box next/image component to accomplish this. |
Interesting. I didn't realize this was supported in self-hosted environments. Are the images CDN'd as a result? If so, how long are they retained for? |
The docs have a good section on caching behaviour and our CDN respects whatever we set for the |
Has anyone managed to find a way to manually add the |
@monolithed I ended up writing my own img component. Just ignore the TypeScript warning that next/image should be used. |
@cbratschi, could you provide an example where you were able to get it working? I tried passing the |
@monolithed This is a company project and I cannot share any source code. We ported our current logic from our web platform to Next.js. Yes, we use a lot of custom logic including direct blurhash support and lazy loading. Basically start with <img ... /> and add the features you need. You can also have a look at the Next.js source code. |
I'm also surprised with this behavior, but it seems (by the silence of Nextjs team) it is by design and not a bug. My custom Avatar componet, which uses images of max size 128x128px, now produces several completely unnecessary srcset items, larges one being 1080x108px. Docs say:
... so it seems they trim items that are too small. I just don't understand why they also don't trim items that are too large. At the end, for small(er) images, I reverted back to basic IMG tag and will extend it as needed. |
@psiho, you're absolutely right. Typically, image sizes are tailored to match various screen dimensions, and distorting or arbitrarily altering them is unacceptable. I'd go even further to say that sometimes images meant for smaller screens end up looking wider than those intended for larger ones. There can't be a one-size-fits-all solution in this matter. No one except the users of this component can determine what the correct image sizes should be. The component's authors suggest using 1400px 800px 450px If you pay attention, you'll notice that for a medium-sized screen (800px), a larger image is being used compared to a larger screen (1400px). And this is a common practice on many websites. Even Lightroom warns when the original image is smaller than the requested dimensions: |
I've recently come across this issue and it completely baffled me at first. The way I'd prefer a single way to generate I'd also like it if, when I use the I wrote about this issue in a recent post: https://morganfeeney.com/posts/sizes-attribute-nextjs-image |
Wonder if |
What version of Next.js are you using?
11.0.1
What version of Node.js are you using?
14.15.4
What browser are you using?
Chrome Version 92.0.4515.107 (Official Build) (x86_64)
What operating system are you using?
macOS 11.5
How are you deploying your application?
Vercel
Describe the Bug
When I supply valid
sizes
, Nextjs ignores the intersection between these sizes and thedeviceSizes
&imageSizes
contained in thenext.config.js
file. As a result, a file that will only ever appear on-screen at 280px (or 560px @ 2.0 pixel density, or 840px @ 3.0 pixel density) still generates every size, all the way up to 3840w.Fortunately, the browser still requests the correct image for each screen resolution, but there are many sizes which are unreachable by any browser or device, and these are typically the largest, most resource-intensive ones to produce and maintain.
Expected Behavior
If provided, Nextjs should parse the
sizes
value to determine whichdeviceSizes
&imageSizes
are applicable to each image. Since both monitors & devices have a pixel density from 1.0 to 3.0 (or higher, though I recommend this as a sane limit), the smallest value in the sizes can be used as-is as a lower bound, whereas the largestsizes
value should be multiplied by 3 to define an upper bound. Once these values are defined, they can be used as a floor/ceiling against the union of values indeviceSizes
&imageSizes
when generating scaled/optimized images. Values just outside of the target range may be worth generating, as well, so the browser has the flexibility to decide which image is most appropriate to request.For example, given
sizes="(max-width: 399px) 184px,(max-width: 519px) 244px,(max-width: 639px) 200px,(max-width: 767px) 156px,(max-width: 1023px) 220px,(max-width: 1279px) 280px,280px"
:Given my current
next.config.js
include the following:...the smallest file size that needs to be generated is 144px and the largest is 880px. Everything outside of that range can be ignored for the generation of this
srcSet
. In addition to the smallest, note this includes:Each of these files takes time, processing & storage resources to generate and deploy. Eliminating them will deploy faster, require less processing, and require a smaller storage footprint. Additionally, the smaller
srcSet
will produce smaller HTML files, so they will download faster.To Reproduce
Start with a source image provided at 3840px width and use
import
as recommended by the docs:import missedTarget from '/public/images/missed-target.jpg';
Use
next/image
component with valid sizes smaller than the union ofdeviceSizes
&imageSizes
:Optionally, provide a
next.config.js
including the following:The rendered result will look like this:
Note the
srcSet
includes all image sizes – all the way up to 3840 – regardless of whether they could possibly be displayed by any browser or device.The text was updated successfully, but these errors were encountered: