nuxt / image

Plug-and-play image optimization for Nuxt applications.
https://image.nuxt.com
MIT License
1.35k stars 271 forks source link

Image size set to `1w` if sizes is set to `100vw` #1433

Open stijns96 opened 3 months ago

stijns96 commented 3 months ago

Somehow 100vw will be converted to a 1w and 2w url and I don't understand why.

Local

<img
  sizes="(max-width: 640px) 100vw, (max-width: 768px) 50vw, 400px"
  srcset="
    /_ipx/w_1/twitter-image.png     1w,
    /_ipx/w_2/twitter-image.png     2w,
    /_ipx/w_320/twitter-image.png 320w,
    /_ipx/w_400/twitter-image.png 400w,
    /_ipx/w_640/twitter-image.png 640w,
    /_ipx/w_800/twitter-image.png 800w
  "
  src="/_ipx/w_800/twitter-image.png"
/>

Storyblok

<img
  sizes="(max-width: 640px) 100vw, (max-width: 768px) 50vw, 400px"
  srcset="
    https://a.storyblok.com/f/23194/3000x2000/09das0980/image.png/m/1x0     1w,
    https://a.storyblok.com/f/23194/3000x2000/09das0980/image.png/m/2x0     2w,
    https://a.storyblok.com/f/23194/3000x2000/09das0980/image.png/m/320x0 320w,
    https://a.storyblok.com/f/23194/3000x2000/09das0980/image.png/m/400x0 400w,
    https://a.storyblok.com/f/23194/3000x2000/09das0980/image.png/m/640x0 640w,
    https://a.storyblok.com/f/23194/3000x2000/09das0980/image.png/m/800x0 800w
  "
  src="https://a.storyblok.com/f/23194/3000x2000/09das0980/image.png/m/800x0"
/>
stijns96 commented 3 months ago

Meanwhile I get why. There is no breakpoint to match, but still this should work. I like the way the srcset is build up from the sizes, but sometimes it's works against it.

Sometimes it would he better to just have an array of widths that the srcset should be build of.

Most of the time you only need a px size if the container reaches it's max width. Everything below should use vw

dbismut commented 2 months ago

It's actually really weird and it's very strange that this issue doesn't have more upvotes as it basically affects all providers.

The problem lies in that logic: https://github.com/nuxt/image/blob/3175790db095c9a46535cdb0ae9cbb18e0b48bc2/src/runtime/utils/index.ts#L128-L136

For a size input such as "sm: 100vw lg:480px" this function should return a sizes object such as:

sizes = {
  lg: '480px',
  sm: '100vw',
}

But whenever a size is provided without a media query, this function sets it to 1px in size. So: "100vw lg:480px" will return:

sizes = {
  '1px': '100vw',
  lg: '480px',
}

Then further there is some logic that converts the keys to their actual screens values, so the object will be transformed with actual pixel values instead of screen keys, let's says like this:

sizes = {
  '1px': '100vw',
  1280: '480px',
}

And finally with default densities being [1, 2] the sizes object will multiply its keys by x2 hence the 1w and 2w that are so weird. Then it seems also that the package doesn't generate all the srcsets matching the screens defined in nuxt config.

Anyway long story short:

This is a major inconvenience and make this package pretty much useless without a fix.

stijns96 commented 2 months ago

Hi @dbismut ,

Nice explenation! I already moved away from this package... it indeed makes it useless now.

dbismut commented 2 months ago

Do you have a drop in replacement?

stijns96 commented 2 months ago

Do you have a drop in replacement?

Well, I created my own image component. It's fully based on the Storyblok image service

iBobik commented 1 month ago

Do we really need to specify 100vw on the start? Isn’t enough this? xs:100vw sm:100vw md:100vw lg:100vw xl:100vw 2xl:100vw

(Related: #267 )

dbismut commented 1 month ago

Of course this can work but it is fastidious, it's like setting media queries for every breakpoint for each responsive class, that doesn't make sense.

Plus it is supposed to work from the docs.

Also nuxt/image is confusing breakpoints and screen variants which are two different things.