ESLint Updates (#25895)

* lots of eslint updates * adds eslint: dirs flag to next.config.js * update lint command in docs * update per review comments * re-add --no-lint command for build * Apply suggestions from code review Co-authored-by: JJ Kasper <[email protected]>
vercel · Jun 8, 2021 · 50a36ee · 50a36ee
1 parent 7b77415
commit 50a36ee
Show file tree

Hide file tree

Showing 19 changed files with 247 additions and 90 deletions.
diff --git a/docs/api-reference/cli.md b/docs/api-reference/cli.md
@@ -86,12 +86,15 @@ npx next start -p 4000
 
 ## Lint
 
-`next lint` runs ESLint for all files in the `pages` directory and provides a guided setup to install any required dependencies if ESLint is not already configured in your application.
+`next lint` runs ESLint for all files in the `pages`, `components`, and `lib` directories. It also
+provides a guided setup to install any required dependencies if ESLint is not already configured in
+your application.
 
-You can also run ESLint on other directories with the `--dir` flag:
+If you have other directories that you would like to lint, you can specify them using the `--dir`
+flag:
 
 ```bash
-next lint --dir components
+next lint --dir utils
 ```
 
 ## Telemetry

diff --git a/docs/api-reference/next.config.js/ignoring-eslint.md b/docs/api-reference/next.config.js/ignoring-eslint.md
@@ -0,0 +1,39 @@
+---
+description: Next.js reports ESLint errors and warnings during builds by default. Learn how to opt-out of this behavior here.
+---
+
+# Ignoring ESLint
+
+When ESLint is detected in your project, Next.js fails your **production build** (`next build`) when errors are present.
+
+If you'd like Next.js to dangerously produce production code even when your application has ESLint errors, you can disable the built-in linting step completely.
+
+> Be sure you have configured ESLint to run in a separate part of your workflow (for example, in CI or a pre-commit hook).
+
+Open `next.config.js` and enable the `ignoreDuringBuilds` option in the `eslint` config:
+
+```js
+module.exports = {
+  eslint: {
+    // Warning: Dangerously allow production builds to successfully complete even if
+    // your project has ESLint errors.
+    ignoreDuringBuilds: true,
+  },
+}
+```
+
+## Related
+
+<div class="card">
+  <a href="/docs/api-reference/next.config.js/introduction.md">
+    <b>Introduction to next.config.js:</b>
+    <small>Learn more about the configuration file used by Next.js.</small>
+  </a>
+</div>
+
+<div class="card">
+  <a href="/docs/basic-features/eslint.md">
+    <b>ESLint:</b>
+    <small>Get started with ESLint in Next.js.</small>
+  </a>
+</div>
diff --git a/docs/basic-features/eslint.md b/docs/basic-features/eslint.md
@@ -4,16 +4,24 @@ description: Next.js provides an integrated ESLint experience by default. These
 
 # ESLint
 
-Since version **11.0.0**, Next.js provides an integrated [ESLint](https://eslint.org/) experience out of the box. To get started, run `next lint`:
+Since version **11.0.0**, Next.js provides an integrated [ESLint](https://eslint.org/) experience out of the box. Add `next lint` as a script to `package.json`:
+
+```json
+"scripts": {
+  "lint": "next lint"
+}
+```
+
+Then run `npm run lint` or `yarn lint`:
 
 ```bash
-next lint
+yarn lint
 ```
 
 If you don't already have ESLint configured in your application, you will be guided through the installation of the required packages.
 
 ```bash
-next lint
+yarn lint
 
 # You'll see instructions like these:
 #
@@ -42,25 +50,46 @@ We recommend using an appropriate [integration](https://eslint.org/docs/user-gui
 
 Once ESLint has been set up, it will automatically run during every build (`next build`). Errors will fail the build, while warnings will not.
 
-If you do not want ESLint to run as a build step, it can be disabled using the `--no-lint` flag:
+If you do not want ESLint to run as a build step, refer to the documentation for [Ignoring ESLint](/docs/api-reference/next.config.js/ignoring-eslint.md):
 
-```bash
-next build --no-lint
-```
+## Linting Custom Directories
 
-**This is not recommended** unless you have configured ESLint to run in a separate part of your workflow (for example, in CI or a pre-commit hook).
+By default, Next.js will run ESLint for all files in the `pages/`, `components/`, and `lib/` directories. However, you can specify which directories using the `dirs` option in the `eslint` config in `next.config.js` for production builds:
 
-## Linting Custom Directories
+```js
+module.exports = {
+  eslint: {
+    dirs: ['pages', 'utils'], // Only run ESLint on the 'pages' and 'utils' directories during production builds (next build)
+  },
+}
+```
 
-By default, Next.js will only run ESLint for all files in the `pages/` directory. However, you can specify other custom directories to run by using the `--dir` flag in `next lint`:
+Similarly, the `--dir` flag can be used for `next lint`:
 
 ```bash
-next lint --dir components --dir lib
+yarn lint --dir pages --dir utils
 ```
 
 ## ESLint Plugin
 
-Next.js provides an ESLint plugin, [`eslint-plugin-next`](https://www.npmjs.com/package/@next/eslint-plugin-next), making it easier to catch common issues and problems in a Next.js application. The full set of rules can be found in the [package repository](https://github.com/vercel/next.js/tree/master/packages/eslint-plugin-next/lib/rules).
+Next.js provides an ESLint plugin, [`eslint-plugin-next`](https://www.npmjs.com/package/@next/eslint-plugin-next), making it easier to catch common issues and problems in a Next.js application. The full set of rules is as follows:
+
+|     | Rule                                                                                           | Description                                                      |
+| :-: | ---------------------------------------------------------------------------------------------- | ---------------------------------------------------------------- |
+| ✔️  | [next/google-font-display](https://nextjs.org/docs/messages/google-font-display)               | Enforce optional or swap font-display behavior with Google Fonts |
+| ✔️  | [next/google-font-preconnect](https://nextjs.org/docs/messages/google-font-preconnect)         | Enforce preconnect usage with Google Fonts                       |
+| ✔️  | [next/link-passhref](https://nextjs.org/docs/messages/link-passhref)                           | Enforce passHref prop usage with custom Link components          |
+| ✔️  | [next/no-css-tags](https://nextjs.org/docs/messages/no-css-tags)                               | Prevent manual stylesheet tags                                   |
+| ✔️  | [next/no-document-import-in-page](https://nextjs.org/docs/messages/no-document-import-in-page) | Disallow importing next/document outside of pages/document.js    |
+| ✔️  | [next/no-head-import-in-document](https://nextjs.org/docs/messages/no-head-import-in-document) | Disallow importing next/head in pages/document.js                |
+| ✔️  | [next/no-html-link-for-pages](https://nextjs.org/docs/messages/no-html-link-for-pages)         | Prohibit HTML anchor links to pages without a Link component     |
+| ✔️  | [next/no-img-element](https://nextjs.org/docs/messages/no-img-element)                         | Prohibit usage of HTML &lt;img&gt; element                       |
+| ✔️  | [next/no-page-custom-font](https://nextjs.org/docs/messages/no-page-custom-font)               | Prevent page-only custom fonts                                   |
+| ✔️  | [next/no-sync-scripts](https://nextjs.org/docs/messages/no-sync-scripts)                       | Forbid synchronous scripts                                       |
+| ✔️  | [next/no-title-in-document-head](https://nextjs.org/docs/messages/no-title-in-document-head)   | Disallow using &lt;title&gt; with Head from next/document        |
+| ✔️  | [next/no-unwanted-polyfillio](https://nextjs.org/docs/messages/no-unwanted-polyfillio)         | Prevent duplicate polyfills from Polyfill.io                     |
+
+- ✔: Enabled in the recommended configuration
 
 ## Base Configuration
 
@@ -82,14 +111,14 @@ You can see the full details of the shareable configuration in the [`eslint-conf
 
 ## Disabling Rules
 
-If you would like to modify any rules provided by the supported plugins (`react`, `react-hooks`, `next`), you can directly modify them using the `rules` property in your `.eslintrc`:
+If you would like to modify or disable any rules provided by the supported plugins (`react`, `react-hooks`, `next`), you can directly change them using the `rules` property in your `.eslintrc`:
 
 ```js
 {
   "extends": "next",
   "rules": {
     "react/no-unescaped-entities": "off",
-    "@next/next/no-page-custom-font": "error",
+    "@next/next/no-page-custom-font": "off",
   }
 }
 ```

diff --git a/docs/manifest.json b/docs/manifest.json
@@ -344,6 +344,10 @@
               "title": "Configuring onDemandEntries",
               "path": "/docs/api-reference/next.config.js/configuring-onDemandEntries.md"
             },
+            {
+              "title": "Ignoring ESLint",
+              "path": "/docs/api-reference/next.config.js/ignoring-eslint.md"
+            },
             {
               "title": "Ignoring TypeScript Errors",
               "path": "/docs/api-reference/next.config.js/ignoring-typescript-errors.md"

diff --git a/packages/next/build/index.ts b/packages/next/build/index.ts
@@ -213,12 +213,15 @@ export default async function build(
       typeCheckingSpinner.stopAndPersist()
     }
 
-    if (runLint) {
+    const ignoreESLint = Boolean(config.eslint?.ignoreDuringBuilds)
+    const lintDirs = config.eslint?.dirs
+    if (!ignoreESLint && runLint) {
       await nextBuildSpan
         .traceChild('verify-and-lint')
         .traceAsyncFn(async () => {
           await verifyAndLint(
             dir,
+            lintDirs,
             config.experimental.cpus,
             config.experimental.workerThreads
           )

diff --git a/packages/next/cli/next-lint.ts b/packages/next/cli/next-lint.ts
@@ -2,6 +2,8 @@
 import { existsSync } from 'fs'
 import arg from 'next/dist/compiled/arg/index.js'
 import { resolve, join } from 'path'
+import chalk from 'chalk'
+
 import { cliCommand } from '../bin/next'
 import { runLintCheck } from '../lib/eslint/runLintCheck'
 import { printAndExit } from '../server/lib/utils'
@@ -55,18 +57,23 @@ const nextLint: cliCommand = (argv) => {
   }
 
   const dirs: string[] = args['--dir']
-  const lintDirs = dirs
-    ? dirs.reduce((res: string[], d: string) => {
-        const currDir = join(baseDir, d)
-        if (!existsSync(currDir)) return res
-        res.push(currDir)
-        return res
-      }, [])
-    : null
+  const lintDirs = (dirs ?? ['pages', 'components', 'lib']).reduce(
+    (res: string[], d: string) => {
+      const currDir = join(baseDir, d)
+      if (!existsSync(currDir)) return res
+      res.push(currDir)
+      return res
+    },
+    []
+  )
 
   runLintCheck(baseDir, lintDirs)
     .then((results) => {
-      if (results) console.log(results)
+      if (results) {
+        console.log(results)
+      } else {
+        console.log(chalk.green('✔ No ESLint warnings or errors'))
+      }
     })
     .catch((err) => {
       printAndExit(err.message)

diff --git a/packages/next/lib/eslint/customFormatter.ts b/packages/next/lib/eslint/customFormatter.ts
@@ -70,12 +70,15 @@ function formatMessage(
 }
 
 export function formatResults(baseDir: string, results: LintResult[]): string {
-  return (
-    results
-      .filter(({ messages }) => messages?.length)
-      .map(({ messages, filePath }) =>
-        formatMessage(baseDir, messages, filePath)
-      )
-      .join('\n') + '\n'
-  )
+  const formattedResults = results
+    .filter(({ messages }) => messages?.length)
+    .map(({ messages, filePath }) => formatMessage(baseDir, messages, filePath))
+    .join('\n')
+
+  return formattedResults.length > 0
+    ? formattedResults +
+        `\n\n${chalk.bold(
+          'Need to disable some ESLint rules? Learn more here:'
+        )} https://nextjs.org/docs/basic-features/eslint#disabling-rules\n`
+    : ''
 }
diff --git a/packages/next/lib/eslint/runLintCheck.ts b/packages/next/lib/eslint/runLintCheck.ts
@@ -8,7 +8,6 @@ import * as CommentJson from 'next/dist/compiled/comment-json'
 import { formatResults } from './customFormatter'
 import { writeDefaultConfig } from './writeDefaultConfig'
 import { findPagesDir } from '../find-pages-dir'
-
 import { CompileError } from '../compile-error'
 import {
   hasNecessaryDependencies,
@@ -29,7 +28,7 @@ const linteableFiles = (dir: string) => {
 async function lint(
   deps: NecessaryDependencies,
   baseDir: string,
-  lintDirs: string[] | null,
+  lintDirs: string[],
   eslintrcFile: string | null,
   pkgJsonPath: string | null
 ): Promise<string | null> {
@@ -103,13 +102,7 @@ async function lint(
     }
   }
 
-  // If no directories to lint are provided, only the pages directory will be linted
-  const filesToLint = lintDirs
-    ? lintDirs.map(linteableFiles)
-    : linteableFiles(pagesDir)
-
-  const results = await eslint.lintFiles(filesToLint)
-
+  const results = await eslint.lintFiles(lintDirs.map(linteableFiles))
   if (ESLint.getErrorResults(results)?.length > 0) {
     throw new CompileError(await formatResults(baseDir, results))
   }
@@ -118,7 +111,7 @@ async function lint(
 
 export async function runLintCheck(
   baseDir: string,
-  lintDirs: string[] | null,
+  lintDirs: string[],
   lintDuringBuild: boolean = false
 ): Promise<string | null> {
   try {

diff --git a/packages/next/lib/has-necessary-dependencies.ts b/packages/next/lib/has-necessary-dependencies.ts
@@ -70,18 +70,15 @@ export async function hasNecessaryDependencies(
   const removalLintMsg =
     `\n\n` +
     (lintDuringBuild
-      ? `If you do not want to run ESLint during builds, run ${chalk.bold.cyan(
-          'next build --no-lint'
-        )}` +
+      ? `If you do not want to run ESLint during builds, ` +
         (!!eslintrcFile
-          ? ` or remove the ${chalk.bold(
+          ? `remove the ${chalk.bold(
               basename(eslintrcFile)
-            )} file from your package root.`
+            )} file from your package root`
           : pkgJsonEslintConfig
-          ? ` or remove the ${chalk.bold(
-              'eslintConfig'
-            )} field from package.json.`
-          : '')
+          ? `remove the ${chalk.bold('eslintConfig')} field from package.json`
+          : '') +
+        ' or disable it in next.config.js.\n\nSee https://nextjs.org/docs/api-reference/next.config.js/ignoring-eslint.'
       : `Once installed, run ${chalk.bold.cyan('next lint')} again.`)
   const removalMsg = checkTSDeps ? removalTSMsg : removalLintMsg
 

diff --git a/packages/next/lib/verifyAndLint.ts b/packages/next/lib/verifyAndLint.ts
@@ -1,8 +1,11 @@
 import chalk from 'chalk'
 import { Worker } from 'jest-worker'
+import { existsSync } from 'fs'
+import { join } from 'path'
 
 export async function verifyAndLint(
   dir: string,
+  configLintDirs: string[] | undefined,
   numWorkers: number | undefined,
   enableWorkerThreads: boolean | undefined
 ): Promise<void> {
@@ -17,7 +20,17 @@ export async function verifyAndLint(
     lintWorkers.getStdout().pipe(process.stdout)
     lintWorkers.getStderr().pipe(process.stderr)
 
-    const lintResults = await lintWorkers.runLintCheck(dir, null, true)
+    const lintDirs = (configLintDirs ?? ['pages', 'components', 'lib']).reduce(
+      (res: string[], d: string) => {
+        const currDir = join(dir, d)
+        if (!existsSync(currDir)) return res
+        res.push(currDir)
+        return res
+      },
+      []
+    )
+
+    const lintResults = await lintWorkers.runLintCheck(dir, lintDirs, true)
     if (lintResults) {
       console.log(lintResults)
     }

diff --git a/test/integration/eslint/custom-directories/.eslintrc b/test/integration/eslint/custom-directories/.eslintrc
@@ -0,0 +1,7 @@
+{
+  "extends": "next",
+  "root": true,
+  "rules": {
+    "@next/next/no-html-link-for-pages": 0
+  }
+}
diff --git a/test/integration/eslint/custom-directories/custom/index.js b/test/integration/eslint/custom-directories/custom/index.js
@@ -0,0 +1,7 @@
+const Custom = () => (
+  <div>
+    <script src="https://example.com" />
+  </div>
+)
+
+export default Custom
diff --git a/test/integration/eslint/custom-directories/next.config.js b/test/integration/eslint/custom-directories/next.config.js
@@ -0,0 +1,5 @@
+module.exports = {
+  eslint: {
+    dirs: ['utils', 'custom'],
+  },
+}
diff --git a/test/integration/eslint/custom-directories/pages/index.js b/test/integration/eslint/custom-directories/pages/index.js
@@ -0,0 +1,8 @@
+const Home = () => (
+  <div>
+    <p>Home</p>
+    <a href="test">Test link</a>
+  </div>
+)
+
+export default Home
diff --git a/test/integration/eslint/custom-directories/utils/index.js b/test/integration/eslint/custom-directories/utils/index.js
@@ -0,0 +1,3 @@
+const Utils = () => <div>/* Badly formatted comment */</div>
+
+export default Utils
diff --git a/test/integration/eslint/ignore-during-builds/.eslintrc b/test/integration/eslint/ignore-during-builds/.eslintrc
@@ -0,0 +1,7 @@
+{
+  "extends": "next",
+  "root": true,
+  "rules": {
+    "@next/next/no-html-link-for-pages": 0
+  }
+}