From 96beaaf7b317757cc125e73e763bc0f1ec1f984a Mon Sep 17 00:00:00 2001
From: Steven <steven@ceriously.com>
Date: Mon, 12 Feb 2024 11:19:13 -0500
Subject: [PATCH] fix(next/image): improve warning when `fill` and
 `sizes="100vw"`

---
 packages/next/src/client/image-component.tsx  | 30 ++++++++++++-------
 .../app-dir/app/fill-warnings/page.js         |  6 ++++
 .../next-image-new/app-dir/test/index.test.ts |  3 ++
 .../default/pages/fill-warnings.js            |  6 ++++
 .../next-image-new/default/test/index.test.ts |  3 ++
 5 files changed, 38 insertions(+), 10 deletions(-)

diff --git a/packages/next/src/client/image-component.tsx b/packages/next/src/client/image-component.tsx
index 0d9c80c12adaf..80e8aea263e86 100644
--- a/packages/next/src/client/image-component.tsx
+++ b/packages/next/src/client/image-component.tsx
@@ -53,6 +53,7 @@ type ImageElementProps = ImgProps & {
   onLoadingCompleteRef: React.MutableRefObject<OnLoadingComplete | undefined>
   setBlurComplete: (b: boolean) => void
   setShowAltText: (b: boolean) => void
+  sizesInput: string | undefined
 }
 
 // See https://stackoverflow.com/q/39777833/266535 for why we use this ref
@@ -63,7 +64,8 @@ function handleLoading(
   onLoadRef: React.MutableRefObject<OnLoad | undefined>,
   onLoadingCompleteRef: React.MutableRefObject<OnLoadingComplete | undefined>,
   setBlurComplete: (b: boolean) => void,
-  unoptimized: boolean
+  unoptimized: boolean,
+  sizesInput: string | undefined
 ) {
   const src = img?.src
   if (!img || img['data-loaded-src'] === src) {
@@ -115,16 +117,19 @@ function handleLoading(
     if (process.env.NODE_ENV !== 'production') {
       const origSrc = new URL(src, 'http://n').searchParams.get('url') || src
       if (img.getAttribute('data-nimg') === 'fill') {
-        if (
-          !unoptimized &&
-          (!img.getAttribute('sizes') || img.getAttribute('sizes') === '100vw')
-        ) {
+        if (!unoptimized && (!sizesInput || sizesInput === '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`
-            )
+            if (sizesInput === '100vw') {
+              warnOnce(
+                `Image with src "${origSrc}" has "fill" prop and "sizes" prop of "100vw", but image is not rendered at full viewport width. Please adjust "sizes" to improve page performance. Read more: https://nextjs.org/docs/api-reference/next/image#sizes`
+              )
+            } else {
+              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`
+              )
+            }
           }
         }
         if (img.parentElement) {
@@ -197,6 +202,7 @@ const ImageElement = forwardRef<HTMLImageElement | null, ImageElementProps>(
       onLoadingCompleteRef,
       setBlurComplete,
       setShowAltText,
+      sizesInput,
       onLoad,
       onError,
       ...rest
@@ -262,7 +268,8 @@ const ImageElement = forwardRef<HTMLImageElement | null, ImageElementProps>(
                 onLoadRef,
                 onLoadingCompleteRef,
                 setBlurComplete,
-                unoptimized
+                unoptimized,
+                sizesInput
               )
             }
           },
@@ -274,6 +281,7 @@ const ImageElement = forwardRef<HTMLImageElement | null, ImageElementProps>(
             setBlurComplete,
             onError,
             unoptimized,
+            sizesInput,
             forwardedRef,
           ]
         )}
@@ -285,7 +293,8 @@ const ImageElement = forwardRef<HTMLImageElement | null, ImageElementProps>(
             onLoadRef,
             onLoadingCompleteRef,
             setBlurComplete,
-            unoptimized
+            unoptimized,
+            sizesInput
           )
         }}
         onError={(event) => {
@@ -406,6 +415,7 @@ export const Image = forwardRef<HTMLImageElement | null, ImageProps>(
             onLoadingCompleteRef={onLoadingCompleteRef}
             setBlurComplete={setBlurComplete}
             setShowAltText={setShowAltText}
+            sizesInput={props.sizes}
             ref={forwardedRef}
           />
         }
diff --git a/test/integration/next-image-new/app-dir/app/fill-warnings/page.js b/test/integration/next-image-new/app-dir/app/fill-warnings/page.js
index e24ef4af21209..625cb8f5cbe6a 100644
--- a/test/integration/next-image-new/app-dir/app/fill-warnings/page.js
+++ b/test/integration/next-image-new/app-dir/app/fill-warnings/page.js
@@ -28,6 +28,12 @@ const Page = () => {
       >
         <Image id="fill-image-3" src="/wide.png" fill />
       </div>
+      <div
+        id="image-container-4"
+        style={{ position: 'relative', width: '30vw', height: '30vw' }}
+      >
+        <Image id="fill-image-4" src="/test.png" sizes="100vw" fill />
+      </div>
     </div>
   )
 }
diff --git a/test/integration/next-image-new/app-dir/test/index.test.ts b/test/integration/next-image-new/app-dir/test/index.test.ts
index ca63f206a9333..037823fd10c7e 100644
--- a/test/integration/next-image-new/app-dir/test/index.test.ts
+++ b/test/integration/next-image-new/app-dir/test/index.test.ts
@@ -1318,6 +1318,9 @@ function runTests(mode) {
         expect(warnings).toContain(
           'Image with src "/wide.png" has "fill" but is missing "sizes" prop. Please add it to improve page performance. Read more:'
         )
+        expect(warnings).toContain(
+          'Image with src "/test.png" has "fill" prop and "sizes" prop of "100vw", but image is not rendered at full viewport width. Please adjust "sizes" to improve page performance. Read more:'
+        )
       })
       it('should not log warnings when image unmounts', async () => {
         browser = await webdriver(appPort, '/should-not-warn-unmount')
diff --git a/test/integration/next-image-new/default/pages/fill-warnings.js b/test/integration/next-image-new/default/pages/fill-warnings.js
index e24ef4af21209..625cb8f5cbe6a 100644
--- a/test/integration/next-image-new/default/pages/fill-warnings.js
+++ b/test/integration/next-image-new/default/pages/fill-warnings.js
@@ -28,6 +28,12 @@ const Page = () => {
       >
         <Image id="fill-image-3" src="/wide.png" fill />
       </div>
+      <div
+        id="image-container-4"
+        style={{ position: 'relative', width: '30vw', height: '30vw' }}
+      >
+        <Image id="fill-image-4" src="/test.png" sizes="100vw" fill />
+      </div>
     </div>
   )
 }
diff --git a/test/integration/next-image-new/default/test/index.test.ts b/test/integration/next-image-new/default/test/index.test.ts
index f71a1cd8e658e..0d56881c4ee37 100644
--- a/test/integration/next-image-new/default/test/index.test.ts
+++ b/test/integration/next-image-new/default/test/index.test.ts
@@ -1319,6 +1319,9 @@ function runTests(mode) {
         expect(warnings).toContain(
           'Image with src "/wide.png" has "fill" but is missing "sizes" prop. Please add it to improve page performance. Read more:'
         )
+        expect(warnings).toContain(
+          'Image with src "/test.png" has "fill" prop and "sizes" prop of "100vw", but image is not rendered at full viewport width. Please adjust "sizes" to improve page performance. Read more:'
+        )
       })
       it('should not log warnings when image unmounts', async () => {
         browser = await webdriver(appPort, '/should-not-warn-unmount')