Skip to content

Commit

Permalink
Add warning when parent styles break next/image (#28221)
Browse files Browse the repository at this point in the history
Fixes #27644
  • Loading branch information
styfle committed Aug 18, 2021
1 parent 7ea7c23 commit ce18756
Show file tree
Hide file tree
Showing 4 changed files with 67 additions and 1 deletion.
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') {

This comment has been minimized.

Copy link
@FilipChalupa

FilipChalupa Oct 27, 2021

@styfle What's wrong with parent being any non static position. I set parent to position: absolute; top: 0; left: 0; right: 0; bottom: 0; a lot a this check spams dev console. Official nextjs demo uses position: fixed; and I believe it's legitimate usecase. https://image-component.nextjs.gallery/background

I didn't find any explanation in the issue mentioned in commit message.

Thank you for your answer.

This comment has been minimized.

Copy link
@styfle

styfle Oct 27, 2021

Author Member

Its in the linked issue #27644

If you see a false positive, please submit a PR with a test and we'll take a look, thanks!

This comment has been minimized.

Copy link
@styfle

styfle Oct 27, 2021

Author Member

I see what you mean, its affecting the demo. I'll create a PR in a minute, thanks 👍

This comment has been minimized.

Copy link
@styfle

styfle Oct 27, 2021

Author Member

Fixed in #30453

This comment has been minimized.

Copy link
@FilipChalupa

FilipChalupa Oct 27, 2021

Thank you for your quick response. :)

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

0 comments on commit ce18756

Please sign in to comment.