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

Update no-img-element lint rule #43982

Merged
merged 7 commits into from Dec 13, 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
2 changes: 1 addition & 1 deletion docs/basic-features/eslint.md
Expand Up @@ -94,7 +94,7 @@ Next.js provides an ESLint plugin, [`eslint-plugin-next`](https://www.npmjs.com/
| ✔️ | [@next/next/no-head-element](/docs/messages/no-head-element.md) | Prevent usage of `<head>` element. |
| ✔️ | [@next/next/no-head-import-in-document](/docs/messages/no-head-import-in-document.md) | Prevent usage of `next/head` in `pages/_document.js`. |
| ✔️ | [@next/next/no-html-link-for-pages](/docs/messages/no-html-link-for-pages.md) | Prevent usage of `<a>` elements to navigate to internal Next.js pages. |
| ✔️ | [@next/next/no-img-element](/docs/messages/no-img-element.md) | Prevent usage of `<img>` element to prevent layout shift. |
| ✔️ | [@next/next/no-img-element](/docs/messages/no-img-element.md) | Prevent usage of `<img>` element due to slower LCP and higher bandwidth. |
| ✔️ | [@next/next/no-page-custom-font](/docs/messages/no-page-custom-font.md) | Prevent page-only custom fonts. |
| ✔️ | [@next/next/no-script-component-in-head](/docs/messages/no-script-component-in-head.md) | Prevent usage of `next/script` in `next/head` component. |
| ✔️ | [@next/next/no-styled-jsx-in-document](/docs/messages/no-styled-jsx-in-document.md) | Prevent usage of `styled-jsx` in `pages/_document.js`. |
Expand Down
46 changes: 27 additions & 19 deletions errors/no-img-element.md
@@ -1,51 +1,59 @@
# No Img Element
# No img element

> Prevent usage of `<img>` element to prevent layout shift.
> Prevent usage of `<img>` element to prevent layout shift and favor [optimized images](https://nextjs.org/docs/basic-features/image-optimization).

### Why This Error Occurred

An `<img>` element was used to display an image. Use either `<picture>` in conjunction with `<img>` element, or use `next/image` that has better performance and automatic Image Optimization over `<img>`.
An `<img>` element was used to display an image.

### Possible Ways to Fix It

Import and use the `<Image />` component:
Use [`next/image`](https://nextjs.org/docs/api-reference/next/image) to improve performance with automatic [Image Optimization](https://nextjs.org/docs/basic-features/image-optimization).

> Note: If deploying to a [managed hosting provider](https://nextjs.org/docs/deployment), remember to check pricing since optimized images might be charged differently that the original images. If self-hosting, remember to install [`sharp`](https://www.npmjs.com/package/sharp) and check if your server has enough storage to cache the optimized images.

```jsx
import Image from 'next/image'

function Home() {
return (
<>
<Image
src="https://example.com/test"
alt="Landscape picture"
width={500}
height={500}
/>
</>
<Image
src="https://example.com/hero.jpg"
alt="Landscape picture"
width={800}
height={500}
/>
)
}

export default Home
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason we didn't include the section on unoptimized?

Copy link
Member Author

@styfle styfle Dec 13, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Its on line 32 but linked to instead of copy pasta. This doc is for the lint error, not really for next/image itself.

```

If you would like to use `next/image` featrues such as blur-up placeholders but disable Image Optimization, you can do so using [unoptimized](https://nextjs.org/docs/api-reference/next/image#unoptimized).

<br />

Use `<picture>` in conjunction with `<img>` element:
Or, use a `<picture>` element with the nested `<img>` element:

```jsx
function Home() {
return (
<>
<picture>
<source srcSet="https://example.com/test" type="image/webp" />
<img src="https://example.com/test" alt="Landscape picture" />
</picture>
</>
<picture>
<source srcSet="https://example.com/hero.avif" type="image/avif" />
<source srcSet="https://example.com/hero.webp" type="image/webp" />
<img
src="https://example.com/hero.jpg"
alt="Landscape picture"
width={800}
height={500}
/>
</picture>
)
}
```

### Useful Links

- [Image Component and Image Optimization](https://nextjs.org/docs/basic-features/image-optimization)
- [next/image API Reference](https://nextjs.org/docs/api-reference/next/image)
- [Largest Contentful Paint (LCP)](https://nextjs.org/learn/seo/web-performance/lcp)
5 changes: 3 additions & 2 deletions packages/eslint-plugin-next/src/rules/no-img-element.ts
Expand Up @@ -5,7 +5,8 @@ const url = 'https://nextjs.org/docs/messages/no-img-element'
export = defineRule({
meta: {
docs: {
description: 'Prevent usage of `<img>` element to prevent layout shift.',
description:
'Prevent usage of `<img>` element due to slower LCP and higher bandwidth.',
category: 'HTML',
recommended: true,
url,
Expand All @@ -30,7 +31,7 @@ export = defineRule({

context.report({
node,
message: `Do not use \`<img>\` element. Use \`<Image />\` from \`next/image\` instead. See: ${url}`,
message: `Using \`<img>\` could result in slower LCP and higher bandwidth. Use \`<Image />\` from \`next/image\` instead to utilize Image Optimization. See: ${url}`,
})
},
}
Expand Down
16 changes: 8 additions & 8 deletions test/integration/eslint/test/index.test.js
Expand Up @@ -94,7 +94,7 @@ describe('ESLint', () => {
'Error: `next/head` should not be imported in `pages/_document.js`. Use `<Head />` from `next/document` instead'
)
expect(output).toContain(
'Warning: Do not use `<img>` element. Use `<Image />` from `next/image` instead'
'Warning: Using `<img>` could result in slower LCP and higher bandwidth. Use `<Image />` from `next/image` instead to utilize Image Optimization.'
)
expect(output).toContain('Warning: Do not include stylesheets manually')
expect(output).toContain(
Expand Down Expand Up @@ -327,7 +327,7 @@ describe('ESLint', () => {
'Error: `next/head` should not be imported in `pages/_document.js`. Use `<Head />` from `next/document` instead'
)
expect(output).toContain(
'Warning: Do not use `<img>` element. Use `<Image />` from `next/image` instead'
'Warning: Using `<img>` could result in slower LCP and higher bandwidth. Use `<Image />` from `next/image` instead to utilize Image Optimization.'
)
expect(output).toContain('Warning: Do not include stylesheets manually')
expect(output).toContain(
Expand All @@ -349,7 +349,7 @@ describe('ESLint', () => {

const output = stdout + stderr
expect(output).toContain(
'Warning: Do not use `<img>` element. Use `<Image />` from `next/image` instead.'
'Warning: Using `<img>` could result in slower LCP and higher bandwidth. Use `<Image />` from `next/image` instead to utilize Image Optimization.'
)
expect(output).toContain('Error: Synchronous scripts should not be used.')
})
Expand Down Expand Up @@ -385,7 +385,7 @@ describe('ESLint', () => {

const output = stdout + stderr
expect(output).toContain(
'Warning: Do not use `<img>` element. Use `<Image />` from `next/image` instead.'
'Warning: Using `<img>` could result in slower LCP and higher bandwidth. Use `<Image />` from `next/image` instead to utilize Image Optimization.'
)
expect(output).toContain('Error: Synchronous scripts should not be used.')
})
Expand Down Expand Up @@ -658,7 +658,7 @@ describe('ESLint', () => {

expect(output).toContain('pages/bar.js')
expect(output).toContain(
'Do not use `<img>` element. Use `<Image />` from `next/image` instead.'
'Warning: Using `<img>` could result in slower LCP and higher bandwidth. Use `<Image />` from `next/image` instead to utilize Image Optimization.'
)

expect(output).not.toContain('pages/index.js')
Expand Down Expand Up @@ -703,7 +703,7 @@ describe('ESLint', () => {
}),
expect.objectContaining({
message:
'Do not use `<img>` element. Use `<Image />` from `next/image` instead. See: https://nextjs.org/docs/messages/no-img-element',
'Using `<img>` could result in slower LCP and higher bandwidth. Use `<Image />` from `next/image` instead to utilize Image Optimization. See: https://nextjs.org/docs/messages/no-img-element',
}),
])
)
Expand Down Expand Up @@ -742,7 +742,7 @@ describe('ESLint', () => {
'img elements must have an alt prop, either with meaningful text, or an empty string for decorative images.'
)
expect(fileOutput).toContain(
'Do not use `<img>` element. Use `<Image />` from `next/image` instead. See: https://nextjs.org/docs/messages/no-img-element'
"Using `<img>` could result in slower LCP and higher bandwidth because it doesn't utilize Image Optimization. Use `<Image />` from `next/image` instead to utilize Image Optimization. See: https://nextjs.org/docs/messages/no-img-element"
)

expect(fileOutput).toContain('file-linting/pages/index.js')
Expand Down Expand Up @@ -782,7 +782,7 @@ describe('ESLint', () => {
'img elements must have an alt prop, either with meaningful text, or an empty string for decorative images.'
)
expect(output).toContain(
'Do not use `<img>` element. Use `<Image />` from `next/image` instead. See: https://nextjs.org/docs/messages/no-img-element'
'Using `<img>` could result in slower LCP and higher bandwidth. Use `<Image />` from `next/image` instead to utilize Image Optimization. See: https://nextjs.org/docs/messages/no-img-element'
)

expect(output).toContain('pages/index.cjs')
Expand Down
4 changes: 2 additions & 2 deletions test/unit/eslint-plugin-next/no-img-element.test.ts
Expand Up @@ -81,7 +81,7 @@ ruleTester.run('no-img-element', rule, {
errors: [
{
message:
'Do not use `<img>` element. Use `<Image />` from `next/image` instead. ' +
'Using `<img>` could result in slower LCP and higher bandwidth. Use `<Image />` from `next/image` instead to utilize Image Optimization. ' +
'See: https://nextjs.org/docs/messages/no-img-element',
type: 'JSXOpeningElement',
},
Expand All @@ -104,7 +104,7 @@ ruleTester.run('no-img-element', rule, {
errors: [
{
message:
'Do not use `<img>` element. Use `<Image />` from `next/image` instead. ' +
'Using `<img>` could result in slower LCP and higher bandwidth. Use `<Image />` from `next/image` instead to utilize Image Optimization. ' +
'See: https://nextjs.org/docs/messages/no-img-element',
type: 'JSXOpeningElement',
},
Expand Down