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

ESLint Updates #25895

Merged
merged 8 commits into from
Jun 8, 2021
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 6 additions & 3 deletions docs/api-reference/cli.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
41 changes: 41 additions & 0 deletions docs/api-reference/next.config.js/ignoring-eslint.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
---
description: Next.js reports ESLint errors and warnings during builds by default. Learn to opt-out of this behavior here.
housseindjirdeh marked this conversation as resolved.
Show resolved Hide resolved
---

# 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: {
// !! WARN !!
// Dangerously allow production builds to successfully complete even if
// your project has ESLint errors.
// !! WARN !!
housseindjirdeh marked this conversation as resolved.
Show resolved Hide resolved
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>
39 changes: 25 additions & 14 deletions docs/basic-features/eslint.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,13 @@ description: Next.js provides an integrated ESLint experience by default. These
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`:

```bash
next lint
npx next lint
Copy link
Member

Choose a reason for hiding this comment

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

Ideally I'd want to recommend npm run lint or yarn run lint to ensure it's always using the Next.js version that is installed in the project. This would mean we'd create a project with "lint": "next lint" in scripts as well

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ah good point, updated!

```

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
npx next lint

# You'll see instructions like these:
#
Expand Down Expand Up @@ -42,25 +42,36 @@ 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:

```bash
next build --no-lint
```

**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).
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):

## Linting Custom Directories

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`:
By default, Next.js will run ESLint for all files in the `pages/`, `components/`, and `lib/` directories. However, you can specify other custom directories to run by using the `--dir` flag in `next lint`:
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 way to change these defaults? E.g. maybe I don't want lib. Could be follow up later 😄

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Just added the option to configure this via next.config.js:

module.exports = {
  eslint: {
    dirs: ['pages', 'components'] // Only run ESLint on pages and components directories during builds
  }
}

f57a046#diff-ea22ff5b8d78b521a09affbcc239809dc95143cda0370d9992420ab86172ea58


```bash
next lint --dir components --dir lib
npx next lint --dir utils --dir custom
```

## 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

Expand All @@ -82,14 +93,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",
}
}
```
Expand Down
4 changes: 4 additions & 0 deletions docs/manifest.json
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
5 changes: 2 additions & 3 deletions packages/next/build/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -119,8 +119,7 @@ export default async function build(
dir: string,
conf = null,
reactProductionProfiling = false,
debugOutput = false,
runLint = true
debugOutput = false
): Promise<void> {
const nextBuildSpan = trace('next-build')

Expand Down Expand Up @@ -213,7 +212,7 @@ export default async function build(
typeCheckingSpinner.stopAndPersist()
}

if (runLint) {
if (!Boolean(config.eslint?.ignoreDuringBuilds)) {
await nextBuildSpan
.traceChild('verify-and-lint')
.traceAsyncFn(async () => {
Expand Down
4 changes: 1 addition & 3 deletions packages/next/cli/next-build.ts
Original file line number Diff line number Diff line change
Expand Up @@ -98,9 +98,7 @@ const nextBuild: cliCommand = (argv) => {
}

return preflight()
.then(() =>
build(dir, null, args['--profile'], args['--debug'], !args['--no-lint'])
)
.then(() => build(dir, null, args['--profile'], args['--debug']))
.catch((err) => {
console.error('')
console.error('> Build error occurred')
Expand Down
8 changes: 7 additions & 1 deletion packages/next/cli/next-lint.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down Expand Up @@ -66,7 +68,11 @@ const nextLint: cliCommand = (argv) => {

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)
Expand Down
19 changes: 11 additions & 8 deletions packages/next/lib/eslint/customFormatter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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`
: ''
}
11 changes: 8 additions & 3 deletions packages/next/lib/eslint/runLintCheck.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,10 @@ import * as CommentJson from 'next/dist/compiled/comment-json'

import { formatResults } from './customFormatter'
import { writeDefaultConfig } from './writeDefaultConfig'

import { getPackageVersion } from '../get-package-version'
import { findDir } from '../find-dir'
import { findPagesDir } from '../find-pages-dir'

import { CompileError } from '../compile-error'
import {
hasNecessaryDependencies,
Expand Down Expand Up @@ -74,6 +75,8 @@ async function lint(
}

const pagesDir = findPagesDir(baseDir)
const componentsDir = findDir(baseDir, 'components')
const libDir = findDir(baseDir, 'lib')

if (nextEslintPluginIsEnabled) {
let updatedPagesDir = false
Expand All @@ -98,10 +101,12 @@ async function lint(
}
}

// If no directories to lint are provided, only the pages directory will be linted
// If no directories to lint are provided, the pages, components, and lib directories will be linted
const filesToLint = lintDirs
? lintDirs.map(linteableFiles)
: linteableFiles(pagesDir)
: [pagesDir, componentsDir, libDir]
.filter((dir) => dir && dir.length > 0)
.map(linteableFiles)

const results = await eslint.lintFiles(filesToLint)

Expand Down
13 changes: 13 additions & 0 deletions packages/next/lib/find-dir.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
import path from 'path'
import { existsSync } from './find-pages-dir'

export function findDir(dir: string, dirName: string): string {
// prioritize ./pages over ./src/pages
let curDir = path.join(dir, dirName)
if (existsSync(curDir)) return curDir

curDir = path.join(dir, dirName)
if (existsSync(curDir)) return curDir

return ''
}
15 changes: 6 additions & 9 deletions packages/next/lib/has-necessary-dependencies.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.'
Copy link
Member

Choose a reason for hiding this comment

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

Should we only recommend this config option instead of recommending deleting the configuration? Would make it easier to toggle removing a bad config from editing right away.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah that probably makes sense (I only recently added that config option)

Fixed here: #25917

: `Once installed, run ${chalk.bold.cyan('next lint')} again.`)
const removalMsg = checkTSDeps ? removalTSMsg : removalLintMsg

Expand Down
7 changes: 7 additions & 0 deletions test/integration/eslint/ignore-during-builds/.eslintrc
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"extends": "next",
"root": true,
"rules": {
"@next/next/no-html-link-for-pages": 0
}
}
9 changes: 9 additions & 0 deletions test/integration/eslint/ignore-during-builds/next.config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
module.exports = {
eslint: {
// !! WARN !!
// Dangerously allow production builds to successfully complete even if
// your project has ESLint errors.
// !! WARN !!
ignoreDuringBuilds: true,
},
}
8 changes: 8 additions & 0 deletions test/integration/eslint/ignore-during-builds/pages/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
const Home = () => (
<div>
<p>Home</p>
/* Badly formatted comment */
</div>
)

export default Home
Loading