Skip to content

Commit

Permalink
Show error message when using legacy props on new next/image (#41930)
Browse files Browse the repository at this point in the history
This PR shows a better error message when using legacy props on new
`next/image` (introduced in Next.js 13) and makes the codemod more
discoverable.
  • Loading branch information
styfle committed Oct 27, 2022
1 parent 0beed35 commit 191710d
Show file tree
Hide file tree
Showing 17 changed files with 230 additions and 72 deletions.
4 changes: 4 additions & 0 deletions errors/manifest.json
Expand Up @@ -346,6 +346,10 @@
"title": "next-image-unconfigured-host",
"path": "/errors/next-image-unconfigured-host.md"
},
{
"title": "next-image-upgrade-to-13",
"path": "/errors/next-image-upgrade-to-13.md"
},
{
"title": "next-script-for-ga",
"path": "/errors/next-script-for-ga.md"
Expand Down
40 changes: 40 additions & 0 deletions errors/next-image-upgrade-to-13.md
@@ -0,0 +1,40 @@
# `next/image` changed in version 13

#### Why This Error Occurred

Starting in Next.js 13, the `next/image` component has undergone some major changes.

Compared to the legacy component, the new `next/image` component has the following changes:

- Removes `<span>` wrapper around `<img>` in favor of [native computed aspect ratio](https://caniuse.com/mdn-html_elements_img_aspect_ratio_computed_from_attributes)
- Adds support for canonical `style` prop
- Removes `layout` prop in favor of `style` or `className`
- Removes `objectFit` prop in favor of `style` or `className`
- Removes `objectPosition` prop in favor of `style` or `className`
- Removes `IntersectionObserver` implementation in favor of [native lazy loading](https://caniuse.com/loading-lazy-attr)
- Removes `lazyBoundary` prop since there is no native equivalent
- Removes `lazyRoot` prop since there is no native equivalent
- Removes `loader` config in favor of [`loader`](#loader) prop
- Changed `alt` prop from optional to required

#### Possible Ways to Fix It

Run the [next-image-to-legacy-image](https://nextjs.org/docs/advanced-features/codemods#next-image-to-legacy-image) codemod to automatically change `next/image` imports to `next/legacy/image`, for example:

```
npx @next/codemod next-image-to-legacy-image .
```

After running this codemod, you can optionally upgrade `next/legacy/image` to the new `next/image` with another codemod, for example:

```
npx @next/codemod next-image-experimental .
```

Please note this second codemod is experimental and only covers static usage, not dynamic usage (such `<Image {...props} />`).

### Useful Links

- [Next.js 13 Blog Post](https://nextjs.org/blog/next-13)
- [`next/image` Documentation](https://nextjs.org/docs/api-reference/next/image)
- [`next/legacy/image` Documentation](https://nextjs.org/docs/api-reference/next/legacy/image)
6 changes: 6 additions & 0 deletions packages/next-codemod/transforms/next-image-experimental.ts
Expand Up @@ -57,6 +57,12 @@ function findAndReplaceProps(
objectPosition = String(a.value.value)
return false
}
if (a.name.name === 'lazyBoundary') {
return false
}
if (a.name.name === 'lazyRoot') {
return false
}

if (a.name.name === 'style') {
if (
Expand Down
33 changes: 15 additions & 18 deletions packages/next/client/image.tsx
Expand Up @@ -394,24 +394,6 @@ const ImageElement = ({
if (!srcString) {
console.error(`Image is missing required "src" property:`, img)
}
if (
img.getAttribute('objectFit') ||
img.getAttribute('objectfit')
) {
console.error(
`Image has unknown prop "objectFit". Did you mean to use the "style" prop instead?`,
img
)
}
if (
img.getAttribute('objectPosition') ||
img.getAttribute('objectposition')
) {
console.error(
`Image has unknown prop "objectPosition". Did you mean to use the "style" prop instead?`,
img
)
}
if (img.getAttribute('alt') === null) {
console.error(
`Image is missing required "alt" property. Please add Alternative Text to describe the image for screen readers and search engines.`
Expand Down Expand Up @@ -560,6 +542,21 @@ 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 @@ -7,12 +7,7 @@ const Page = () => {
return (
<div>
<h1 id="page-header">Static Image</h1>
<Image
id="basic-static"
src={testTall}
layout="fixed"
placeholder="blur"
/>
<Image id="basic-static" src={testTall} placeholder="blur" />
</div>
)
}
Expand Down
Expand Up @@ -7,12 +7,7 @@ const Page = () => {
return (
<div>
<h1 id="page-header">Static Image</h1>
<Image
id="basic-static"
src={testTall}
layout="fixed"
placeholder="blur"
/>
<Image id="basic-static" src={testTall} placeholder="blur" />
</div>
)
}
Expand Down
18 changes: 0 additions & 18 deletions test/integration/next-image-new/default/pages/invalid-objectfit.js

This file was deleted.

Expand Up @@ -29,7 +29,6 @@ const Page = () => {
<ImageWithMessage
id="3"
src="/test.svg"
layout="responsive"
width="1200"
height="1200"
idToCount={idToCount}
Expand Down
Expand Up @@ -15,7 +15,8 @@ const Page = () => {
width={200}
height={200}
src="/test.jpg"
objectFit="cover"
fill
style={{ objectFit: 'cover' }}
/>

<Image
Expand Down
Expand Up @@ -6,14 +6,7 @@ const Page = () => {
return (
<>
<h1>Warning should print at most once</h1>
<Image
id="w"
layout="unknown"
src="/test.png"
width="400"
height="400"
sizes="50vw"
/>
<Image id="w" src="/test.png" width="400" height="400" sizes="50vw" />
<button onClick={() => setCount(count + 1)}>Count: {count}</button>
<footer>footer here</footer>
</>
Expand Down
14 changes: 0 additions & 14 deletions test/integration/next-image-new/default/test/index.test.ts
Expand Up @@ -912,20 +912,6 @@ function runTests(mode) {
)
expect(warnings.length).toBe(1)
})

it('should show console error for objectFit and objectPosition', async () => {
const browser = await webdriver(appPort, '/invalid-objectfit')

expect(await hasRedbox(browser)).toBe(false)

await check(async () => {
return (await browser.log()).map((log) => log.message).join('\n')
}, /Image has unknown prop "objectFit"/gm)

await check(async () => {
return (await browser.log()).map((log) => log.message).join('\n')
}, /Image has unknown prop "objectPosition"/gm)
})
} else {
//server-only tests
it('should not create an image folder in server/chunks', async () => {
Expand Down
14 changes: 14 additions & 0 deletions test/integration/next-image-new/invalid-layout/pages/index.js
@@ -0,0 +1,14 @@
import React from 'react'
import Image from 'next/image'
import logo from '../public/logo.png'

const Page = () => {
return (
<div>
<h1>Should not use "layout" prop</h1>
<Image id="invalid-layout" layout="responsive" src={logo} />
</div>
)
}

export default Page
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
66 changes: 66 additions & 0 deletions test/integration/next-image-new/invalid-layout/test/index.test.ts
@@ -0,0 +1,66 @@
/* eslint-env jest */

import { join } from 'path'
import {
findPort,
hasRedbox,
killApp,
launchApp,
nextBuild,
} from 'next-test-utils'
import webdriver from 'next-webdriver'

const appDir = join(__dirname, '../')
let appPort: number
let app
let stderr = ''
const msg =
/Error: Image with src "(.*)logo(.*)png" has legacy prop "layout". Did you forget to run the codemod?./

function runTests({ isDev }) {
it('should show error', async () => {
if (isDev) {
const browser = await webdriver(appPort, '/')
expect(await hasRedbox(browser, true)).toBe(true)
expect(stderr).toMatch(msg)
} else {
expect(stderr).toMatch(msg)
}
})
}

describe('Missing Import Image Tests', () => {
describe('dev mode', () => {
beforeAll(async () => {
stderr = ''
appPort = await findPort()
app = await launchApp(appDir, appPort, {
onStderr(msg) {
stderr += msg || ''
},
})
})
afterAll(async () => {
if (app) {
await killApp(app)
}
})

runTests({ isDev: true })
})

describe('server mode', () => {
beforeAll(async () => {
stderr = ''
const result = await nextBuild(appDir, [], { stderr: true })
stderr = result.stderr
})
afterAll(async () => {
if (app) {
await killApp(app)
}
})

runTests({ isDev: false })
})
})
14 changes: 14 additions & 0 deletions test/integration/next-image-new/invalid-objectfit/pages/index.js
@@ -0,0 +1,14 @@
import React from 'react'
import Image from 'next/image'
import logo from '../public/logo.png'

const Page = () => {
return (
<div>
<h1>Should not use "objectFit" prop</h1>
<Image id="invalid-layout" objectFit="contain" src={logo} />
</div>
)
}

export default Page
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
@@ -0,0 +1,66 @@
/* eslint-env jest */

import { join } from 'path'
import {
findPort,
hasRedbox,
killApp,
launchApp,
nextBuild,
} from 'next-test-utils'
import webdriver from 'next-webdriver'

const appDir = join(__dirname, '../')
let appPort: number
let app
let stderr = ''
const msg =
/Error: Image with src "(.*)logo(.*)png" has legacy prop "objectFit". Did you forget to run the codemod?./

function runTests({ isDev }) {
it('should show error', async () => {
if (isDev) {
const browser = await webdriver(appPort, '/')
expect(await hasRedbox(browser, true)).toBe(true)
expect(stderr).toMatch(msg)
} else {
expect(stderr).toMatch(msg)
}
})
}

describe('Missing Import Image Tests', () => {
describe('dev mode', () => {
beforeAll(async () => {
stderr = ''
appPort = await findPort()
app = await launchApp(appDir, appPort, {
onStderr(msg) {
stderr += msg || ''
},
})
})
afterAll(async () => {
if (app) {
await killApp(app)
}
})

runTests({ isDev: true })
})

describe('server mode', () => {
beforeAll(async () => {
stderr = ''
const result = await nextBuild(appDir, [], { stderr: true })
stderr = result.stderr
})
afterAll(async () => {
if (app) {
await killApp(app)
}
})

runTests({ isDev: false })
})
})

0 comments on commit 191710d

Please sign in to comment.