Migrates to Eslint's flat config #6276
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Draft until #6219 is fixed. |
clemyan
reviewed
May 7, 2024
eslint.config.mjs
Outdated
Comment on lines
9
to
41
| ignores: [ | ||
| `.yarn`, | ||
|
|
||
| `packages/docusaurus/.docusaurus`, | ||
|
|
||
| `packages/yarnpkg-pnp/sources/hook.js`, | ||
| `packages/yarnpkg-pnp/sources/esm-loader/built-loader.js`, | ||
|
|
||
| `packages/yarnpkg-libzip/sources/libzipAsync.js`, | ||
| `packages/yarnpkg-libzip/sources/libzipSync.js`, | ||
|
|
||
| // The parsers are auto-generated | ||
| `packages/yarnpkg-parsers/sources/grammars/*.js`, | ||
|
|
||
| `packages/yarnpkg-core/sources/worker-zip/index.js`, | ||
| `packages/yarnpkg-cli/bundles/yarn.js`, | ||
|
|
||
| `packages/*/lib`, | ||
| `packages/*/bin`, | ||
| `packages/*/build`, | ||
| `packages/**/*fixtures*`, | ||
| `packages/yarnpkg-libzip/artifacts`, | ||
| `packages/plugin-compat/extra/fsevents/fsevents-*.js`, | ||
|
|
||
| // Files generated by Gatsby | ||
| `packages/gatsby/public`, | ||
| // Files generated by TypeDoc`, | ||
| `packages/gatsby/static/api`, | ||
|
|
||
| // Minimize the diff with upstream`, | ||
| `packages/yarnpkg-pnp/sources/node`, | ||
| `packages/yarnpkg-pnp/sources/loader/node-options*` | ||
| ], |
There was a problem hiding this comment.
- The gatsby rules should be safe to remove
- Is
yarnpkg-clithe only workspace to generate abundledirectory? Because other places just ignorespackages/*/bundle - Should the
yarnpkg-libuiartifacts also be here? - Should the these be grouped by package?
There was a problem hiding this comment.
I removed Gatsby, but other changes will be for another PR (I'd like to keep the changes here as minimal as possible).
|
FYI, ESLint recently released the |
## What's the problem this PR addresses? ESM loaders aren't supported by the sdks because we only `require().setup()` but we don't `register()` the ESM loader hook. Fixes #6219 ## How did you fix it? - Add support for ESM loaders to the SDK. - Check whether the `findPnpApi` function could be found in the `module` builtin. If not, tries to setup the environment accordingly. This is because in the case of the SDK, the subprocesses have been created without the `--require .pnp.loader.mjs` flag, so the PnP runtime won't be setup. ## 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.
|
@arcanis Should this have removed https://github.com/yarnpkg/berry/blob/d3695b1834f702675063c5d3ae45f0af6d8d3fa8/packages/acceptance-tests/.eslintrc.js as well? |
arcanis
added a commit
that referenced
this pull request
Jul 24, 2024
## 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 <nison.mael@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What's the problem this PR addresses?
This lets us remove the Rushstack fix, and dogfood the VSCode SDK with the flat config (which is the only available config in Eslint 9). I would have migrated us to Eslint 9, but eslint-plugin-react isn't compatible yet.
How did you fix it?
Migrate to eslint.config.mjs.
Checklist