Update no-img-element lint rule (#43982)

This updates the `no-img-element` lint rule to explain the trade-offs of enabling or disabling Image Optimization.
vercel · Dec 13, 2022 · 6dd74a6 · 6dd74a6
1 parent e89bcde
commit 6dd74a6
Show file tree

Hide file tree

Showing 5 changed files with 41 additions and 32 deletions.
diff --git a/docs/basic-features/eslint.md b/docs/basic-features/eslint.md
@@ -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`.                                             |

diff --git a/errors/no-img-element.md b/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
 ```
 
+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)
diff --git a/packages/eslint-plugin-next/src/rules/no-img-element.ts b/packages/eslint-plugin-next/src/rules/no-img-element.ts
@@ -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,
@@ -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}`,
         })
       },
     }

diff --git a/test/integration/eslint/test/index.test.js b/test/integration/eslint/test/index.test.js
@@ -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(
@@ -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(
@@ -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.')
     })
@@ -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.')
     })
@@ -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')
@@ -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',
             }),
           ])
         )
@@ -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. 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')
@@ -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')

diff --git a/test/unit/eslint-plugin-next/no-img-element.test.ts b/test/unit/eslint-plugin-next/no-img-element.test.ts
@@ -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',
         },
@@ -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',
         },