Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork2.8k
fix(typescript-estree): don't consider a cached program unless it's specified in the currentparserOptions.project config#5999
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
Uh oh!
There was an error while loading.Please reload this page.
Conversation
…pecified in the current `parserOptions.project` config
Thanks for the PR,@bradzacher! 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. |
nx-cloudbot commentedNov 16, 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.
netlifybot commentedNov 16, 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. |
| @@ -1,14 +1,11 @@ | |||
| import path from 'path'; | |||
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.
the changes here were extracted from#5916 as an easy test for this behaviour.
could add a regression test against this, but I didn't bother because ultimately it's such a rare case.
codecovbot commentedNov 16, 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 #5999 +/- ##======================================= Coverage 91.42% 91.42% ======================================= Files 365 365 Lines 12339 12342 +3 Branches 3609 3610 +1 =======================================+ Hits 11281 11284 +3 Misses 753 753 Partials 305 305
Flags with carried forward coverage won't be shown.Click here to find out more.
|
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.
Nice find!
| // so we don't want to consider it for caching purposes. | ||
| // | ||
| // if we did consider it we might return a program for a project | ||
| // that wasn't specified in the current parser run (which is obv bad!). |
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.
To you, maybe 😛
| // that wasn't specified in the current parser run (which isobvbad!). | |
| // that wasn't specified in the current parser run (which is bad!). |
PR Checklist
Overview
Discovered this while working on#5916
Previously we would consider any cached program as fair game during parsing.
Most of the time this isn't a problem because users always have the same config for an entire lint run.
However if the user has a different value for
parserOptions.projectaround the place, then we would behave incorrectly - we would iterate through our cached programs and return a matching program for a file even if its tsconfig wasn't included in the currentparserOptions.project! This is really bad because it means we'll lint a file with probably incorrect types!!!I discovered this in the above PR because I defined two separate test runs with different tsconfigs in the same file.
Because our rule tester uses the exact same filename, the parser checked the cache, found the old program matched, and returned it - meaning we never used the new tsconfig (causing the test to fail!)
The fix is simply to enforce that we only consider a program from the cache if and only if its tsconfig exists in the current
parserOptions.projectset.