Skip to content
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

Image source set is incorrectly trimmed #41516

Open
1 task done
quarhodron opened this issue Oct 18, 2022 · 2 comments
Open
1 task done

Image source set is incorrectly trimmed #41516

quarhodron opened this issue Oct 18, 2022 · 2 comments
Labels
bug Issue was opened via the bug report template. Image (next/image) Related to Next.js Image Optimization. stale The issue has not seen recent activity.

Comments

@quarhodron
Copy link

quarhodron commented Oct 18, 2022

Verify canary release

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

Provide environment information

Codesandbox env:

   Operating System:
      Platform: linux
      Arch: x64
      Version: #45~20.04.1-Ubuntu SMP Mon Apr 4 09:38:31 UTC 2022
    Binaries:
      Node: 14.19.3
      npm: 6.14.17
      Yarn: 1.22.19
      pnpm: N/A
    Relevant packages:
      next: 12.3.2-canary.29
      eslint-config-next: N/A
      react: 18.2.0
      react-dom: 18.2.0

My local env where the problem is the same as in Codesandbox:

    Operating System:
      Platform: darwin
      Arch: x64
      Version: Darwin Kernel Version 19.6.0: Tue Jun 21 21:18:39 PDT 2022; root:xnu-6153.141.66~1/RELEASE_X86_64
    Binaries:
      Node: 16.14.2
      npm: 8.5.0
      Yarn: N/A
      pnpm: N/A
    Relevant packages:
      next: 12.2.5
      eslint-config-next: 12.2.5
      react: 17.0.2
      react-dom: 17.0.2

What browser are you using? (if relevant)

No response

How are you deploying your application? (if relevant)

No response

Describe the Bug

My next.config.js for images is as the following:

images: {
  deviceSizes: [800, 1600, 2000, 4000],
  imageSizes: [200, 400]
}

The sizes property of next/image is sizes="(min-width: 1010px) 250px, 100vw".

Result: The source set includes only device sizes, so [800, 1600, 2000, 4000] since there is 100vw.

It's because the docs state:

If the sizes property includes sizes such as 50vw, which represent a percentage of the viewport width, then the source set is trimmed to not include any values which are too small to ever be necessary.

However, in sizes there is also 250px and apparently, it's not taken into account. If it was, also 400 would be included in image srcset.

Expected Behavior

Not only vw should be taken into account when trimming a source set but also other units. In this example, only 200 should be omitted but 400 should be included.

Link to reproduction

https://codesandbox.io/s/gifted-wu-59ou5o?file=/pages/index.tsx

To Reproduce

Just inspect Elements in Devtools. In my case it looks:

<img 
alt="" 
sizes="(min-width: 1010px) 250px, 100vw" 
srcset="https://cdn-yams.new.godt.no/api/v1/godt-recipe/images/a6/a675016e-31a2-40e7-b320-1ea87b5f80c2?rule=w800_auto 800w, 
https://cdn-yams.new.godt.no/api/v1/godt-recipe/images/a6/a675016e-31a2-40e7-b320-1ea87b5f80c2?rule=w1600_auto 1600w, 
https://cdn-yams.new.godt.no/api/v1/godt-recipe/images/a6/a675016e-31a2-40e7-b320-1ea87b5f80c2?rule=w2000_auto 2000w, 
https://cdn-yams.new.godt.no/api/v1/godt-recipe/images/a6/a675016e-31a2-40e7-b320-1ea87b5f80c2?rule=w4000_auto 4000w" 
src="https://cdn-yams.new.godt.no/api/v1/godt-recipe/images/a6/a675016e-31a2-40e7-b320-1ea87b5f80c2?rule=w4000_auto" 
width="1280" height="720" decoding="async" data-nimg="future" loading="lazy" style="color:transparent" class="Image_image__EqICW"
>
@quarhodron quarhodron added the bug Issue was opened via the bug report template. label Oct 18, 2022
@balazsorban44 balazsorban44 added the Image (next/image) Related to Next.js Image Optimization. label Oct 18, 2022
@Laityned
Copy link
Contributor

Laityned commented Oct 21, 2022

I made a draft PR for a possible solution to fix it, but this does not include the solution for the complete problem with the trimming function at this moment:

I think the problem consists of two parts:

  • trimming does happen based on the smallest device size multiplied with the smallest viewport but this not take the media queries into account -> the srcset is not completely trimmed
  • the image sizes are in pixels, but the media queries with accessory size could be in a (nonconvertible) unit, such as a non-default value for EM.

A solution would be to not trim the sources at all, or only trim the sources if all values are in vws

@vercel-release-bot
Copy link
Collaborator

This issue has been automatically marked as stale due to two years of inactivity. It will be closed in 7 days unless there’s further input. If you believe this issue is still relevant, please leave a comment or provide updated details. Thank you.

@vercel-release-bot vercel-release-bot added the stale The issue has not seen recent activity. label Jan 11, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Issue was opened via the bug report template. Image (next/image) Related to Next.js Image Optimization. stale The issue has not seen recent activity.
Projects
None yet
Development

No branches or pull requests

4 participants