Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork2.8k
feat(typescript-estree): allow specifying project: true#6084
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
feat(typescript-estree): allow specifying project: true#6084
Uh oh!
There was an error while loading.Please reload this page.
Conversation
nx-cloudbot commentedNov 25, 2022 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
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 onhttps://opencollective.com/typescript-eslint. |
netlifybot commentedNov 25, 2022 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
✅ Deploy Preview fortypescript-eslint ready!
To edit notification comments on pull requests, go to yourNetlify site settings. |
Uh oh!
There was an error while loading.Please reload this page.
packages/typescript-estree/src/create-program/createWatchProgram.ts OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
bradzacher commentedNov 25, 2022 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
codecovbot commentedNov 25, 2022 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Codecov Report
Additional details and impacted files@@ Coverage Diff @@## main #6084 +/- ##======================================= Coverage 91.52% 91.52% ======================================= Files 371 372 +1 Lines 12651 12689 +38 Branches 3717 3730 +13 =======================================+ Hits 11579 11614 +35 Misses 754 754- Partials 318 321 +3
Flags with carried forward coverage won't be shown.Click here to find out more.
|
f461f16
toe3ef2ba
Comparee3ef2ba
toefec873
Comparepackages/typescript-estree/src/parseSettings/getProjectConfigFiles.ts OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
packages/typescript-estree/src/parseSettings/getProjectConfigFiles.ts OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
packages/typescript-estree/src/parseSettings/getProjectConfigFiles.ts OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
packages/typescript-estree/src/parseSettings/getProjectConfigFiles.ts OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
Co-authored-by: Brad Zacher <brad.zacher@gmail.com>
b86ce01
to9c17395
Compare9c17395
tof330e06
CompareJoshuaKGoldberg commentedFeb 4, 2023 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
ugh git |
packages/typescript-estree/src/parseSettings/createParseSettings.ts OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Overall this LGTM - so stamped
Before you land it - could you do a perf test just so we can update the docs with messaging around perf if we need to?
To test the perf I'd just use thetime
tool against main then against your branch:
git checkout main && yarn && time ./node_modules/.bin/eslint .
git checkout parser-options-project-true && yarn && time ./node_modules/.bin/eslint .
You'll see a line like this:
./node_modules/.bin/eslint . 62.97s user 4.09s system 167% cpu 40.009 total
The first number is the one to compare.
I'd do a couple runs of each just to smooth any outlier slowness.
I'd expect this to be a little slower due to the lookup required for every file - but I think that with the caching you've built in it shouldn't be MUCH slower. For a big, deeply nested project they might find it has more of an impact.
If the difference is minuscule then I don't think we need to worry about any messaging, but if it's different enough one way or the other it's probably worth mentioning!
But this is why it'd be worth a test - just to see if it is actually different enough.
If you wanted you could go hog-ham and investigate the proper perf trace before and after:
#6366 (comment)
The thing that would be interesting to see would be the cost of thecreateParseSettings
before and after as that's where your changes are.
Would be interesting to see the proper breakdown!
JoshuaKGoldberg commentedFeb 10, 2023 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
👌 |
Uh oh!
There was an error while loading.Please reload this page.
PR Checklist
Overview
Allows users to specify
parserOptions.project: true
to indicate each file should be linted with the closesttsconfig.json
to that file.tsconfig.json
files might be moved between lintstsconfig.json
to re-lint a currently visible file in an IDE. But as soon as the file is changed, we'll know to re-check its relativetsconfig.json
anyway.Throwing up a draft now to be very visible about progress.✅