From 98cfdca6a6391f363672e1bc2c6b1208de2b98dd Mon Sep 17 00:00:00 2001 From: Steven Date: Fri, 18 Feb 2022 15:38:28 -0500 Subject: [PATCH 1/2] Improve `next/image` warnings to avoid printing more than once --- packages/next/client/image.tsx | 29 ++++++++++++------ .../default/pages/warning-once.js | 23 ++++++++++++++ .../default/test/index.test.js | 30 +++++++++++++++++++ 3 files changed, 73 insertions(+), 9 deletions(-) create mode 100644 test/integration/image-component/default/pages/warning-once.js diff --git a/packages/next/client/image.tsx b/packages/next/client/image.tsx index c16b26a05678..0c11ea607eb8 100644 --- a/packages/next/client/image.tsx +++ b/packages/next/client/image.tsx @@ -23,6 +23,17 @@ if (typeof window === 'undefined') { ;(global as any).__NEXT_IMAGE_IMPORTED = true } +let warnOnce = (_: string) => {} +if (process.env.NODE_ENV !== 'production') { + const warnings = new Set() + warnOnce = (msg: string) => { + if (!warnings.has(msg)) { + console.warn(msg) + } + warnings.add(msg) + } +} + const VALID_LOADING_VALUES = ['lazy', 'eager', undefined] as const type LoadingValue = typeof VALID_LOADING_VALUES[number] type ImageConfig = ImageConfigComplete & { allSizes: number[] } @@ -274,7 +285,7 @@ function handleLoading( if (!parent.position) { // The parent has not been rendered to the dom yet and therefore it has no position. Skip the warnings for such cases. } else if (layout === 'responsive' && parent.display === 'flex') { - console.warn( + warnOnce( `Image with src "${src}" may not render properly as a child of a flex container. Consider wrapping the image with a div to configure the width.` ) } else if ( @@ -283,7 +294,7 @@ function handleLoading( parent.position !== 'fixed' && parent.position !== 'absolute' ) { - console.warn( + warnOnce( `Image with src "${src}" may not render properly with a parent using position:"${parent.position}". Consider changing the parent style to position:"relative" with a width and height.` ) } @@ -410,7 +421,7 @@ export default function Image({ ) } if (layout === 'fill' && (width || height)) { - console.warn( + warnOnce( `Image with src "${src}" and "layout='fill'" has unused properties assigned. Please remove "width" and "height".` ) } @@ -427,13 +438,13 @@ export default function Image({ ) } if (sizes && layout !== 'fill' && layout !== 'responsive') { - console.warn( + warnOnce( `Image with src "${src}" has "sizes" property but it will be ignored. Only use "sizes" with "layout='fill'" or "layout='responsive'".` ) } if (placeholder === 'blur') { if (layout !== 'fill' && (widthInt || 0) * (heightInt || 0) < 1600) { - console.warn( + warnOnce( `Image with src "${src}" is smaller than 40x40. Consider removing the "placeholder='blur'" property to improve performance.` ) } @@ -453,12 +464,12 @@ export default function Image({ } } if ('ref' in rest) { - console.warn( + warnOnce( `Image with src "${src}" is using unsupported "ref" property. Consider using the "onLoadingComplete" property instead.` ) } if ('style' in rest) { - console.warn( + warnOnce( `Image with src "${src}" is using unsupported "style" property. Please use the "className" property instead.` ) } @@ -475,7 +486,7 @@ export default function Image({ url = new URL(urlStr) } catch (err) {} if (urlStr === src || (url && url.pathname === src && !url.search)) { - console.warn( + warnOnce( `Image with src "${src}" has a "loader" property that does not implement width. Please implement it or use the "unoptimized" property instead.` + `\nRead more: https://nextjs.org/docs/messages/next-image-missing-loader-width` ) @@ -500,7 +511,7 @@ export default function Image({ !lcpImage.src.startsWith('blob:') ) { // https://web.dev/lcp/#measure-lcp-in-javascript - console.warn( + warnOnce( `Image with src "${lcpImage.src}" was detected as the Largest Contentful Paint (LCP). Please add the "priority" property if this image is above the fold.` + `\nRead more: https://nextjs.org/docs/api-reference/next/image#priority` ) diff --git a/test/integration/image-component/default/pages/warning-once.js b/test/integration/image-component/default/pages/warning-once.js new file mode 100644 index 000000000000..ed1c47b077d9 --- /dev/null +++ b/test/integration/image-component/default/pages/warning-once.js @@ -0,0 +1,23 @@ +import React from 'react' +import Image from 'next/image' + +const Page = () => { + const [count, setCount] = React.useState(0) + return ( + <> +

Warning should print at most once

+ + +
footer here
+ + ) +} + +export default Page diff --git a/test/integration/image-component/default/test/index.test.js b/test/integration/image-component/default/test/index.test.js index 2821fe553d6c..cb83bacb4d09 100644 --- a/test/integration/image-component/default/test/index.test.js +++ b/test/integration/image-component/default/test/index.test.js @@ -812,6 +812,36 @@ function runTests(mode) { await browser.elementById('without-loader').getAttribute('srcset') ).toBe('/test.svg 1x, /test.svg 2x') }) + + it('should warn at most once even after state change', async () => { + const browser = await webdriver(appPort, '/warning-once') + await browser.eval(`document.querySelector("footer").scrollIntoView()`) + await browser.eval(`document.querySelector("button").click()`) + await browser.eval(`document.querySelector("button").click()`) + const count = await browser.eval( + `document.querySelector("button").textContent` + ) + expect(count).toBe('Count: 2') + await check(async () => { + const result = await browser.eval( + 'document.getElementById("w").naturalWidth' + ) + if (result < 1) { + throw new Error('Image not loaded') + } + return 'done' + }, 'done') + const warnings = (await browser.log('browser')) + .map((log) => log.message) + .filter((log) => log !== 'Next.js page already hydrated') + expect(warnings[0]).toMatch( + 'Image with src "/test.png" has "sizes" property but it will be ignored.' + ) + expect(warnings[1]).toMatch( + 'Image with src "/test.png" was detected as the Largest Contentful Paint (LCP).' + ) + expect(warnings.length).toBe(2) + }) } else { //server-only tests it('should not create an image folder in server/chunks', async () => { From 25e18b9afe213ff6849a5e11a35e1102344f176d Mon Sep 17 00:00:00 2001 From: Steven Date: Fri, 18 Feb 2022 16:11:56 -0500 Subject: [PATCH 2/2] Filter log warnings for image only --- test/integration/image-component/default/test/index.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/integration/image-component/default/test/index.test.js b/test/integration/image-component/default/test/index.test.js index cb83bacb4d09..96d1041b6b59 100644 --- a/test/integration/image-component/default/test/index.test.js +++ b/test/integration/image-component/default/test/index.test.js @@ -833,7 +833,7 @@ function runTests(mode) { }, 'done') const warnings = (await browser.log('browser')) .map((log) => log.message) - .filter((log) => log !== 'Next.js page already hydrated') + .filter((log) => log.startsWith('Image with src')) expect(warnings[0]).toMatch( 'Image with src "/test.png" has "sizes" property but it will be ignored.' )