Skip to content

Commit f93437d

Browse files
clemyanarcanis
andauthored
Enhance ESLint config (#6357)
## What's the problem this PR addresses? Follow up to #6276 and #6352. The ESLint config has room for improvement - The include patterns are unnecessary wide and causes a lot more files to be linted by default (See #6352, which is only a partial fix) - The ignore patterns can be simplified (#6276 (comment)) - There are some extraneous config files and config items - The `@typescript-eslint/naming-convention` rule config in `@yarnpkg/eslint-config` is shadowed by the one in `eslint.config.mjs` ## How did you fix it? ### Wide include The culprit is this line https://github.com/yarnpkg/berry/blob/57ed709ceea1f45568f860729cc18ae324334289/eslint.config.mjs#L43 By only specifying a negated pattern, it matches **ALL** other files that are not globally ignored. Under flat config, each file matched by at least one config item (and is not globally ignored) is eligible for linting. This causes the VSCode plugin to attempt to lint pretty much every file. The correct way to use both `files` and `ignores` to accurately specify the set of files to be linted. With that, we can simplify the `test:lint` command to just lint the whole repo and we have a single source of truth of what should be linted. This also aligns with the [default behavior of ESLint v9](https://eslint.org/blog/2024/04/eslint-v9.0.0-released/#running-eslint-with-no-file-arguments) for when we migrate. I have also taken the opportunity to widen the typescript files globs to include `.cts` and `.mts` if we create those in the future. ### Ignore patterns See #6276 (comment). Added a few missed ignores. Also grouped them by workspace. ### Extraneous configs See #6276 (comment). There are two sets of configs that specify jest globals for tests using `env` but that is not supported in flat config and has already been migrated in #6276. ### `@typescript-eslint/naming-convention` The `@typescript-eslint/naming-convention` is configured twice, once in `@yarnpkg/eslint-config` and once in `eslint.config.mjs`. But due to a wide include glob in the latter, the former one is actually almost entirely shadowed -- it only takes effect on `sources/index.ts` and `sources/Plugin.ts` of each workspace. There are already ~100 violations of the rule that went unreported due to this shadowing. I have removed the shadowed config item since it isn't really being enforced. (Or is that actually intentional?) ## Checklist <!--- Don't worry if you miss something, chores are automatically tested. --> <!--- This checklist exists to help you remember doing the chores when you submit a PR. --> <!--- Put an `x` in all the boxes that apply. --> - [x] I have read the [Contributing Guide](https://yarnpkg.com/advanced/contributing). <!-- See https://yarnpkg.com/advanced/contributing#preparing-your-pr-to-be-released for more details. --> <!-- Check with `yarn version check` and fix with `yarn version check -i` --> - [x] I have set the packages that need to be released for my changes to be effective. <!-- The "Testing chores" workflow validates that your PR follows our guidelines. --> <!-- If it doesn't pass, click on it to see details as to what your PR might be missing. --> - [x] I will check that all automated PR checks pass before the PR gets reviewed. --------- Co-authored-by: Maël Nison <[email protected]>
1 parent cae85b3 commit f93437d

File tree

73 files changed

+1472
-133
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

73 files changed

+1472
-133
lines changed

.pnp.cjs

Lines changed: 750 additions & 54 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.

.yarn/versions/a712705e.yml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
releases:
2+
"@yarnpkg/eslint-config": minor

eslint.config.mjs

Lines changed: 39 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -3,44 +3,61 @@ import eslintConfig from '@yarnpkg/eslint-config';
33

44
// eslint-disable-next-line arca/no-default-export
55
export default [
6-
...eslintConfig,
7-
...reactEslintConfig,
8-
96
{
107
ignores: [
11-
`*.json`,
128
`**/coverage/**`,
9+
`**/__snapshots__/**`,
1310
`.yarn`,
11+
`.pnp.cjs`,
12+
`.pnp.loader.mjs`,
1413

15-
`packages/docusaurus/.docusaurus`,
16-
17-
`packages/yarnpkg-pnp/sources/hook.js`,
18-
`packages/yarnpkg-pnp/sources/esm-loader/built-loader.js`,
14+
// Pre-compiled binaries
15+
`packages/*/lib`,
16+
`packages/*/bin`,
17+
`packages/*/build`,
18+
`packages/*/bundles`,
1919

20-
`packages/yarnpkg-libzip/sources/libzipAsync.js`,
21-
`packages/yarnpkg-libzip/sources/libzipSync.js`,
20+
// Test fixtures
21+
`packages/**/*fixtures*`,
2222

23-
// The parsers are auto-generated
24-
`packages/yarnpkg-parsers/sources/grammars/*.js`,
23+
// Generated files for website
24+
`packages/docusaurus/.docusaurus`,
2525

26+
// Generated compressed worker
2627
`packages/yarnpkg-core/sources/worker-zip/index.js`,
27-
`packages/yarnpkg-cli/bundles/yarn.js`,
2828

29-
`packages/*/lib`,
30-
`packages/*/bin`,
31-
`packages/*/build`,
32-
`packages/**/*fixtures*`,
29+
// Build output for libui
30+
`packages/yarnpkg-libui/sources/**/*.js`,
31+
`packages/yarnpkg-libui/sources/**/*.d.ts`,
32+
33+
// Pre-compiled from C sources
34+
`packages/yarnpkg-libzip/sources/libzipAsync.js`,
35+
`packages/yarnpkg-libzip/sources/libzipSync.js`,
36+
// The C sources themselves
3337
`packages/yarnpkg-libzip/artifacts`,
34-
`packages/plugin-compat/extra/fsevents/fsevents-*.js`,
3538

36-
// Minimize the diff with upstream`,
39+
// Generated compressed hooks
40+
`packages/yarnpkg-pnp/sources/hook.js`,
41+
`packages/yarnpkg-pnp/sources/esm-loader/built-loader.js`,
42+
// Minimize the diff with upstream
3743
`packages/yarnpkg-pnp/sources/node`,
3844
`packages/yarnpkg-pnp/sources/loader/node-options*`,
45+
46+
// Generated PEG.js grammars
47+
`packages/yarnpkg-parsers/sources/grammars/*.js`,
48+
49+
// Patched fsevents
50+
`packages/plugin-compat/extra/fsevents/fsevents-*.js`,
3951
],
4052
},
4153

54+
...eslintConfig,
55+
...reactEslintConfig,
56+
4257
{
43-
files: [`!packages/*/sources/{index,Plugin}.ts`],
58+
name: `berry/naming-convention`,
59+
files: [`**/*.ts`, `**/*.cts`, `**/*.mts`, `**/*.tsx`],
60+
ignores: [`packages/*/sources/{index,Plugin}.ts`],
4461
rules: {
4562
'@typescript-eslint/naming-convention': [`error`, {
4663
selector: `typeLike`,
@@ -54,6 +71,7 @@ export default [
5471
},
5572

5673
{
74+
name: `berry/env/acceptance-tests`,
5775
files: [`packages/acceptance-tests/pkg-tests-specs/**/*.test.{js,ts}`],
5876
languageOptions: {
5977
globals: {
@@ -64,6 +82,7 @@ export default [
6482
},
6583

6684
{
85+
name: `berry/rules`,
6786
rules: {
6887
'no-restricted-properties': [2,
6988
{

package.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
],
88
"devDependencies": {
99
"@arcanis/sherlock": "^2.0.3",
10+
"@eslint/config-inspector": "^0.5.0",
1011
"@types/jest": "^28.1.6",
1112
"@types/micromatch": "^4.0.1",
1213
"@types/node": "^18.17.15",
@@ -54,7 +55,7 @@
5455
"build:plugin-workspace-tools": "yarn node -r ./scripts/setup-ts-execution ./scripts/create-mock-plugin.ts workspace-tools",
5556
"build:plugin-commands": "node ./scripts/gen-plugin-commands.js > packages/yarnpkg-cli/sources/pluginCommands.ts",
5657
"build:compile": "rm -rf \"$0\"/lib && mkdir -p \"$0\"/lib && rsync -a --include '*.d.ts' --exclude '*.ts' --exclude '*.tsx' \"$0\"/sources/ \"$0\"/lib/ && node scripts/compile \"$@\"",
57-
"test:lint": "eslint --max-warnings 0 \"./**/*.@(tsx|ts|js|mjs)\"",
58+
"test:lint": "eslint --max-warnings 0 .",
5859
"test:unit": "jest",
5960
"typecheck:all": "tsc -p ."
6061
},

packages/acceptance-tests/.eslintrc.js

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

packages/eslint-config/index.js

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -8,28 +8,35 @@ import typescript from './rules/typescript.js';
88

99
// eslint-disable-next-line arca/no-default-export
1010
export default [
11-
...bestPractices,
12-
...errors,
13-
...style,
14-
...typescript,
15-
1611
{
12+
name: `@yarnpkg/setup`,
1713
languageOptions: {
1814
parser: tsParser,
1915
sourceType: `module`,
16+
},
17+
},
18+
{
19+
name: `@yarnpkg/env`,
20+
languageOptions: {
2021
globals: {
2122
...globals.node,
2223
...globals.es2021,
2324
},
2425
},
2526
},
26-
2727
{
28+
name: `@yarnpkg/env/tests`,
2829
files: [`**/*.test.*`],
30+
ignores: [`**/__snapshots__/**`],
2931
languageOptions: {
3032
globals: {
3133
...globals.jest,
3234
},
3335
},
3436
},
37+
38+
...bestPractices,
39+
...errors,
40+
...style,
41+
...typescript,
3542
];

packages/eslint-config/rules/best-practices.js

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@ import arcaEslint from 'eslint-plugin-arca';
44
// eslint-disable-next-line arca/no-default-export
55
export default [
66
{
7+
name: `@yarnpkg/configs/best-practices`,
8+
79
plugins: {
810
[`arca`]: arcaEslint,
911
[`@typescript-eslint`]: typescriptEslint,

packages/eslint-config/rules/errors.js

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@ import arcaEslint from 'eslint-plugin-arca';
44
// eslint-disable-next-line arca/no-default-export
55
export default [
66
{
7+
name: `@yarnpkg/configs/errors`,
8+
79
plugins: {
810
[`arca`]: arcaEslint,
911
[`@typescript-eslint`]: typescriptEslint,
@@ -73,10 +75,4 @@ export default [
7375
'valid-typeof': 2,
7476
},
7577
},
76-
{
77-
files: [`*.test.{js,ts}`],
78-
env: {
79-
jest: true,
80-
},
81-
},
8278
];

packages/eslint-config/rules/react.js

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@ import reactEslint from 'eslint-plugin-react';
44
// eslint-disable-next-line arca/no-default-export
55
export default [
66
{
7+
name: `@yarnpkg/configs/react`,
8+
79
plugins: {
810
[`arca`]: arcaEslint,
911
[`react`]: reactEslint,

packages/eslint-config/rules/style.js

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@ import arcaEslint from 'eslint-plugin-arca';
44
// eslint-disable-next-line arca/no-default-export
55
export default [
66
{
7+
name: `@yarnpkg/configs/style`,
8+
79
plugins: {
810
[`arca`]: arcaEslint,
911
[`@typescript-eslint`]: typescriptEslint,
@@ -22,16 +24,6 @@ export default [
2224

2325
'@typescript-eslint/comma-spacing': 2,
2426

25-
'@typescript-eslint/naming-convention': [`error`, {
26-
selector: `default`,
27-
format: [`camelCase`, `UPPER_CASE`, `PascalCase`],
28-
filter: {
29-
regex: `^(__.*|__non_webpack_require__|npm(_[a-z]+)+)$`,
30-
match: false,
31-
},
32-
leadingUnderscore: `allow`,
33-
}],
34-
3527
'@typescript-eslint/func-call-spacing': 2,
3628

3729
'@typescript-eslint/indent': [`error`, 2, {

packages/eslint-config/rules/typescript.js

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,9 @@ import typescriptEslint from '@typescript-eslint/eslint-plugin';
33
// eslint-disable-next-line arca/no-default-export
44
export default [
55
{
6-
files: [`**/*.{ts,tsx}`],
6+
name: `@yarnpkg/configs/typescript`,
7+
8+
files: [`**/*.ts`, `**/*.cts`, `**/*.mts`, `**/*.tsx`],
79

810
plugins: {
911
[`@typescript-eslint`]: typescriptEslint,

0 commit comments

Comments
 (0)