Thanks for the PR, @43081j!

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.

✅ Deploy Preview for typescript-eslint ready!

Name	Link
🔨 Latest commit	1ace444
🔍 Latest deploy log	https://app.netlify.com/sites/typescript-eslint/deploys/67fd6d226c3e8200088c1f61
😎 Deploy Preview	https://deploy-preview-10579--typescript-eslint.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.
Lighthouse	1 paths audited Performance: 77 (🔴 down 19 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 your Netlify site configuration.

View your CI Pipeline Execution ↗ for commit 1ace444.

☁️ Nx Cloud last updated this comment at 2025-04-14 20:33:29 UTC

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.82%. Comparing base (9531492) to head (1ace444).
Report is 1 commits behind head on main.

@@            Coverage Diff             @@
##             main   #10579      +/-   ##
==========================================
+ Coverage   84.31%   90.82%   +6.51%     
==========================================
  Files         497      497              
  Lines       27766    50192   +22426     
  Branches     5075     8267    +3192     
==========================================
+ Hits        23411    45588   +22177     
- Misses       4184     4589     +405     
+ Partials      171       15     -156     

This is awesome, thanks for sending @43081j! I'd really like to dive in soon and see if we can get this merged. But we're a little swamped catching up on issues after the holiday & new year break, and this is a lot of changes. So it'll probably be a little while till we can take a deep look. 😬

In the meantime, some notes:

• eslint-plugin is one of our biggest packages. Is there a reason you wanted to start with this, and not a smaller one like eslint-plugin-internal or type-utils?
• Codecov is reporting a significant % drop, which means coverage is probably not set up right with it - could you take a look please?
• Although we all would prefer to move away from implicit globals, adding in the import { describe, it, ... } from 'vitest' is adding a lot to the diff. Is there a reason not to enable Vitest globals for now - with a followup to remove it later?

eslint-plugin is one of our biggest packages. Is there a reason you wanted to start with this, and not a smaller one like eslint-plugin-internal or type-utils?

no particular reason, i just chose the package i was looking at at the time

Codecov is reporting a significant % drop, which means coverage is probably not set up right with it - could you take a look please?

ill have a look

Although we all would prefer to move away from implicit globals, adding in the import { describe, it, ... } from 'vitest' is adding a lot to the diff. Is there a reason not to enable Vitest globals for now - with a followup to remove it later?

we could use globals but i would say the diff is actually mostly from snapshot changes than it is from imports.

i think without the snapshot changes, it'd be a pretty small diff. so i'd prefer to keep them, but let me know if you don't

i manually compared the coverage locally and it seems to all be right. just vite reports more statements, and there's now a tools directory included which wasn't before

if you could have a look at some point too, that'd be helpful

i think without the snapshot changes, it'd be a pretty small diff. so i'd prefer to keep them, but let me know if you don't

+1 from me on the small diff 😄. I'd prefer not to keep them. The smaller the diff the better!

if you could have a look at some point too, that'd be helpful

👍 ACK, we can help as part of the review if it's not figured out

+1 from me on the small diff 😄. I'd prefer not to keep them. The smaller the diff the better!

what i meant was that the imports don't add much to the diff, since the only reason this is a "big PR" is the snapshot changes rather than those imports.

i'd rather keep the imports seeing as its where we want to be

if you diff without snapshots, its small (but of course we need those snapshot updates in this PR)

as for the coverage change, im pretty sure it is just vite doing a much better job by using v8 coverage. that and the added tools/ directory seem to account for the difference

+1 from me on having the smallest diff.

Since this is already a large change, having it be gradual is a big plus. I think that moving from Vitest globals to using imports can be done separately on a future PR, which would help not miss anything on this PR.

i think we're getting lost a bit here

the imports don't contribute much at all to the diff. most of the diff is from snapshot updates, maybe its worth taking another look

im happy to use globals but i think we're picking at a minor thing that actually doesn't contribute much to the diff

i've now switched to using globals

we need to re-open #10582 and land #10680 before this

Starting to see this once in a while:

Error: Test timed out in 5000ms.
If this is a long-running test, pass a timeout value as the last argument or configure it globally with "testTimeout".

https://cloud.nx.app/runs/EisURTJuXm/task/eslint-plugin%3Atest

Something to keep an eye on...