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

Enhance ESLint config #6357

Merged
merged 8 commits into from
Jul 24, 2024
Merged

Enhance ESLint config #6357

merged 8 commits into from
Jul 24, 2024

Conversation

clemyan
Copy link
Member

@clemyan clemyan commented Jun 26, 2024

What's the problem this PR addresses?

Follow up to #6276 and #6352. The ESLint config has room for improvement

How did you fix it?

Wide include

The culprit is this line

files: [`!packages/*/sources/{index,Plugin}.ts`],

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

  • I have set the packages that need to be released for my changes to be effective.
  • I will check that all automated PR checks pass before the PR gets reviewed.

@arcanis arcanis merged commit f93437d into master Jul 24, 2024
22 of 26 checks passed
@arcanis arcanis deleted the clemyan/eslint branch July 24, 2024 13:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants