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

Warn when legacy prop detected on next/image #42102

Merged
merged 7 commits into from Oct 30, 2022
Merged
Show file tree
Hide file tree
Changes from 5 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
81 changes: 65 additions & 16 deletions packages/next/client/image.tsx
Expand Up @@ -108,6 +108,27 @@ export type ImageProps = Omit<
blurDataURL?: string
unoptimized?: boolean
onLoadingComplete?: OnLoadingComplete
/**
* @deprecated Use `fill` prop instead of `layout="fill"` or change import to `next/legacy/image`.
* @see https://nextjs.org/docs/api-reference/next/legacy/image
*/
layout?: string
/**
* @deprecated Use `style` prop instead.
*/
objectFit?: string
/**
* @deprecated Use `style` prop instead.
*/
objectPosition?: string
/**
* @deprecated This prop does not do anything.
*/
lazyBoundary?: string
/**
* @deprecated This prop does not do anything.
*/
lazyRoot?: string
}

type ImageElementProps = Omit<ImageProps, 'src' | 'alt' | 'loader'> & {
Expand Down Expand Up @@ -467,6 +488,11 @@ export default function Image({
onLoadingComplete,
placeholder = 'empty',
blurDataURL,
layout,
objectFit,
objectPosition,
lazyBoundary,
lazyRoot,
...all
}: ImageProps) {
const configContext = useContext(ImageConfigContext)
Expand All @@ -479,7 +505,6 @@ export default function Image({

let rest: Partial<ImageProps> = all
let loader: ImageLoaderWithConfig = rest.loader || defaultLoader

// Remove property so it's not spread on <img> element
delete rest.loader

Expand All @@ -503,6 +528,28 @@ export default function Image({
}
}

if (layout) {
if (layout === 'fill') {
fill = true
}
const layoutToStyle: Record<string, Record<string, string> | undefined> = {
intrinsic: { maxWidth: '100%', height: 'auto' },
responsive: { width: '100%', height: 'auto' },
}
const layoutToSizes: Record<string, string | undefined> = {
responsive: '100vw',
fill: '100vw',
}
const layoutStyle = layoutToStyle[layout]
if (layoutStyle) {
style = { ...style, ...layoutStyle }
}
const layoutSizes = layoutToSizes[layout]
if (layoutSizes && !sizes) {
sizes = layoutSizes
}
}

let staticSrc = ''
let widthInt = getInt(width)
let heightInt = getInt(height)
Expand Down Expand Up @@ -546,21 +593,6 @@ export default function Image({
}
src = typeof src === 'string' ? src : staticSrc

for (const legacyProp of [
'layout',
'objectFit',
'objectPosition',
'lazyBoundary',
'lazyRoot',
]) {
if (legacyProp in rest) {
throw new Error(
`Image with src "${src}" has legacy prop "${legacyProp}". Did you forget to run the codemod?` +
`\nRead more: https://nextjs.org/docs/messages/next-image-upgrade-to-13`
)
}
}

let isLazy =
!priority && (loading === 'lazy' || typeof loading === 'undefined')
if (src.startsWith('data:') || src.startsWith('blob:')) {
Expand Down Expand Up @@ -691,6 +723,21 @@ export default function Image({
}
}

for (const [legacyKey, legacyValue] of Object.entries({
layout,
objectFit,
objectPosition,
lazyBoundary,
lazyRoot,
})) {
if (legacyValue) {
warnOnce(
`Image with src "${src}" has legacy prop "${legacyKey}". Did you forget to run the codemod?` +
`\nRead more: https://nextjs.org/docs/messages/next-image-upgrade-to-13`
)
}
}

if (
typeof window !== 'undefined' &&
!perfObserver &&
Expand Down Expand Up @@ -737,6 +784,8 @@ export default function Image({
top: 0,
right: 0,
bottom: 0,
objectFit,
objectPosition,
}
: {},
showAltText ? {} : { color: 'transparent' },
Expand Down
@@ -0,0 +1,26 @@
import Image from 'next/image'

export default function Page() {
return (
<div className="page">
<h1>Using legacy prop layout="fill"</h1>
<p>
Even though we don't support "layout" in next/image, we can try to
correct the style and print a warning.
</p>
<div style={{ position: 'relative', width: '200px', height: '400px' }}>
<Image
id="img"
layout="fill"
src="/test.jpg"
alt="my fill image"
quality={50}
objectFit="cover"
objectPosition="10% 10%"
sizes="200px"
priority
/>
</div>
</div>
)
}
@@ -0,0 +1,22 @@
import Image from 'next/image'

export default function Page() {
return (
<div className="page">
<h1>Using legacy prop layout="responsive"</h1>
<p>
Even though we don't support "layout" in next/image, we can try to
correct the style and print a warning.
</p>
<Image
id="img"
layout="responsive"
src="/test.png"
width="100"
height="100"
alt="my responsive image"
priority
/>
</div>
)
}
58 changes: 58 additions & 0 deletions test/integration/next-image-new/default/test/index.test.ts
Expand Up @@ -669,6 +669,64 @@ function runTests(mode) {
).toBe('color:transparent')
})

it('should warn when legacy prop layout=fill', async () => {
let browser = await webdriver(appPort, '/legacy-layout-fill')
const img = await browser.elementById('img')
expect(img).toBeDefined()
expect(await img.getAttribute('data-nimg')).toBe('fill')
expect(await img.getAttribute('sizes')).toBe('200px')
expect(await img.getAttribute('src')).toBe(
'/_next/image?url=%2Ftest.jpg&w=3840&q=50'
)
expect(await img.getAttribute('srcset')).toContain(
'/_next/image?url=%2Ftest.jpg&w=640&q=50 640w,'
)
expect(await img.getAttribute('style')).toBe(
'position:absolute;height:100%;width:100%;left:0;top:0;right:0;bottom:0;object-fit:cover;object-position:10% 10%;color:transparent'
)
if (mode === 'dev') {
expect(await hasRedbox(browser)).toBe(false)
const warnings = (await browser.log())
.map((log) => log.message)
.join('\n')
expect(warnings).toContain(
'Image with src "/test.jpg" has legacy prop "layout". Did you forget to run the codemod?'
)
expect(warnings).toContain(
'Image with src "/test.jpg" has legacy prop "objectFit". Did you forget to run the codemod?'
)
expect(warnings).toContain(
'Image with src "/test.jpg" has legacy prop "objectPosition". Did you forget to run the codemod?'
)
}
})

it('should warn when legacy prop layout=responsive', async () => {
let browser = await webdriver(appPort, '/legacy-layout-responsive')
const img = await browser.elementById('img')
expect(img).toBeDefined()
expect(await img.getAttribute('sizes')).toBe('100vw')
expect(await img.getAttribute('data-nimg')).toBe('1')
expect(await img.getAttribute('src')).toBe(
'/_next/image?url=%2Ftest.png&w=3840&q=75'
)
expect(await img.getAttribute('srcset')).toContain(
'/_next/image?url=%2Ftest.png&w=640&q=75 640w,'
)
expect(await img.getAttribute('style')).toBe(
'color:transparent;width:100%;height:auto'
)
if (mode === 'dev') {
expect(await hasRedbox(browser)).toBe(false)
const warnings = (await browser.log())
.map((log) => log.message)
.join('\n')
expect(warnings).toContain(
'Image with src "/test.png" has legacy prop "layout". Did you forget to run the codemod?'
)
}
})

if (mode === 'dev') {
it('should show missing src error', async () => {
const browser = await webdriver(appPort, '/missing-src')
Expand Down
14 changes: 0 additions & 14 deletions test/integration/next-image-new/invalid-layout/pages/index.js

This file was deleted.

Binary file not shown.

This file was deleted.

This file was deleted.

Binary file not shown.