feat: update to lib types for TS 5.2 RC #7451
Conversation
|
Thanks for the PR, @sosukesuzuki! 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. |
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #7451 +/- ##
==========================================
- Coverage 87.34% 85.25% -2.10%
==========================================
Files 381 386 +5
Lines 13336 13358 +22
Branches 3943 3943
==========================================
- Hits 11648 11388 -260
- Misses 1308 1593 +285
+ Partials 380 377 -3
Flags with carried forward coverage won't be shown. Click here to find out more.
|
4d0ba91 to
5cfbf54
Compare
| @@ -20,6 +20,8 @@ const versions = [ | |||
| '4.8', | |||
| '4.9', | |||
| '5.0', | |||
| '5.1', | |||
| '5.2', | |||
There was a problem hiding this comment.
...huh. Odd that 5.1 wasn't here before.
| "types": ["jest", "node"] | ||
| "module": "Node16", | ||
| "types": ["jest", "node"], | ||
| "moduleResolution": "Node16" |
There was a problem hiding this comment.
Why this change? Tests still work for me locally after reverting it.
There was a problem hiding this comment.
But does it typecheck fine without it?
There's a new error in this version to ensure that things are configured correctly.
There was a problem hiding this comment.
Yeah I like moving to the newer module & module resolution settings, just curious.
Also, per https://www.typescriptlang.org/tsconfig#moduleResolution, moduleResolution matches module by default with node16 and other newer targets.
There was a problem hiding this comment.
Without this change, a type error will occur.
There was a problem hiding this comment.
tsconfig.spec.json:5:15 - error TS5110: Option 'module' must be set to 'Node16' when option 'moduleResolution' is set to 'Node16'.
5 "module": "CommonJS",
~~~~~~~~~~
Fun! In that case, I think we can get away with just the "module": "Node16":
| "moduleResolution": "Node16" |
There was a problem hiding this comment.
Thank you! e829631
There was a problem hiding this comment.
LGTM, thanks! I'll defer to @bradzacher as more knowledgeable on TS upgrades.
Note for any onlookers: we'll still have to add support for using statements separately.
e829631
PR Checklist
Overview
Update TypeScript to 5.2.1-rc