-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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
Updated typescript-eslint to v6 #54693
Changes from 1 commit
a6f12dd
b001345
4951fc2
f810a22
70caa0c
052f6ed
ee035f7
0fde8d4
170c4e8
2b56c5a
880768e
53e2ae1
0b72601
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -62,8 +62,8 @@ | |
"@typescript-eslint/ban-types": "off", | ||
"@typescript-eslint/no-empty-function": "off", | ||
"@typescript-eslint/no-empty-interface": "off", | ||
"@typescript-eslint/no-explicit-any": "off", | ||
"@typescript-eslint/no-unused-vars": "off", | ||
"@typescript-eslint/sort-type-constituents": "off", | ||
|
||
// Pending https://github.com/typescript-eslint/typescript-eslint/issues/4820 | ||
"@typescript-eslint/prefer-optional-chain": "off", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this off because it causes lots of churn? If so, I'd like to see it turned on, but maybe in a separate PR. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, and also because the rule has more false positives right now than I'm comfortable with - especially in a codebase like TypeScript's that does a lot of funky stuff with scoping. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm honestly shocked we aren't getting lints from declare const a: string | undefined
declare const b: string
const foo = a || b; Where the actual intent is that the empty string goes away. But maybe that's not a false positive. |
||
|
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
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.
Is this off because the tsconfig has the equivalent Typescript setting on? If not, what kind and how many unused variables do we have?
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.
Yes, it's this; honestly I would much prefer if we didn't use TS for this at all and instead made it a lint. So annoying to have to ensure my variables are all used just to run tests or something. At least debug skips typecheck.
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.
Only 18 failures at the moment. Not bad!
However, typescript-eslint/typescript-eslint#5017 -> gajus/eslint-plugin-jsdoc#858 is impacting the codebase here. Probably best to set up the workarounds in a separate PR IMO.