-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
fix(typescript-estree): update conditions for unsupported version warning #10385
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
fix(typescript-estree): update conditions for unsupported version warning #10385
Conversation
Thanks for the PR, @inga-lovinde! 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!
To edit notification comments on pull requests, go to your Netlify site configuration. |
☁️ Nx Cloud ReportCI is running/has finished running commands for commit cd99119. As they complete they will appear below. Click to see the status, the terminal output, and the build insights. 📂 See all runs for this CI Pipeline Execution ✅ Successfully ran 2 targetsSent with 💌 from NxCloud. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #10385 +/- ##
=======================================
Coverage 86.75% 86.75%
=======================================
Files 443 443
Lines 15289 15289
Branches 4446 4446
=======================================
Hits 13264 13264
Misses 1672 1672
Partials 353 353
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.
💡 Aha! Thank you for the catch. Clearly this file keeps flying under the radar. Much appreciated @inga-lovinde!
@JoshuaKGoldberg , since this file has to be maintained manually, maybe there could be a test catching the inconsistency between the main package.json and the range defined in That would require updating it to expose |
That's a good idea, I like it. Would you be open to filing a followup issue for that please? 🙏 I ask because we're going to publish our weekly major in about 15 hours and I don't know that we'd be able to get a nontrivial test like that approved in time. |
I can try to open an issue tomorrow (it's 3 AM here right now), maybe also create a PR even, after this one is merged :) |
test for this: #10386 |
PR Checklist
Overview
I don't really know what I'm doing (this is my first PR to this repository), but I think that in #10372 , the update to version list in
typescript-estree
might have slipped through,typescript-estree
was still declaring that it only supports TS versions below 5.7.0 (so users of TS 5.7 are currently still getting warnings about unsupported TS version even on the latest typescript-eslint at 8.15.1-alpha.6).Seeing how for previous TS versions, const SUPPORTED_TYPESCRIPT_VERSIONS was updated in the same "feat: support TypeScript X.Y" pull requests as everything else, it seems that it should have went there, but it didn't, so I'm creating a separate PR for it.
I also took a liberty of upgrading the lowest supported version from 4.7.4 to 4.8.4, because the comment says "This needs to be kept in sync with package.json in the typescript-eslint monorepo", and lowest supported
typescript
version in package.json in the typescript-eslint monorepo was updated from 4.7.4 to 4.8.4 in #8973 but didn't make its way towarnAboutTSVersion.ts
.