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

Add warning when parent styles break next/image #28221

Merged
merged 3 commits into from Aug 18, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
15 changes: 14 additions & 1 deletion packages/next/client/image.tsx
Expand Up @@ -245,6 +245,7 @@ function defaultImageLoader(loaderProps: ImageLoaderProps) {
function handleLoading(
img: HTMLImageElement | null,
src: string,
layout: LayoutValue,
placeholder: PlaceholderValue,
onLoadingComplete?: OnLoadingComplete
) {
Expand All @@ -267,6 +268,18 @@ function handleLoading(
// underlying DOM element because it could be misused.
onLoadingComplete({ naturalWidth, naturalHeight })
}
if (process.env.NODE_ENV !== 'production') {
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' && parent?.position !== 'relative') {
console.warn(
`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.`
)
}
}
})
}
}
Expand Down Expand Up @@ -612,7 +625,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 }}
/>
Expand Down
@@ -0,0 +1,13 @@
import React from 'react'
import Image from 'next/image'
import img from '../public/test.jpg'

const Page = () => {
return (
<div style={{ position: 'static' }}>
<Image id="img" layout="fill" src={img} />
</div>
)
}

export default Page
@@ -0,0 +1,13 @@
import React from 'react'
import Image from 'next/image'
import img from '../public/test.jpg'

const Page = () => {
return (
<div style={{ display: 'flex' }}>
<Image id="img" layout="responsive" src={img} />
</div>
)
}

export default Page
27 changes: 27 additions & 0 deletions test/integration/image-component/default/test/index.test.js
Expand Up @@ -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')
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 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'
)
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
)
})

it('should warn when using a very small image with placeholder=blur', async () => {
const browser = await webdriver(appPort, '/small-img-import')

Expand Down