From 50a36eecd35e45125491a692a3fc5333cc165b0c Mon Sep 17 00:00:00 2001 From: Houssein Djirdeh Date: Tue, 8 Jun 2021 15:46:00 -0700 Subject: [PATCH] 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 --- docs/api-reference/cli.md | 9 +- .../next.config.js/ignoring-eslint.md | 39 +++++++++ docs/basic-features/eslint.md | 57 +++++++++---- docs/manifest.json | 4 + packages/next/build/index.ts | 5 +- packages/next/cli/next-lint.ts | 25 ++++-- packages/next/lib/eslint/customFormatter.ts | 19 +++-- packages/next/lib/eslint/runLintCheck.ts | 13 +-- .../next/lib/has-necessary-dependencies.ts | 15 ++-- packages/next/lib/verifyAndLint.ts | 15 +++- .../eslint/custom-directories/.eslintrc | 7 ++ .../eslint/custom-directories/custom/index.js | 7 ++ .../eslint/custom-directories/next.config.js | 5 ++ .../eslint/custom-directories/pages/index.js | 8 ++ .../eslint/custom-directories/utils/index.js | 3 + .../eslint/ignore-during-builds/.eslintrc | 7 ++ .../ignore-during-builds/next.config.js | 9 ++ .../ignore-during-builds/pages/index.js | 8 ++ test/integration/eslint/test/index.test.js | 82 +++++++++++-------- 19 files changed, 247 insertions(+), 90 deletions(-) create mode 100644 docs/api-reference/next.config.js/ignoring-eslint.md create mode 100644 test/integration/eslint/custom-directories/.eslintrc create mode 100644 test/integration/eslint/custom-directories/custom/index.js create mode 100644 test/integration/eslint/custom-directories/next.config.js create mode 100644 test/integration/eslint/custom-directories/pages/index.js create mode 100644 test/integration/eslint/custom-directories/utils/index.js create mode 100644 test/integration/eslint/ignore-during-builds/.eslintrc create mode 100644 test/integration/eslint/ignore-during-builds/next.config.js create mode 100644 test/integration/eslint/ignore-during-builds/pages/index.js diff --git a/docs/api-reference/cli.md b/docs/api-reference/cli.md index ab91148fb834a..f13204ad7808f 100644 --- 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 new file mode 100644 index 0000000000000..6a4c06799ed7e --- /dev/null +++ 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 + +
+ + Introduction to next.config.js: + Learn more about the configuration file used by Next.js. + +
+ +
+ + ESLint: + Get started with ESLint in Next.js. + +
diff --git a/docs/basic-features/eslint.md b/docs/basic-features/eslint.md index 040bb5fb95758..6d16e640f5c76 100644 --- 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 <img> 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 <title> 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 index ceefa4ceca61c..e87c8b06e3866 100644 --- 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 index e5c44640df2e9..ff02753bb52e0 100644 --- 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 index cc8216c840151..02e4216008fb8 100755 --- 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 index 5e7d6cf9c3118..5d956860a568f 100644 --- 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 index c94b89e96b94e..781f6bbc60d7f 100644 --- 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 { @@ -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 { try { diff --git a/packages/next/lib/has-necessary-dependencies.ts b/packages/next/lib/has-necessary-dependencies.ts index 9d6abff18d371..cf69f826994f7 100644 --- 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 index 3b0a48a56157f..798f2b6ff9360 100644 --- 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 { @@ -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 new file mode 100644 index 0000000000000..22b9919b24161 --- /dev/null +++ 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 new file mode 100644 index 0000000000000..3735bac42588d --- /dev/null +++ b/test/integration/eslint/custom-directories/custom/index.js @@ -0,0 +1,7 @@ +const Custom = () => ( +
+