typescript-eslint/typescript-eslintPublic

NotificationsYou must be signed in to change notification settings
Fork2.8k
Star15.7k

chore: clean up ESLint config post`vitest` migration#11135

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Jump to bottom

Merged

JoshuaKGoldberg merged 5 commits intotypescript-eslint:mainfromaryaemami59:chore/cleanup-eslint-config

May 16, 2025

Merged

chore: clean up ESLint config post`vitest` migration#11135

JoshuaKGoldberg merged 5 commits intotypescript-eslint:mainfromaryaemami59:chore/cleanup-eslint-config

May 16, 2025

+92 −67

Conversation

Copy link

Contributor

aryaemami59 commentedMay 5, 2025

PR Checklist

Addresses an existing open issue:fixesRepo: clean upeslint.config.mjs postvitest migration #11132
That issue was marked asaccepting prs
Steps inContributing were taken

Overview

chore: cleanup ESLint config file

89e0650

Copy link

Contributor

typescript-eslintbot commentedMay 5, 2025

Thanks for the PR,@aryaemami59!

typescript-eslint is a 100% community driven project, and we are incredibly grateful that you are contributing to that community.

The core maintainers work on this in their personal time, so please understand that it may not be possible for them to review your work immediately.

Thanks again!

🙏Please, if you or your company is finding typescript-eslint valuable, help us sustain the project by sponsoring it transparently on https://opencollective.com/typescript-eslint.

Copy link

netlifybot commentedMay 5, 2025•
edited
Loading

✅ Deploy Preview fortypescript-eslint ready!

Name	Link
🔨 Latest commit	`1551966`
🔍 Latest deploy log	https://app.netlify.com/projects/typescript-eslint/deploys/6825fffa27af400008951b63
😎 Deploy Preview	https://deploy-preview-11135--typescript-eslint.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.
Lighthouse	1 paths audited Performance: 98 (🔴 down 1 from production) Accessibility: 100 (no change from production) Best Practices: 100 (no change from production) SEO: 98 (no change from production) PWA: 80 (no change from production) View the detailed breakdown and full score reports

To edit notification comments on pull requests, go to yourNetlify project configuration.

Copy link

nx-cloudbot commentedMay 5, 2025•
edited
Loading

View yourCI Pipeline Execution ↗ for commit1551966.

Command	Status	Duration	Result
`nx test eslint-plugin`	✅ Succeeded	6m 24s	View ↗
`nx run eslint-plugin:test -- --coverage`	✅ Succeeded	6m 38s	View ↗
`nx test eslint-plugin --coverage=false`	✅ Succeeded	5m 19s	View ↗
`nx test type-utils`	✅ Succeeded	24s	View ↗
`nx test eslint-plugin-internal`	✅ Succeeded	11s	View ↗
`nx test visitor-keys`	✅ Succeeded	3s	View ↗
`nx run types:build`	✅ Succeeded	1s	View ↗
`nx test typescript-eslint`	✅ Succeeded	5s	View ↗
`Additional runs (26)`	✅ Succeeded	...	View ↗

☁️Nx Cloud last updated this comment at2025-05-15 15:08:44 UTC

aryaemami59 marked this pull request as ready for review

May 5, 2025 10:29

aryaemami59 added2 commits

May 5, 2025 05:35

removejiti

5938d9b

Merge branch 'main' ofhttps://github.com/typescript-eslint/typescrip…

20b2c16

…t-eslint into chore/cleanup-eslint-config

Copy link

codecovbot commentedMay 7, 2025•
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.92%. Comparing base(af077a0) to head(1551966).
Report is 2 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@##             main   #11135   +/-   ##=======================================  Coverage   90.92%   90.92%           =======================================  Files         499      499             Lines       50845    50845             Branches     8384     8384           =======================================  Hits        46231    46231             Misses       4599     4599             Partials       15       15

Flag	Coverage Δ
unittest	`90.92% <ø> (ø)`

Flags with carried forward coverage won't be shown.Click here to find out more.

🚀 New features to boost your workflow:

❄️Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Merge branch 'main' ofhttps://github.com/typescript-eslint/typescrip…

9c57e2d

…t-eslint into chore/cleanup-eslint-config

JoshuaKGoldberg approved these changes

May 12, 2025

View reviewed changes

Copy link

Member

JoshuaKGoldberg left a comment

Choose a reason for hiding this comment

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

Thanks!

JoshuaKGoldberg added the 1 approval>=1 team member has approved this PR; we're now leaving it open for more reviews before we merge label

May 12, 2025

Merge branch 'main' ofhttps://github.com/typescript-eslint/typescrip…

1551966

…t-eslint into chore/cleanup-eslint-config

JoshuaKGoldberg merged commitff2a785 intotypescript-eslint:main

May 16, 2025

59 checks passed

Copy link

Member

JamesHenry commentedMay 16, 2025

nx config wise this is a very confusing change tbh.

Please can you share your reasoning?

JamesHenry mentioned this pull request

May 16, 2025

chore: remove postinstall steps from CI to speed up prepare-install action#11219

Open

3 tasks

Copy link

ContributorAuthor

aryaemami59 commentedMay 16, 2025

@JamesHenry why is it confusing? Or rather which part is confusing? I'm more than happy to elaborate.

Copy link

Member

JamesHenry commentedMay 16, 2025

Why did you add the eslint plugin here? Only to then immediately remove it again herechore: update some package scripts and dependency config #10765
Executors and inferred plugins are two different approaches to the same problem, in this PR you are simultaneously increasing the usage of both
Why are you specifying that all usage of a specific executor depends upon specific tasks ("@nx/eslint:lint": { "dependsOn": ["eslint-plugin-internal:build", "typescript-eslint:build"]), that is a very unusual thing to do, and I don't think it's accurate here

The mixed usage and application and then seeming upcoming undoing of things is why I wanted to understand the broader reasoning here.

I think it's best if a publish a video on these concepts to bring us on the same page

Copy link

ContributorAuthor

aryaemami59 commentedMay 16, 2025•
edited
Loading

@JamesHenry

Why did you add the eslint plugin here? Only to then immediately remove it again herechore: fixnx usage with lifecycle scripts across all projects #10765

That was actually done by mistake I just fixed it.

aryaemami59 commented

May 16, 2025

View reviewed changes

nx.json

Comment on lines +32 to +38

		},
		{
		"plugin": "@nx/eslint/plugin",
		"include": ["packages/*"],
		"options": {
		"targetName": "lint"
		}

Copy link

ContributorAuthor

aryaemami59May 16, 2025

Choose a reason for hiding this comment

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

Reference

aryaemami59 commented

May 16, 2025

View reviewed changes

nx.json

Comment on lines +92 to +101

		"@nx/eslint:lint": {
		"dependsOn": ["eslint-plugin-internal:build", "typescript-eslint:build"],
		"options": {
		"noEslintrc": true,
		"cache": true,
		"eslintConfig": "{workspaceRoot}/eslint.config.mjs"
		},
		"outputs": ["{options.outputFile}"],
		"cache": true
		},