fix(typescript-eslint): make exported configs/plugin compatible with defineConfig()
#11190
Conversation
|
Thanks for the PR, @kirkwaiblinger! 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 project configuration. |
|
View your CI Pipeline Execution ↗ for commit 6730308.
☁️ Nx Cloud last updated this comment at |
defineConfig()defineConfig()
|
|
||
| /* eslint-disable @typescript-eslint/no-unnecessary-type-assertion -- These "unnecessary" assertions work around "requires explicit type annotation" errors. */ |
There was a problem hiding this comment.
could this be done as const configs = { ... } satisfies Record<string, eslintConfigHelpers.Config | eslintConfigHelpers.Config[]>?
There was a problem hiding this comment.
Hmm, well, now I can no longer repro the "requires a type assertion" error at all, maybe because I ended up adding the eslint/config-helpers package in the package.json.
So looks like the satisfies option is available to us (as well as just const configs = { ... }). I'll switch it to satisfies.
| }; | ||
| /* eslint-enable @typescript-eslint/no-unnecessary-type-assertion */ | ||
|
|
||
| export type Config = TSESLint.FlatConfig.ConfigFile; |
There was a problem hiding this comment.
I can't remember -- is this the loose type or the strict type?
Perhaps we should deprecate this to rename it? Or change the type to the ESLint types?
There was a problem hiding this comment.
This is a loose one; it's just a T | Promise<T> version of the type of the parameter type of tseslint.config(...configs).
I think deprecating makes sense 👍
| @@ -51,6 +51,7 @@ | |||
| "check-types": "npx nx typecheck" | |||
| }, | |||
| "dependencies": { | |||
| "@eslint/config-helpers": "^0.2", | |||
There was a problem hiding this comment.
ugh an 0.x version sucks cos it means that every minor release we'll be forced to update our version or else users will be blocked.
ESLint itself depends on this package so in some future when we drop v8 and older v9 versions -- we would be able to make this a peer dependency and just rely on the version installed that way.
chore(typescript-eslint): add type tests for `defineConfig`
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #11190 +/- ##
=======================================
Coverage 90.92% 90.92%
=======================================
Files 499 499
Lines 50845 50845
Branches 8384 8384
=======================================
Hits 46231 46231
Misses 4599 4599
Partials 15 15
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
|
ah, wait, redrafting since I forgot about the integration test failures |
Warning
I'll be travelling until May 27th, and don't expect to be able to spend time on this until then. If someone else is able to resolve the remaining failing integration tests in my absence and get this over the finish line, please feel free to do so!
PR Checklist
defineConfig()types #10899Overview
This switches our exports to use
defineConfig()-compatible types, rather than making our type declarations compatible with eslint core's. The type declarations that we have for things likeFlatConfig.Configand such are deliberately wide/loose, since they're meant to be used as a highly permissive parameter type fortseslint.config(). So we don't want to be exporting our configs as those types anymore.