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

next/image gives false warning for sizes: 100vw #58586

Closed
1 task done
cibulka opened this issue Nov 17, 2023 · 6 comments · Fixed by #61949
Closed
1 task done

next/image gives false warning for sizes: 100vw #58586

cibulka opened this issue Nov 17, 2023 · 6 comments · Fixed by #61949
Assignees
Labels
bug Issue was opened via the bug report template. Image (next/image) Related to Next.js Image Optimization. linear: next Confirmed issue that is tracked by the Next.js team. locked

Comments

@cibulka
Copy link

cibulka commented Nov 17, 2023

Link to the code that reproduces this issue

https://codesandbox.io/p/sandbox/unruffled-haze-nnxhyk?file=%2Fapp%2Fpage.tsx%3A4%2C28

To Reproduce

  1. Start the application in dev mode (npm run dev)
  2. Include the Next/Image like this:
<Image 
      alt="Photo" 
      src="https://images.unsplash.com/photo-1682686581484-a220483e6291?q=80&w=2970&auto=format&fit=crop&ixlib=rb-4.0.3&ixid=M3wxMjA3fDF8MHxwaG90by1wYWdlfHx8fGVufDB8fHx8fA%3D%3D" 
      fill 
      sizes="100vw" 
      style={{ objectFit: 'cover' }}
/>
  1. Receive the following warning: Image with src "https://images.unsplash.com/photo-1682686581484-a220483e6291?q=80&w=2970&auto=format&fit=crop&ixlib=rb-4.0.3&ixid=M3wxMjA3fDF8MHxwaG90by1wYWdlfHx8fGVufDB8fHx8fA%3D%3D" has "fill" but is missing "sizes" prop. Please add it to improve page performance.

Current vs. Expected behavior

Current behaviour

Next/Image with sizes="100vw" and fill! gives the following warning: Image with src "https://placehold.co/600x400" has "fill" but is missing "sizes" prop. Please add it to improve page performance.`

Epx

Having the attribute sizes marked as 100vw is a valid use-case for full-page images (such as backgrounds). According to developer.mozilla.org there is no other way how to express 100% of viewport for this attribute than 100vw.

Note: The source size value must not be specified as a percentage of the container size; that is, lengths such as 50% or 100% are not allowed, as there would be uncertainty as to what the specified value is a percentage of.

Verify canary release

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

Provide environment information

Operating System:
  Platform: darwin
  Arch: arm64
  Version: Darwin Kernel Version 23.0.0: Fri Sep 15 14:41:43 PDT 2023; root:xnu-10002.1.13~1/RELEASE_ARM64_T6000
Binaries:
  Node: 20.9.0
  npm: 10.1.0
  Yarn: 1.22.19
  pnpm: 8.10.2
Relevant Packages:
  next: 14.0.3
  eslint-config-next: 14.0.3
  react: 18.2.0
  react-dom: 18.2.0
  typescript: 5.2.2
Next.js Config:
  output: N/A

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

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

Additional context

In a Next.js dicusssion there are some solutions proposed that remove the error, such as using values as 100% or auto. They remove the warning, but none of them is a valid usage of sizes attribute.

NEXT-2441

@cibulka cibulka added the bug Issue was opened via the bug report template. label Nov 17, 2023
@github-actions github-actions bot added the Image (next/image) Related to Next.js Image Optimization. label Nov 17, 2023
@styfle styfle added the please add a complete reproduction Please add a complete reproduction. label Jan 29, 2024
Copy link
Contributor

We cannot recreate the issue with the provided information. Please add a reproduction in order for us to be able to investigate.

Why was this issue marked with the please add a complete reproduction label?

To be able to investigate, we need access to a reproduction to identify what triggered the issue. We prefer a link to a public GitHub repository (template for App Router, template for Pages Router), but you can also use these templates: CodeSandbox: App Router or CodeSandbox: Pages Router.

To make sure the issue is resolved as quickly as possible, please make sure that the reproduction is as minimal as possible. This means that you should remove unnecessary code, files, and dependencies that do not contribute to the issue. Ensure your reproduction does not depend on secrets, 3rd party registries, private dependencies, or any other data that cannot be made public. Avoid a reproduction including a whole monorepo (unless relevant to the issue). The easier it is to reproduce the issue, the quicker we can help.

Please test your reproduction against the latest version of Next.js (next@canary) to make sure your issue has not already been fixed.

If you cannot create a clean reproduction, another way you can help the maintainers' job is to pinpoint the canary version of next that introduced the issue. Check out our releases, and try to find the first canary release that introduced the issue. This will help us narrow down the scope of the issue, and possibly point to the PR/code change that introduced it. You can install a specific version of next by running npm install next@<version>.

I added a link, why was it still marked?

Ensure the link is pointing to a codebase that is accessible (e.g. not a private repository). "example.com", "n/a", "will add later", etc. are not acceptable links -- we need to see a public codebase. See the above section for accepted links.

What happens if I don't provide a sufficient minimal reproduction?

Issues with the please add a complete reproduction label that receives no meaningful activity (e.g. new comments with a reproduction link) are automatically closed and locked after 30 days.

If your issue has not been resolved in that time and it has been closed/locked, please open a new issue with the required reproduction.

I did not open this issue, but it is relevant to me, what can I do to help?

Anyone experiencing the same issue is welcome to provide a minimal reproduction following the above steps. Furthermore, you can upvote the issue using the 👍 reaction on the topmost comment (please do not comment "I have the same issue" without reproduction steps). Then, we can sort issues by votes to prioritize.

I think my reproduction is good enough, why aren't you looking into it quicker?

We look into every Next.js issue and constantly monitor open issues for new comments.

However, sometimes we might miss one or two due to the popularity/high traffic of the repository. We apologize, and kindly ask you to refrain from tagging core maintainers, as that will usually not result in increased priority.

Upvoting issues to show your interest will help us prioritize and address them as quickly as possible. That said, every issue is important to us, and if an issue gets closed by accident, we encourage you to open a new one linking to the old issue and we will look into it.

Useful Resources

@styfle
Copy link
Member

styfle commented Jan 29, 2024

I visited the link and can't reproduce your issue.

image

@styfle
Copy link
Member

styfle commented Jan 29, 2024

Having the attribute sizes marked as 100vw is a valid use-case for full-page images (such as backgrounds).

Please note that the warning is only printed when the image is rendered less than 60vw, meaning 100vw is too large and should be adjusted.

if (
!unoptimized &&
(!img.getAttribute('sizes') || img.getAttribute('sizes') === '100vw')
) {
let widthViewportRatio =
img.getBoundingClientRect().width / window.innerWidth
if (widthViewportRatio < 0.6) {
warnOnce(
`Image with src "${origSrc}" has "fill" but is missing "sizes" prop. Please add it to improve page performance. Read more: https://nextjs.org/docs/api-reference/next/image#sizes`
)
}
}

@styfle styfle closed this as not planned Won't fix, can't repro, duplicate, stale Feb 12, 2024
@cibulka
Copy link
Author

cibulka commented Feb 12, 2024

Having the attribute sizes marked as 100vw is a valid use-case for full-page images (such as backgrounds).

Please note that the warning is only printed when the image is rendered less than 60vw, meaning 100vw is too large and should be adjusted.

if (
!unoptimized &&
(!img.getAttribute('sizes') || img.getAttribute('sizes') === '100vw')
) {
let widthViewportRatio =
img.getBoundingClientRect().width / window.innerWidth
if (widthViewportRatio < 0.6) {
warnOnce(
`Image with src "${origSrc}" has "fill" but is missing "sizes" prop. Please add it to improve page performance. Read more: https://nextjs.org/docs/api-reference/next/image#sizes`
)
}
}

I see! In that case I believe the warning is misleading. The image is not missing the sizes prop, it just does not have appropriate value. Maybe it would make sense to adjust the error message?

@styfle styfle reopened this Feb 12, 2024
@styfle styfle added linear: next Confirmed issue that is tracked by the Next.js team. and removed please add a complete reproduction Please add a complete reproduction. labels Feb 12, 2024
@styfle styfle self-assigned this Feb 12, 2024
@styfle
Copy link
Member

styfle commented Feb 12, 2024

Good point! Fixing in PR #61949

Copy link
Contributor

This closed issue has been automatically locked because it had no new activity for 2 weeks. If you are running into a similar issue, please create a new issue with the steps to reproduce. Thank you.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 27, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Issue was opened via the bug report template. Image (next/image) Related to Next.js Image Optimization. linear: next Confirmed issue that is tracked by the Next.js team. locked
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants