From 76ed6cd396bad242c37b9b63188aea65c62e741c Mon Sep 17 00:00:00 2001 From: Steven Date: Tue, 17 Aug 2021 18:59:51 -0400 Subject: [PATCH 1/2] Add warning when parent styles will break `next/image` --- packages/next/client/image.tsx | 18 ++++++++++++- .../pages/layout-fill-inside-nonrelative.js | 13 +++++++++ .../pages/layout-responsive-inside-flex.js | 13 +++++++++ .../default/test/index.test.js | 27 +++++++++++++++++++ 4 files changed, 70 insertions(+), 1 deletion(-) create mode 100644 test/integration/image-component/default/pages/layout-fill-inside-nonrelative.js create mode 100644 test/integration/image-component/default/pages/layout-responsive-inside-flex.js diff --git a/packages/next/client/image.tsx b/packages/next/client/image.tsx index 78e263d0aa5bb2d..a5bcceae4e63669 100644 --- a/packages/next/client/image.tsx +++ b/packages/next/client/image.tsx @@ -245,6 +245,7 @@ function defaultImageLoader(loaderProps: ImageLoaderProps) { function handleLoading( img: HTMLImageElement | null, src: string, + layout: LayoutValue, placeholder: PlaceholderValue, onLoadingComplete?: OnLoadingComplete ) { @@ -267,6 +268,21 @@ function handleLoading( // underlying DOM element because it could be misused. onLoadingComplete({ naturalWidth, naturalHeight }) } + if (process.env.NODE_ENV !== 'production') { + const parentStyle = img.parentElement?.parentElement?.style + if (layout === 'responsive' && parentStyle?.display === 'flex') { + console.warn( + `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 ( + layout === 'fill' && + parentStyle?.position !== 'relative' + ) { + console.warn( + `Image with src "${src}" may not render properly with a parent using position:"${parentStyle?.position}". Consider changing the parent style to position:"relative" with a width and height.` + ) + } + } }) } } @@ -612,7 +628,7 @@ export default function Image({ className={className} ref={(img) => { setRef(img) - handleLoading(img, srcString, placeholder, onLoadingComplete) + handleLoading(img, srcString, layout, placeholder, onLoadingComplete) }} style={{ ...imgStyle, ...blurStyle }} /> diff --git a/test/integration/image-component/default/pages/layout-fill-inside-nonrelative.js b/test/integration/image-component/default/pages/layout-fill-inside-nonrelative.js new file mode 100644 index 000000000000000..bbcb08668695f79 --- /dev/null +++ b/test/integration/image-component/default/pages/layout-fill-inside-nonrelative.js @@ -0,0 +1,13 @@ +import React from 'react' +import Image from 'next/image' +import img from '../public/test.jpg' + +const Page = () => { + return ( +
+ +
+ ) +} + +export default Page diff --git a/test/integration/image-component/default/pages/layout-responsive-inside-flex.js b/test/integration/image-component/default/pages/layout-responsive-inside-flex.js new file mode 100644 index 000000000000000..d6ab8d32debcbab --- /dev/null +++ b/test/integration/image-component/default/pages/layout-responsive-inside-flex.js @@ -0,0 +1,13 @@ +import React from 'react' +import Image from 'next/image' +import img from '../public/test.jpg' + +const Page = () => { + return ( +
+ +
+ ) +} + +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 aff13d076e6ff2c..6be77fdb6d20db9 100644 --- a/test/integration/image-component/default/test/index.test.js +++ b/test/integration/image-component/default/test/index.test.js @@ -582,6 +582,33 @@ function runTests(mode) { ) }) + it('should warn when img with layout=responsive is inside flex container', async () => { + const browser = await webdriver(appPort, '/layout-responsive-inside-flex') + + const warnings = (await browser.log('browser')) + .map((log) => log.message) + .join('\n') + expect(await hasRedbox(browser)).toBe(false) + expect(warnings).toMatch( + /Image with src (.*)jpg(.*) may not render properly as a child of a flex container. Consider wrapping the image with a div to configure the width/gm + ) + }) + + it('should warn when img with layout=fill is inside a container without position relative', async () => { + const browser = await webdriver( + appPort, + '/layout-fill-inside-nonrelative' + ) + + const warnings = (await browser.log('browser')) + .map((log) => log.message) + .join('\n') + expect(await hasRedbox(browser)).toBe(false) + expect(warnings).toMatch( + /Image with src (.*)jpg(.*) may not render properly with a parent using position:\"static\". Consider changing the parent style to position:\"relative\"/gm + ) + }) + it('should warn when using a very small image with placeholder=blur', async () => { const browser = await webdriver(appPort, '/small-img-import') From 66f94d025fc30e5b977d2c865670ba779cd154ea Mon Sep 17 00:00:00 2001 From: Steven Date: Tue, 17 Aug 2021 19:34:26 -0400 Subject: [PATCH 2/2] Fix tests --- packages/next/client/image.tsx | 11 ++++------- .../default/pages/layout-fill-inside-nonrelative.js | 2 +- .../default/pages/layout-responsive-inside-flex.js | 2 +- .../image-component/default/test/index.test.js | 6 +++--- 4 files changed, 9 insertions(+), 12 deletions(-) diff --git a/packages/next/client/image.tsx b/packages/next/client/image.tsx index a5bcceae4e63669..951d8c60c6d6188 100644 --- a/packages/next/client/image.tsx +++ b/packages/next/client/image.tsx @@ -269,17 +269,14 @@ function handleLoading( onLoadingComplete({ naturalWidth, naturalHeight }) } if (process.env.NODE_ENV !== 'production') { - const parentStyle = img.parentElement?.parentElement?.style - if (layout === 'responsive' && parentStyle?.display === 'flex') { + const parent = img.parentElement?.parentElement?.style + if (layout === 'responsive' && parent?.display === 'flex') { console.warn( `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 ( - layout === 'fill' && - parentStyle?.position !== 'relative' - ) { + } else if (layout === 'fill' && parent?.position !== 'relative') { console.warn( - `Image with src "${src}" may not render properly with a parent using position:"${parentStyle?.position}". Consider changing the parent style to position:"relative" with a width and height.` + `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.` ) } } diff --git a/test/integration/image-component/default/pages/layout-fill-inside-nonrelative.js b/test/integration/image-component/default/pages/layout-fill-inside-nonrelative.js index bbcb08668695f79..b4462487616404c 100644 --- a/test/integration/image-component/default/pages/layout-fill-inside-nonrelative.js +++ b/test/integration/image-component/default/pages/layout-fill-inside-nonrelative.js @@ -5,7 +5,7 @@ import img from '../public/test.jpg' const Page = () => { return (
- +
) } diff --git a/test/integration/image-component/default/pages/layout-responsive-inside-flex.js b/test/integration/image-component/default/pages/layout-responsive-inside-flex.js index d6ab8d32debcbab..923c9810cb1747b 100644 --- a/test/integration/image-component/default/pages/layout-responsive-inside-flex.js +++ b/test/integration/image-component/default/pages/layout-responsive-inside-flex.js @@ -5,7 +5,7 @@ import img from '../public/test.jpg' const Page = () => { return (
- +
) } diff --git a/test/integration/image-component/default/test/index.test.js b/test/integration/image-component/default/test/index.test.js index 6be77fdb6d20db9..555a2f134fe4ce1 100644 --- a/test/integration/image-component/default/test/index.test.js +++ b/test/integration/image-component/default/test/index.test.js @@ -584,7 +584,7 @@ function runTests(mode) { it('should warn when img with layout=responsive is inside flex container', async () => { const browser = await webdriver(appPort, '/layout-responsive-inside-flex') - + await browser.eval(`document.getElementById("img").scrollIntoView()`) const warnings = (await browser.log('browser')) .map((log) => log.message) .join('\n') @@ -599,13 +599,13 @@ function runTests(mode) { appPort, '/layout-fill-inside-nonrelative' ) - + await browser.eval(`document.getElementById("img").scrollIntoView()`) const warnings = (await browser.log('browser')) .map((log) => log.message) .join('\n') expect(await hasRedbox(browser)).toBe(false) expect(warnings).toMatch( - /Image with src (.*)jpg(.*) may not render properly with a parent using position:\"static\". Consider changing the parent style to position:\"relative\"/gm + /Image with src (.*)jpg(.*) may not render properly with a parent using position:\\"static\\". Consider changing the parent style to position:\\"relative\\"/gm ) })