Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Changes to configurations for 6.0.0 #6014

Oct 29, 2022 · 11 comments · 16 replies
Discussion options

Edit: see #7130 for small touchups to these changes

Overview

Similar to what @bradzacher did for 5.0.0 (in #3746), I'm going to put forward the new recommended set ahead of time. We're looking for feedback from the community before we go ahead and make the changes.

These new configuration values are what we are planning on releasing with the v6 major version as part of its breaking changes. You can read more about v6 in the Announcing typescript-eslint v6 beta blog post. The original plan for changing the configs was discussed in #6019.

🙏 Please - give v6 a try, and give us feedback on the these new configuration values! Here or in the typescript-eslint Discord's v6 channel. We'll cross-post any important discussions here for visibility.

Table Key

Column Description Emojis
Status Being added, deprecated, or removed
  • 🆕 = newly added to TypeScript-ESLint
  • 🙅 = to be deprecated in the next major
  • 💀 = currently deprecated; to be removed in the next version
TC Requires type checking?
  • 💭 = yes
Ext Extension rule?
  • 🧱 = yes
Rec'd Recommended
  • ➕ = add to recommended this version
  • ➖️ = remove from recommended this version
  • 🟩 = stays in recommended this version
Strict Strict
  • ➕ = add to strict this version
  • ➖️ = remove from strict this version
  • 🔵 = stays in strict this version
Style Style
  • ➕ = add to stylistic this version
  • ➖️ = remove from stylistic this version
  • 🔸 = stays in stylistic this version

Recommendations Table

Hint: search for 🆕 to find newly added rules, and ➕ or ➖ to see config changes.

Rule Status TC Ext Rec'd Strict Style Comment
adjacent-overload-signatures ➖️
array-type ➖️
await-thenable 💭 🟩 naming: #5946
ban-ts-comment 🟩
ban-tslint-comment ➖️
ban-types 🟩 config: #5947
brace-style 🧱 layout 💩
class-literal-property-style ➖️
comma-dangle 🧱 layout 💩
comma-spacing 🧱 layout 💩
consistent-generic-constructors 🆕
consistent-indexed-object-style ➖️
consistent-type-assertions ➖️
consistent-type-definitions ➖️
consistent-type-exports 🆕 💭 too opinionated for any config
consistent-type-imports too opinionated for any config
default-param-last 🧱
dot-notation 💭 🧱 ➖️
explicit-function-return-type too opinionated for any config
explicit-member-accessibility too opinionated for any config
explicit-module-boundary-types too opinionated for any config
func-call-spacing 🧱 layout 💩
indent 🧱 layout 💩
init-declarations 🧱
keyword-spacing 🧱 layout 💩
lines-between-class-members 🧱 layout 💩
lines-around-comment 🧱 layout 💩
member-delimiter-style layout 💩
member-ordering too opinionated for any config
method-signature-style too opinionated for any config
naming-convention 💭 too opinionated for any config
no-array-constructor 🧱 🟩
no-base-to-string 💭 ➖️ required #4999
no-confusing-non-null-assertion ➖️
no-confusing-void-expression 💭
no-dupe-class-members 🧱
no-duplicate-enum-values 🆕
no-duplicate-imports 💀 🧱
no-duplicate-type-constituents 💭
no-dynamic-delete 🔵 too opinionated for recommended
no-empty-function 🧱 ➖️
no-empty-interface ➖️
no-explicit-any ➖️
no-extra-non-null-assertion 🟩
no-extra-parens 🧱 layout 💩
no-extra-semi 🧱 ➖️ layout 💩
no-extraneous-class 🔵
no-floating-promises 💭 🟩
no-for-in-array 💭 🟩
no-implicit-any-catch 💀
no-implied-eval 💭 🧱 🟩
no-inferrable-types ➖️
no-invalid-this 🧱
no-invalid-void-type 🔵 Default options: set allowAsThisParameter to true
no-loop-func 🧱
no-loss-of-precision 🧱 🟩
no-magic-numbers 🧱
no-meaningless-void-operator 💭 🔵
no-misused-new 🟩
no-misused-promises 💭 🟩
no-mixed-enums 💭 🔵
no-namespace 🟩
no-non-null-asserted-nullish-coalescing 🔵
no-non-null-asserted-optional-chain 🟩
no-non-null-assertion ➖️
no-parameter-properties 💀
no-redeclare 🧱
no-redundant-type-constituents 🆕 💭
no-require-imports
no-restricted-imports 🧱
no-shadow 🧱
no-this-alias 🟩
no-throw-literal 💭 🧱 🔵
no-type-alias
no-unnecessary-boolean-literal-compare 💭 🔵
no-unnecessary-condition 💭 🔵
no-unnecessary-qualifier 💭
no-unnecessary-type-arguments 💭 🔵
no-unnecessary-type-assertion 💭 🟩
no-unnecessary-type-constraint 🟩
no-unsafe-argument 💭 🟩
no-unsafe-assignment 💭 🟩
no-unsafe-call 💭 🟩
no-unsafe-declaration-merging 🆕 ➖️
no-unsafe-enum-comparison 🆕 💭 ➖️
no-unsafe-member-access 💭 🟩
no-unsafe-return 💭 🟩
no-unused-expressions 🧱
no-unused-vars 🧱 🟩
no-use-before-define 🧱
no-useless-constructor 🧱 🔵
no-useless-empty-export 🆕
no-var-requires 🟩
non-nullable-type-assertion-style 💭 ➖️
object-curly-spacing 🧱 layout 💩
padding-line-between-statements 🧱 layout 💩
parameter-properties 🆕 too opinionated for any config
prefer-as-const 🟩
prefer-enum-initializers
prefer-for-of
prefer-function-type
prefer-includes 💭 🔵
prefer-literal-enum-member 🔵
prefer-namespace-keyword ➖️
prefer-nullish-coalescing 💭 Default options: set all to false
prefer-optional-chain
prefer-readonly 💭
prefer-readonly-parameter-types 💭
prefer-reduce-type-parameter 💭 🔵 Blocked from recommending on #3440
prefer-regexp-exec 💭
prefer-return-this-type 💭 🔵
prefer-string-starts-ends-with 💭
prefer-ts-expect-error 🔵
promise-function-async 💭
quotes 🧱 layout 💩
require-array-sort-compare 💭 Default options: enable ignoreStringArrays
require-await 💭 🧱 🟩
restrict-plus-operands 💭 🟩 Default options: enable new in #6110 and checkCompoundAssignments
restrict-template-expressions 💭 🟩 Default options: enable all except allowAny
return-await 💭 🧱
semi 🧱 layout 💩
sort-type-constituents 🆕
sort-type-union-intersection-members 💀
space-before-blocks 🆕 🧱 layout 💩
space-before-function-paren 🧱 layout 💩
space-infix-ops 🧱 layout 💩
strict-boolean-expressions 💭
switch-exhaustiveness-check 💭
triple-slash-reference 🟩
type-annotation-spacing layout 💩
typedef
unbound-method 💭 🟩
unified-signatures 🔵
You must be logged in to vote

Replies: 11 comments · 16 replies

Comment options

| consistent-type-assertions | R: ➕🛑 |
consistent-type-assertions is a stylistic rule - some rare people would be okay using angle-bracket assertions.
It was removed from the recommended set in v3.0.0 - #1423


| no-duplicate-enum-values | R: ➕🛑 |

I think this is a good style to enforce and most of the time having a duplicate is a bug.
I think explicitly using an eslint-disable to document that the value is intentionally duplicated is a good thing for code quality.


| no-redundant-type-constituents | R: ➕🛑 |

Some codebases do use redundant constituents as a way of documenting possible values.
These usecases are pretty rare though, I think?
So we're probably good to turn this on.
It should catch more errors than it does flag "intentional" redundant code.


| restrict-plus-operands | R: ➖🛑 | too opinionated for recommended |

This rule was explicitly added to the recommended set in v3.0.0 - #1423
Why do you say that it's too opinionated?


👍 LGTM - no comment

➕🛑

  • no-useless-empty-export
  • no-base-to-string
  • no-redundant-type-constituents
  • no-unsafe-declaration-merging
You must be logged in to vote
4 replies
@JoshuaKGoldberg
Comment options

👍 from me on most of that! Just one thread:

restrict-plus-operands

Is there a strong argument to have this and no-base-to-string in recommended? With no-base-to-string, the recommended config already catches any unsafe concatenations. So what's left is concatenating non-string primitives (bigint, boolean, null, number, undefined) with strings. Which I sometimes do want to do in code for stringification.

@bradzacher
Comment options

no-base-to-string doesn't have handling for a lot of + cases that restrict-plus-operands does:

declare const num: number;
declare const str: string;
declare const bool: boolean;
declare const obj: {};
declare const objToStr: { toString(): string };
declare const any: any;
declare const unk: unknown;

num + num;              // NO error
num + str;              // RPO error
// num + bool;          // TS error
// num + obj;           // TS error
// num + objToStr;      // TS error
num + any;              // RPO error
// num + unk;           // TS error

str + str;              // NO error
str + bool;             // RPO error
str + obj;              // NBTS + RPO error ////////// ONLY CASE NBTS ERRORS ON
str + objToStr;         // RPO error
str + any;              // RPO error
// str + unk;           // TS error

// bool + bool;         // TS error
// bool + obj;          // TS error
// bool + objToStr;     // TS error
bool + any;             // RPO error
// bool + unk;          // TS error

// obj + obj;           // TS error
// obj + objToStr;      // TS error
obj + any;              // RPO error
// obj + unk;           // TS error

// objToStr + objToStr; // TS error
objToStr + any;         // RPO error
// objToStr + unk;      // TS error

any + any;              // RPO error
any + unk;              // RPO error

// unk + unk;           // TS error

payground

@JoshuaKGoldberg
Comment options

Got it, thanks - that makes a lot of sense to me. Keeping in 👍

@JoshuaKGoldberg
Comment options

Filed #6110 to add some friendly options to it. I'll propose we enable those options for both rules by default.

Comment options

Hello,

I was directed here from eslint/eslint#16557 (reply in thread) about feedback on the defaults. Looking at the configuration we have in our app, there are two rules that I would change from the table above:

  1. ban-ts-comment - We find ourselves using this when we are using a new browser API that is not yet available in TS types or just properties or quirks of APIs that are missing from TS types, and it would be too onerous on developers to create replacement types every time. We do ask developers to include a comment explaining why @ts-expect-error or @ts-ignore is being used, so I'd be on board with making this recommended if allow-with-description was enabled.
  2. no-non-null-assertion - I have seen this lead to situations where developers add unnecessary conditionals (and thus extra, untested paths through the code) because a value is non-null for a reason that is invisible to TS. What I'd like the lint settings to encourage developers to do is 1) think about whether the value really is potentially nullish, 2) if the non-nullishness depends on a non-obvious assumption, add a comment. I'm not sure how best to realize this though.

Other than these, I'm on board with the changes to remove rules that have been deemed too opinionated or obsoleted by a code formatter. Thank-you for all the work you are doing on TS-ESLint integration.

You must be logged in to vote
1 reply
@JoshuaKGoldberg
Comment options

ban-ts-comment ... using a new browser API that is not yet available in TS types or just properties or quirks of APIs that are missing from TS types, and it would be too onerous on developers to create replacement types every time

🤔 do you have any examples of this? Out of those two cases:

  • using a new browser API that is not yet available in TS types: the right solution would be to create "polyfill" types. How often does this happen?
  • properties or quirks of APIs that are missing from TS types: IME this is pretty rare, so I'd love to see examples of this happening in real world code.

no-non-null-assertion

Yeah this is unfortunate. I would really love to be able to enable something in the recommended ruleset that discourages type assertions - since they're almost always indicative of a flaw in the type system. But sometimes that flaw is in TypeScript itself, and/or something not really able to be overcome. Hence lumping it into strict-requiring-type-checking. Would love any ideas on other steps we could take!

Comment options

Are there any specific criteria for strict rule sets?
An equivalent mechanism to switch-exhaustiveness-check is enabled by default in many other modern languages (Kotlin, Swift, Rust, Scala, Ocaml, etc). I would be happy to be considered for addition to strict.

You must be logged in to vote
3 replies
@JoshuaKGoldberg
Comment options

Good question. We should document this somewhere! The description on https://typescript-eslint.io/linting/configs#recommended-configurations is a little vague:

Additional strict rules that can also catch bugs but are more opinionated than recommended rules.

The heuristics I've been using have been roughly:

  • Rules in recommended, strict, and stylistic should apply >99% of the time to >99% of projects that follow reasonable configurations (i.e. don't do anything ridiculously spooky, such as non-standard modifications to global APIs)
  • Rules in recommended should apply >99% of the time to projects that follow best practices most TypeScript developers should be comfortable with
  • Rules in strict should apply >99% of the time to projects that follow best practices very experienced TypeScript developers should be comfortable with

How does that feel?

@ishowta
Comment options

I am not an expert, but I think it is generally good.
I would have thought we could set a clear baseline, but it's hard to think about.

@ishowta
Comment options

I've been thinking about the indicators of a very experienced TypeScript developer.

  • Familiar with coding in Typescript
  • Has a good understanding of the Typescript type system
  • Can afford to rewrite old code
  • Can afford to make an effort to typing as accurately as possible

This comment was marked as off-topic.

@bradzacher
Comment options

hey @reintroducing - could you please file an issue for each of your questions?
this discussion thread is specifically about the configuration changes being introduced in v6.

Comment options

hey @reintroducing - could you please file an issue for each of your questions?
this discussion thread is specifically about the configuration changes being introduced in v6.

Absolutely, apologies for the noise.

You must be logged in to vote
1 reply
@reintroducing
Comment options

@bradzacher sorry, i mean this to be a reply to your comment above but did it through my phone and realized just now it went as a new comment (thus creating even more noise). Feel free to mark this one off topic as well (or just completely delete it).

Comment options

Where is @typescript-eslint/no-unsafe-enum-comparison in the list?

You must be logged in to vote
1 reply
@JoshuaKGoldberg
Comment options

Ah good callout, thank you! It was merged after the last time I went over this list. Just updated now to indicate it's meant to move from strict to recommended-type-checked.

Comment options

I'm testing the delta but it doesn't seem to be applying the new rules in some cases. For example, when I comment out my existing no-useless-empty-export, the rule doesn't fire. This is my config:

  "extends": [
    "eslint:recommended",
    "plugin:react/recommended",
    "plugin:react/jsx-runtime",
    "plugin:react-hooks/recommended",
    "plugin:@typescript-eslint/strict-type-checked",
    "plugin:@typescript-eslint/stylistic-type-checked",
    "plugin:eslint-comments/recommended",
    "plugin:import/recommended",
    "plugin:import/typescript",
    "plugin:testing-library/react",

    // This config should always be at the end of this list in order to disable formatting rules
    "prettier"
  ],
You must be logged in to vote
2 replies
@tylerlaprade
Comment options

Is it possible that the other plugins are forcing it back to v5 because they need it as a dependency?

@JoshuaKGoldberg
Comment options

That is a possibility, yes. You can try debugging techniques such as npm ls or manually looking through your lockfile (package-lock.json/yarn.lock/pnpm-lock.yaml). If you want help, our Discord would be the right place to ask. Good question!

Comment options

Would this be a good opportunity to change the default of allowNullableEnum in strict-boolean-expressions to false? As per the rule description, this is unsafe but allowed in the default so I have been overriding the value in my config.

You must be logged in to vote
3 replies
@JoshuaKGoldberg
Comment options

Oof, sorry for the delay @tylerlaprade - I'd missed this comment. I believe we're keeping it true by default for the same reason as allowNumber - that it's a bit too opinionated for folks. If you're interested in having this as a config change I'd suggest filing a new issue for it. We'd probably want to tackle it for v7.

@tylerlaprade
Comment options

Thanks for the reply, @JoshuaKGoldberg! I don't see those two options as equivalent nor as opinionated, though. As per the current documentation, allowNumber is safe to set to true:
image

while allowNullableEnum is unsafe:
image

The default for all other nullable primitives is false, as all of them are unsafe to set to true:
image

Apologies if I missed it, but what makes allowNullableEnum a different case from allowNullableString or allowNullableNumber?

@JoshuaKGoldberg
Comment options

D'oh! You're right, I mistook the two. Per #6760 we're going to want to make a few changes (or at least: I'll propose them). Linking here so it's not missed. Thanks!

Comment options

Noting a change from #7110: we're going to keep no-mixed-enums in strict after all. It's pretty... well, strict.

You must be logged in to vote
0 replies
Comment options

#7110 was merged into the v6 branch, so closing this long-lived discussion out. Thanks everyone! Looking forward to v6. 🥳

You must be logged in to vote
0 replies
Comment options

Just discovered that the default where completely changed for restrict-template-expressions.
It does now allow any (not updated here) and no mention in the release note?
And should I expect that any new param will be true by default making any update potentially reduce the surface scope of the rule?

You must be logged in to vote
0 replies
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin triage Waiting for team members to take a look breaking change This change will require a new major version to be released recommended-rules Discussion about recommended rule sets
7 participants
Converted from issue

This discussion was converted from issue #5900 on November 17, 2022 15:54.

Morty Proxy This is a proxified and sanitized view of the page, visit original site.