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

Adds ESLint with default rule-set #23702

Merged
merged 54 commits into from
Apr 30, 2021
Merged
Show file tree
Hide file tree
Changes from 18 commits
Commits
Show all changes
54 commits
Select commit Hold shift + click to select a range
2145927
re-include ESLint integration
housseindjirdeh Mar 31, 2021
dec0835
moves eslint flag to experimental
housseindjirdeh Mar 31, 2021
da2b9b5
creates next shareable eslint config
housseindjirdeh Mar 31, 2021
708cadf
update lint architecture to provide guided setup
housseindjirdeh Apr 1, 2021
5785c3d
removes eslint check in dev server
housseindjirdeh Apr 2, 2021
7d2c6b9
adds links to plugin rules
housseindjirdeh Apr 2, 2021
220b624
includes typescript parser as a required package if typescript is used
housseindjirdeh Apr 2, 2021
204961f
removes dev time linting
housseindjirdeh Apr 4, 2021
a036090
adds peer dependencies for eslint config
housseindjirdeh Apr 5, 2021
539c89d
updates dependency installing instructions
housseindjirdeh Apr 5, 2021
a1a2000
updates dependency installing instructions part 2
housseindjirdeh Apr 5, 2021
fc81a34
updates eslint plugint tests
housseindjirdeh Apr 5, 2021
cd9587a
updates eslint integration tests
housseindjirdeh Apr 5, 2021
b0bf01e
removes unecessary changes
housseindjirdeh Apr 5, 2021
e925c22
removes js-yaml
housseindjirdeh Apr 5, 2021
579aebc
fixes integration test
housseindjirdeh Apr 5, 2021
74f5daa
updates eslint ignore
housseindjirdeh Apr 5, 2021
9836b64
switch eslint config to unscoped npm package
housseindjirdeh Apr 12, 2021
c56d522
re-use dependency check for typescript for eslint
housseindjirdeh Apr 13, 2021
1dd440a
uses rushstack eslint patch + optimizations
housseindjirdeh Apr 14, 2021
a48e17b
update eslint plugin rules and include error docs
housseindjirdeh Apr 15, 2021
296d9d7
moar updates
housseindjirdeh Apr 16, 2021
6d252e4
adds comment on top of shareable config
housseindjirdeh Apr 16, 2021
290025c
updates test
housseindjirdeh Apr 16, 2021
302a00f
updates manifest json for eslint plugin error docs
housseindjirdeh Apr 16, 2021
c6e100e
minor edits to error docs
housseindjirdeh Apr 16, 2021
c227473
Merge branch 'canary' into eslint
housseindjirdeh Apr 16, 2021
63dd31e
fix ts intent for dev time
housseindjirdeh Apr 16, 2021
89c9e1a
Apply suggestions from code review
housseindjirdeh Apr 17, 2021
0ef0111
fixes eslint resolver extensions
housseindjirdeh Apr 17, 2021
3a5a4e0
adds with-eslint to examples
housseindjirdeh Apr 17, 2021
4fbaac0
minor update
housseindjirdeh Apr 18, 2021
0a6df47
fixes with-eslint example for pnp
housseindjirdeh Apr 18, 2021
d7bbee1
fixes merge conflict
housseindjirdeh Apr 18, 2021
d33f32b
Merge remote-tracking branch 'upstream/canary' into eslint
housseindjirdeh Apr 19, 2021
8dee9c4
re-include worker for lint check
housseindjirdeh Apr 20, 2021
cfb07a0
Merge remote-tracking branch 'upstream/canary' into eslint
housseindjirdeh Apr 20, 2021
d3702aa
revert unnecessary change to test
housseindjirdeh Apr 20, 2021
f818bc3
updates per review
housseindjirdeh Apr 22, 2021
cdeec5a
fix eslint test dir location
housseindjirdeh Apr 22, 2021
f0f4955
check precompile
housseindjirdeh Apr 22, 2021
fbac46e
ncc babel/eslint-parser
housseindjirdeh Apr 22, 2021
e1bdcb7
Merge branch 'canary' into eslint
housseindjirdeh Apr 22, 2021
df68057
add next as peer dep to plugin and resolve path in test
housseindjirdeh Apr 22, 2021
6719fa0
update test
housseindjirdeh Apr 23, 2021
f0954de
resolve merge conflicts
housseindjirdeh Apr 26, 2021
1f12e96
disable single-line no shadow rule
housseindjirdeh Apr 26, 2021
1d740b2
removes missing preload rule from eslint plugin
housseindjirdeh Apr 29, 2021
0b905f6
update wording on no css tags eslint plugin rule
housseindjirdeh Apr 29, 2021
1686dfb
reword html link rule in eslint plugin
housseindjirdeh Apr 29, 2021
c3195e0
removes error doc reference in manifest json
housseindjirdeh Apr 29, 2021
51c8aa9
fix failing test
housseindjirdeh Apr 29, 2021
2a81a5c
Merge branch 'canary' into eslint
housseindjirdeh Apr 29, 2021
01462a1
Merge branch 'canary' of github.com:vercel/next.js into eslint
timneutkens Apr 30, 2021
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
2 changes: 2 additions & 0 deletions .eslintignore
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ node_modules
**/.next/**
**/_next/**
**/dist/**
e2e-tests/**
timneutkens marked this conversation as resolved.
Show resolved Hide resolved
examples/with-typescript-eslint-jest/**
examples/with-kea/**
packages/next/bundles/webpack/packages/*.runtime.js
Expand All @@ -16,4 +17,5 @@ packages/next-codemod/**/*.js
packages/next-codemod/**/*.d.ts
packages/next-env/**/*.d.ts
test/integration/async-modules/**
test/integration/eslint/**
test-timings.json
39 changes: 39 additions & 0 deletions packages/eslint-config-next/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
module.exports = {
extends: [
'plugin:react/recommended',
'plugin:react-hooks/recommended',
'plugin:@next/next/recommended',
],
plugins: ['import', 'react'],
rules: {
'import/no-anonymous-default-export': 'warn',
'react/react-in-jsx-scope': 'off',
},
parser: '@babel/eslint-parser',
parserOptions: {
requireConfigFile: false,
sourceType: 'module',
allowImportExportEverywhere: true,
babelOptions: {
presets: ['next/babel'],
},
},
overrides: [
{
files: ['**/*.ts?(x)'],
parser: '@typescript-eslint/parser',
parserOptions: {
sourceType: 'module',
ecmaFeatures: {
jsx: true,
},
warnOnUnsupportedTypeScriptVersion: true,
},
},
],
settings: {
react: {
version: 'detect',
},
housseindjirdeh marked this conversation as resolved.
Show resolved Hide resolved
},
}
26 changes: 26 additions & 0 deletions packages/eslint-config-next/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
{
"name": "eslint-config-next",
timneutkens marked this conversation as resolved.
Show resolved Hide resolved
"version": "10.1.3",
"description": "ESLint configuration used by NextJS.",
"main": "index.js",
"license": "MIT",
"repository": {
"url": "vercel/next.js",
"directory": "packages/eslint-config-next"
},
"peerDependencies": {
"@babel/core": "^7.13.14",
Copy link
Member

Choose a reason for hiding this comment

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

I'm trying to understand where @babel/core and @babel/eslint-parser parser is used, this will increase install size and run a separate version of Babel than the one that Next.js has bundled which will lead to differences in parsing long-term. We'll likely want to expose a Next.js specific eslint parser.

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 it'll increase install size unfortunately, but I've set it up to point to next/babel so that it stays in line with how Babel is configured in Next.js instead of relying on a user's config.

  
  parserOptions: {
    requireConfigFile: false,
    babelOptions: {
      presets: ['next/babel'],
    },
  },
  

Run a separate version of Babel than the one that Next.js has bundled

Would it make sense to lift @babel/core to the root package.json in order to share the same version across both workspaces?

We'll likely want to expose a Next.js specific eslint parser.

Do you mean create a separate, custom eslint parser to handle experimental syntax instead of relying on @babel/eslint-parser? Wouldn't we still need to align with Babel parsing (or are you thinking that a custom parser could do the same with minimal delta to install size)?

Copy link
Member

Choose a reason for hiding this comment

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

Do you mean create a separate, custom eslint parser to handle experimental syntax instead of relying on @babel/eslint-parser? Wouldn't we still need to align with Babel parsing (or are you thinking that a custom parser could do the same with minimal delta to install size)?

Bundling the eslint parser in the same way we bundle Babel with ncc, that'll allow for the parser to use the internal bundled Babel version.

Copy link
Collaborator Author

@housseindjirdeh housseindjirdeh Apr 22, 2021

Choose a reason for hiding this comment

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

Ah makes sense, thanks.

Done!

"@babel/eslint-parser": "^7.13.14",
"@next/eslint-plugin-next": "^10.1.3",
"@typescript-eslint/parser": "^4.20.0",
"eslint": "^7.23.0",
housseindjirdeh marked this conversation as resolved.
Show resolved Hide resolved
"eslint-plugin-import": "^2.22.1",
housseindjirdeh marked this conversation as resolved.
Show resolved Hide resolved
"eslint-plugin-react": "^7.23.1",
"eslint-plugin-react-hooks": "^4.2.0"
},
"peerDependenciesMeta": {
"@typescript-eslint/parser": {
"optional": true
}
}
}
2 changes: 1 addition & 1 deletion packages/eslint-plugin-next/lib/rules/missing-preload.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ module.exports = {
context.report({
node: linkNode,
message:
'Stylesheet does not have an associated preload tag. This could potentially impact First paint.',
'Stylesheet does not have an associated preload tag. This could potentially impact First paint. See: https://web.dev/preload-critical-assets/.',
Copy link
Member

Choose a reason for hiding this comment

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

It might be good to add docs in the top-level errors folder https://github.com/vercel/next.js/tree/canary/errors for these and link to them via https://nextjs.org/docs/messages to allow to us to provide the direct fix needed to resolve the message and then provide additional resources as links in the document.

Copy link
Collaborator Author

@housseindjirdeh housseindjirdeh Apr 16, 2021

Choose a reason for hiding this comment

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

Ooh great point, didn't even know this folder existed 😅

Done! Added a doc page for each rule.

fix: function (fixer) {
return fixer.insertTextBefore(
linkNode,
Expand Down
2 changes: 1 addition & 1 deletion packages/eslint-plugin-next/lib/rules/no-css-tags.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ module.exports = function (context) {
context.report({
node,
message:
'In order to use external stylesheets use @import in the root stylesheet compiled with NextJS. This ensures proper priority to CSS when loading a webpage.',
'In order to use external stylesheets use @import in the root stylesheet compiled with NextJS. This ensures proper priority to CSS when loading a webpage. See: https://nextjs.org/docs/basic-features/built-in-css-support.',
})
}
},
Expand Down
24 changes: 15 additions & 9 deletions packages/eslint-plugin-next/lib/rules/no-html-link-for-pages.js
Original file line number Diff line number Diff line change
@@ -1,10 +1,18 @@
const path = require('path')
const fs = require('fs')
const { getUrlFromPagesDirectory, normalizeURL } = require('../utils/url')
const {
getUrlFromPagesDirectory,
normalizeURL,
execOnce,
} = require('../utils/url')

const pagesDirWarning = execOnce((pagesDirs) => {
console.warn(
`Pages directory cannot be found at ${pagesDirs.join(' or ')}. ` +
`If using a custom path, please configure with the no-html-link-for-pages rule in your eslint config file`
)
})

//------------------------------------------------------------------------------
// Rule Definition
//------------------------------------------------------------------------------
module.exports = {
meta: {
docs: {
Expand All @@ -26,10 +34,8 @@ module.exports = {
]
const pagesDir = pagesDirs.find((dir) => fs.existsSync(dir))
if (!pagesDir) {
throw new Error(
`Pages directory cannot be found at ${pagesDirs.join(' or ')}. ` +
`If using a custom path, please configure with the no-html-link-for-pages rule`
)
pagesDirWarning(pagesDirs)
return {}
}

const urls = getUrlFromPagesDirectory('/', pagesDir)
Expand Down Expand Up @@ -61,7 +67,7 @@ module.exports = {
if (url.test(normalizeURL(hrefPath))) {
context.report({
node,
message: `You're using <a> tag to navigate to ${hrefPath}. Use Link from 'next/link' to make sure the app behaves like an SPA.`,
message: `You're using <a> tag to navigate to ${hrefPath}. Use Link from 'next/link' to make sure the app behaves like an SPA. See: https://nextjs.org/docs/api-reference/next/link.`,
})
}
})
Expand Down
2 changes: 1 addition & 1 deletion packages/eslint-plugin-next/lib/rules/no-sync-scripts.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ module.exports = function (context) {
context.report({
node,
message:
"A synchronous script tag can impact your webpage's performance",
"A synchronous script tag can impact your webpage's performance. See: https://web.dev/efficiently-load-third-party-javascript/.",
})
}
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ module.exports = {
if (unwantedFeatures.length > 0) {
context.report({
node,
message: `You're requesting polyfills from polyfill.io which are already shipped with NextJS. Please remove ${unwantedFeatures.join(
message: `You're requesting polyfills from polyfill.io which are already shipped with NextJS (see: https://nextjs.org/docs/basic-features/supported-browsers-features). Please remove ${unwantedFeatures.join(
', '
)} from the features list.`,
})
Expand Down
14 changes: 14 additions & 0 deletions packages/eslint-plugin-next/lib/utils/url.js
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,21 @@ function normalizeURL(url) {
return url
}

function execOnce(fn) {
let used = false
let result

return (...args) => {
if (!used) {
used = true
result = fn(...args)
}
return result
}
}

module.exports = {
getUrlFromPagesDirectory,
normalizeURL,
execOnce,
}
15 changes: 15 additions & 0 deletions packages/next/build/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import loadCustomRoutes, {
} from '../lib/load-custom-routes'
import { nonNullable } from '../lib/non-nullable'
import { recursiveDelete } from '../lib/recursive-delete'
import { verifyAndLint } from '../lib/verifyAndLint'
import { verifyTypeScriptSetup } from '../lib/verifyTypeScriptSetup'
import {
BUILD_ID_FILE,
Expand Down Expand Up @@ -191,6 +192,20 @@ export default async function build(
typeCheckingSpinner.stopAndPersist()
}

if (config.experimental.eslint) {
await nextBuildSpan
.traceChild('verify-and-lint')
.traceAsyncFn(async () => {
verifyAndLint(
Copy link
Member

Choose a reason for hiding this comment

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

This is not awaited, which means the trace will instantly resolve (0ms) instead of showing the timing.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fixed 👍

dir,
pagesDir,
config.experimental.cpus,
config.experimental.workerThreads,
!ignoreTypeScriptErrors
)
})
}

const buildSpinner = createSpinner({
prefixText: `${Log.prefixes.info} Creating an optimized production build`,
})
Expand Down
1 change: 1 addition & 0 deletions packages/next/lib/eslint/ESLintCompileError.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export class ESLintCompileError extends Error {}
1 change: 1 addition & 0 deletions packages/next/lib/eslint/ESLintFatalError.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export class ESLintFatalError extends Error {}
80 changes: 80 additions & 0 deletions packages/next/lib/eslint/customFormatter.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
import chalk from 'chalk'
import path from 'path'

export enum MessageSeverity {
Warning = 1,
Error = 2,
}

interface LintMessage {
ruleId: string | null
severity: 1 | 2
message: string
line: number
column: number
}

interface LintResult {
filePath: string
messages: LintMessage[]
errorCount: number
warningCount: number
output?: string
source?: string
}

function formatMessage(
dir: string,
messages: LintMessage[],
filePath: string
): string | void {
let fileName = path.posix.normalize(
path.relative(dir, filePath).replace(/\\/g, '/')
)

if (!fileName.startsWith('.')) {
fileName = './' + fileName
}

let output = '\n' + chalk.cyan(fileName)

for (let i = 0; i < messages.length; i++) {
const { message, severity, line, column, ruleId } = messages[i]

output = output + '\n'

if (line && column) {
output =
output +
chalk.yellow(line.toString()) +
':' +
chalk.yellow(column.toString()) +
' '
}

if (severity === MessageSeverity.Warning) {
output += chalk.yellow.bold('Warning') + ': '
} else {
output += chalk.red.bold('Error') + ': '
}

output += message

if (ruleId) {
output += ' ' + chalk.gray.bold(ruleId)
}
}

return output
}

export function formatResults(baseDir: string, results: LintResult[]): string {
return (
results
.filter(({ messages }) => messages?.length)
.map(({ messages, filePath }) =>
formatMessage(baseDir, messages, filePath)
)
.join('\n') + '\n'
)
}
32 changes: 32 additions & 0 deletions packages/next/lib/eslint/getLintIntent.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
import { promises as fs } from 'fs'

import * as CommentJson from 'next/dist/compiled/comment-json'

export type LintIntent = { firstTimeSetup: boolean }

export async function getLintIntent(
eslintrcFile: string | null,
pkgJsonEslintConfig: string | null
): Promise<LintIntent | false> {
if (eslintrcFile) {
const content = await fs.readFile(eslintrcFile, { encoding: 'utf8' }).then(
(txt) => txt.trim().replace(/\n/g, ''),
() => null
)

// User is setting up ESLint for the first time setup if eslint config exists but is empty
return {
firstTimeSetup:
content === '' ||
content === '{}' ||
content === '---' ||
content === 'module.exports = {}',
}
} else if (pkgJsonEslintConfig) {
return {
firstTimeSetup: CommentJson.stringify(pkgJsonEslintConfig) === '{}',
}
}

return false
}
Loading