Skip to content

Commit 25fcdd8

Browse files
authored
refactor: modernize ESLint configuration (#7080)
* refactor: modernize ESLint configuration This changeset migrates the project to ESLint 9 and to its new configuration format (`eslint.config.js`). To do this, I more or less ditched our current shared configuration in favor of using `recommended` presets from the plugins we use. We haven't been maintaining our ESLint preset since early 2022 and it uses long- outdated lint plugins (e.g. a version of the TypeScript ESLint plugin that was published in mid 2023). Even though this change will likely get rid of some rules we might find helpful and re-enable later, I think on the whole this is a win and a foundation for making ESLint a more useful tool going forward. A few other noteworthy changes here: - Cuts ESLint runtime from ~1 min to ~7s - This update makes a few extraneous dev dependencies explicit. A lot of our ESLint dependencies (eslint, plugins, prettier, etc.) were transitive dependencies from `@netlify/eslint-config-node`, which is a pattern notorious for causing package resolution issues. This change makes these dependencies explicit. - Replaced the lone custom ESLint rule (restricts the use of `process.cwd()`) with a built-in equivalent. - Updated the `tsconfig.json` configuration to use a more accurate configuration for our version of node. (We specified targets, `lib`s, etc. that were outdated for Node 18.) This change was prompted by some lint errors that made no sense. - Added the Vitest ESLint plugin. * build: add lint, lint:fix npm scripts These were added recently then reverted, so I'm adding them back here. * refactor: remove unnecessary typescript `include` glob * refactor: remove unnecessary eslint `ignores` glob * deps: add missing npm-run-all2 dependency Previously, this package was made available transitively--I think via `@netlify/eslint-config-node`. * ci: reduce indirection in lint CI step * build: remove cross-env-shell Not really sure why we even use this--we don't specify any environment variables to these tasks. * build: fix and de-complicate lint, format tasks * style: fix formatting errors
1 parent 0ca991e commit 25fcdd8

31 files changed

+1466
-8486
lines changed

.eslintignore

Lines changed: 0 additions & 3 deletions
This file was deleted.

.eslintrc.cjs

Lines changed: 0 additions & 148 deletions
This file was deleted.

.github/workflows/unit-tests.yml

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,9 +40,15 @@ jobs:
4040
- name: Install core dependencies
4141
run: npm ci --no-audit
4242
if: '${{!steps.release-check.outputs.IS_RELEASE}}'
43+
44+
- name: Formatting
45+
run: npm run format:check
46+
if: '${{!steps.release-check.outputs.IS_RELEASE}}'
47+
4348
- name: Linting
44-
run: npm run format:ci
49+
run: npm run lint
4550
if: '${{!steps.release-check.outputs.IS_RELEASE}}'
51+
4652
- name: Run unit tests
4753
uses: nick-fields/retry@v3
4854
with:

.prettierrc.json

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1,7 @@
1-
"@netlify/eslint-config-node/.prettierrc.json"
1+
{
2+
"semi": false,
3+
"singleQuote": true,
4+
"printWidth": 120,
5+
"proseWrap": "always",
6+
"trailingComma": "all"
7+
}

eslint.config.js

Lines changed: 163 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,163 @@
1+
// @ts-check
2+
import * as path from 'node:path'
3+
import { fileURLToPath } from 'node:url'
4+
import eslint from '@eslint/js'
5+
import { includeIgnoreFile } from '@eslint/compat'
6+
import prettier from 'eslint-config-prettier'
7+
import node from 'eslint-plugin-n'
8+
import vitest from '@vitest/eslint-plugin'
9+
import tseslint from 'typescript-eslint'
10+
11+
const __filename = fileURLToPath(import.meta.url)
12+
const __dirname = path.dirname(__filename)
13+
14+
export default tseslint.config(
15+
// Global rules and configuration
16+
includeIgnoreFile(path.resolve(__dirname, '.gitignore')),
17+
{
18+
linterOptions: {
19+
reportUnusedDisableDirectives: true,
20+
},
21+
},
22+
23+
// JavaScript-specific rules
24+
eslint.configs.recommended,
25+
26+
// Typescript-specific rules
27+
tseslint.configs.strictTypeChecked,
28+
tseslint.configs.stylisticTypeChecked,
29+
{
30+
languageOptions: {
31+
parserOptions: {
32+
projectService: true,
33+
tsconfigRootDir: __dirname,
34+
},
35+
},
36+
},
37+
38+
{
39+
files: ['**/*.?(c|m)js?(x)'],
40+
...tseslint.configs.disableTypeChecked,
41+
},
42+
node.configs['flat/recommended'],
43+
{
44+
rules: {
45+
'n/no-extraneous-import': 'off',
46+
'n/no-extraneous-require': 'off',
47+
'n/no-missing-import': 'off',
48+
'n/no-missing-require': 'off',
49+
'n/no-unpublished-import': 'off',
50+
'n/no-unpublished-require': 'off',
51+
},
52+
},
53+
54+
// Project-specific rules
55+
{
56+
ignores: ['.github/styles/', '**/__fixtures__/'],
57+
},
58+
{
59+
files: ['**/*.?(c|m)ts?(x)'],
60+
rules: {
61+
// `interface` and `type` have different use cases, allow both
62+
'@typescript-eslint/consistent-type-definitions': 'off',
63+
64+
// Ignore underscore-prefixed unused variables (mirrors tsc behavior)
65+
'@typescript-eslint/no-unused-vars': [
66+
'error',
67+
{
68+
args: 'all',
69+
argsIgnorePattern: '^_',
70+
caughtErrors: 'all',
71+
caughtErrorsIgnorePattern: '^_',
72+
destructuredArrayIgnorePattern: '^_',
73+
ignoreRestSiblings: true,
74+
varsIgnorePattern: '^_',
75+
},
76+
],
77+
},
78+
},
79+
{
80+
rules: {
81+
'no-restricted-imports': [
82+
'error',
83+
{
84+
paths: [
85+
{
86+
name: 'node:process',
87+
importNames: ['cwd'],
88+
message: 'Use `command.workingDir` instead.',
89+
},
90+
{
91+
name: 'process',
92+
importNames: ['cwd'],
93+
message: 'Use `command.workingDir` instead.',
94+
},
95+
96+
{
97+
name: 'chalk',
98+
message:
99+
'Use the safe chalk import that handles colors for json output: `import { chalk } from "src/utils/command-helpers.js"`',
100+
},
101+
],
102+
},
103+
],
104+
'no-restricted-properties': [
105+
'error',
106+
{
107+
object: 'process',
108+
property: 'cwd',
109+
message: '`process.cwd` is not permitted; use `command.workingDir` instead.',
110+
},
111+
],
112+
},
113+
},
114+
115+
// Tests
116+
{
117+
files: ['**/*.test.?(c|m)[jt]s?(x)'],
118+
plugins: { vitest },
119+
rules: {
120+
...vitest.configs.recommended.rules,
121+
122+
'vitest/expect-expect': [
123+
'error',
124+
{
125+
assertFunctionNames: [
126+
// Defaults
127+
'assert',
128+
'expect',
129+
130+
// Fix issue where text-context-specific `expect()` calls trigger false positive
131+
't.expect',
132+
'ctx.expect',
133+
'context.expect',
134+
135+
// Custom assertion functions
136+
'assertNetlifyToml',
137+
],
138+
},
139+
],
140+
},
141+
},
142+
143+
// XXX: Temporarily disabled rules: These rules are disabled because we have offending code that we haven't yet fixed.
144+
{
145+
rules: {
146+
// Empty functions and blocks are useful (e.g `noop() {}`, `catch {}`) but can mask unintentionally omitted
147+
// implementation. We should add explanatory comments like `// intentionally empty` and `// ignore error` in these
148+
// scenarios to communicate intent.
149+
'no-empty': 'off',
150+
'@typescript-eslint/no-empty-function': 'off',
151+
152+
'n/no-unsupported-features/node-builtins': [
153+
'error',
154+
{
155+
ignores: ['FormData', 'ReadableStream', 'Response', 'fetch', 'fs/promises.cp'],
156+
},
157+
],
158+
},
159+
},
160+
161+
// Must be last
162+
prettier,
163+
)

0 commit comments

Comments
 (0)