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

feat(typescript-estree): add maximumDefaultProjectFileMatchCount and wide allowDefaultProjectForFiles glob restrictions #8925

Conversation

JoshuaKGoldberg
Copy link
Member

@JoshuaKGoldberg JoshuaKGoldberg commented Apr 15, 2024

PR Checklist

Overview

Adds the two safety checks for allowDefaultProjectForFiles as suggested in the issue:

  • A new maximumDefaultProjectFileMatchCount_THIS_WILL_SLOW_DOWN_LINTING option with a default value of 8
  • A check that allowDefaultProjectForFiles doesn't include **
    • Intentionally allows slightly more permissive matches like ./*.*s, as that's what I'd expect to be convenient & reasonable alongside the count maximum

Adds an FAQs blog post to explain them.

💖

@typescript-eslint
Copy link
Contributor

Thanks for the PR, @JoshuaKGoldberg!

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

netlify bot commented Apr 15, 2024

Deploy Preview for typescript-eslint ready!

Name Link
🔨 Latest commit d00856e
🔍 Latest deploy log https://app.netlify.com/sites/typescript-eslint/deploys/661d4750f3f695000868734c
😎 Deploy Preview https://deploy-preview-8925--typescript-eslint.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
Lighthouse
Lighthouse
1 paths audited
Performance: 98 (🔴 down 1 from production)
Accessibility: 100 (no change from production)
Best Practices: 92 (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.

Copy link
Member Author

Choose a reason for hiding this comment

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

Aside: filed #8926 to reduce the scale of this kind of huge FAQs list.

@@ -499,6 +533,6 @@ If you think you're having issues with performance, see our [Performance Trouble

## Are TypeScript project references supported?

No, TypeScript project references are not yet supported.
Yes, but only with [`EXPERIMENTAL_useProjectService`](../packages/Parser.mdx#experimental_useprojectservice).
Copy link
Member Author

Choose a reason for hiding this comment

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

Aside: we'll definitely want more explanation here 😄 but quickly noted this in the meantime. Once this & a few more v8 milestone PRs are in, #7350 tracks a full blog post / explainer / revamp.

@JoshuaKGoldberg JoshuaKGoldberg added this to the 8.0.0 milestone Apr 15, 2024
@JoshuaKGoldberg JoshuaKGoldberg marked this pull request as ready for review April 15, 2024 15:54
@JoshuaKGoldberg JoshuaKGoldberg merged commit 3fd666f into typescript-eslint:main Apr 25, 2024
59 checks passed
@me4502
Copy link

me4502 commented Apr 30, 2024

Is this meant to impact situations where EXPERIMENTAL_useProjectService is set to true? Not sure if it's a bug (will make an issue if it is), but I'm now seeing Parsing error: Too many files (>8) have matched the default project. for every single file in each workspaces' lint run as of 7.8.0.

From my understanding the project service should be loading the closest tsconfig.json in the file tree, and using the default if it doesn't match an included file, correct? It seems to be either ignoring the tsconfig.json file per workspace or ignoring the includes values in each one.

@JoshuaKGoldberg
Copy link
Member Author

Hmm, every single file hitting this does sound like a bug. I'd appreciate an issue - yes please!

@JoshuaKGoldberg JoshuaKGoldberg deleted the maximumDefaultProjectFileMatchCount_THIS_WILL_SLOW_DOWN_LINTING branch April 30, 2024 05:13
@me4502
Copy link

me4502 commented Apr 30, 2024

Will do, thanks. I haven't managed to reproduce this outside of one specific large & complex monorepo, so I'll see if I can narrow it down to reproduce.

@ehoogeveen-medweb
Copy link
Contributor

I'm also seeing this match every file when linting a directory containing a project config, even after setting allowDefaultProjectForFiles: []. I'll try to get a minimal reproduction.

@ehoogeveen-medweb
Copy link
Contributor

Here's a simple reproduction:

  1. Clone https://github.com/ehoogeveen-medweb/glob-too-wide.git
  2. Run npm ci
  3. Run npx eslint --ext ts .

You should see something like this:

path/to/foo08.ts
  0:0  error  Parsing error: Too many files (>8) have matched the default project.

Having many files run with the default project is known to cause performance issues and slow down linting.

See https://typescript-eslint.io/troubleshooting/#allowdefaultprojectforfiles-glob-too-wide

Matching files:
- path/to/foo00.ts
- path/to/foo01.ts
- path/to/foo02.ts
- path/to/foo03.ts
- path/to/foo04.ts
- path/to/foo05.ts
- path/to/foo06.ts
- path/to/foo07.ts
- path/to/foo08.ts

If you absolutely need more files included, set parserOptions.EXPERIMENTAL_useProjectService.maximumDefaultProjectFileMatchCount_THIS_WILL_SLOW_DOWN_LINTING to a larger value

✖ 1 problem (1 error, 0 warnings)

@stepankuzmin
Copy link

stepankuzmin commented May 4, 2024

Having the same issue with Too many files (>8) have matched the default project after upgrading to 7.8.0.

Setting maximumDefaultProjectFileMatchCount_THIS_WILL_SLOW_DOWN_LINTING to a large number helps.

@JoshuaKGoldberg
Copy link
Member Author

JoshuaKGoldberg commented May 7, 2024

Heyall, per our PR contributing guide please don't post on resolved PRs. The comments tend to get lost and this isn't an effective way to discuss issues. Please file a new issue with the appropriate template if you've got something to report that hasn't yet been reported.

#9032 tracks the general case of this error. Nobody's investigated to see what the actual issue is (if you have time, please do!). That would be the issue to subscribe to for updates. If you've got any new information to add such as investigation results or a new kind of reproduction, please add it there. Saying "I also have this issue!" doesn't give us any information and just gives an unnecessary notification to everyone in the thread.

maximumDefaultProjectFileMatchCount_THIS_WILL_SLOW_DOWN_LINTING is appropriately obnoxiously named. It's a "bandaid" solution to files being unmatched by the project service. A better solution is to figure out why they're unmatched and fix that bug - which is either in the ESLint config being used or in typescript-eslint.

Thanks for trying out the new project service though! Very exciting to see folks using it & giving feedback. Even if it's a bug on our end. 😄 ❤️

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 17, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Enhancement: ban wide globs in parserOptions.useProjectService.allowDefaultProjectForFiles
5 participants