Movatterモバイル変換


[0]ホーム

URL:


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

Sign up
Appearance settings

Configs: Have recommended/strict configs include lesser configs, and simplify type checked names#6019

Discussion options

Description

Edited December 8th, 2022: updated to include@aaronadamsCA's#6019 (comment) and@robertknight's#6014 (comment) to this proposal.

Context

Right now, no config in the following list of configs provided with typescript-eslint includes any other config in that list:

  • plugin:@typescript-eslint/recommended
  • plugin:@typescript-eslint/recommended-requiring-type-checking
  • plugin:@typescript-eslint/strict

That means if you want, say, the strictest, you must enable all three (https://typescript-eslint.io/docs/linting/configs):

{"extends":["plugin:@typescript-eslint/recommended","plugin:@typescript-eslint/recommended-requiring-type-checking","plugin:@typescript-eslint/strict"]}

That's very inconvenient -and sometimes even confusing- for end users.

Breaking Change: Including Lesser Configs

Proposal: how about we have each config in that list also include any previous config in the list? That way if you want, say, thestrict ruleset, you would only need to enable it, notrecommended-* ones:

{"extends":["plugin:@typescript-eslint/strict"]}

Associated Enhancement: More Delineated Configs

We've received occasional user feedback (e.g.#6014 (comment)) that ourrecommended rulesets also include somestylistic rules. The ESLint community has generally moved away from the old 2010s-era eslint-config-airbnb practice of doing that. I agree that we should separate those out.

This proposal suggests we split our recommended configurations into:

  • Functional rule configurations, for best best practices and code correctness:
    • recommended: Recommended rules that you can drop in without additional configuration.
    • recommended-type-checked: Additional recommended rules that require type information.
    • strict: Additional strict rules that can also catch bugs but are more opinionated than recommended rules.
    • strict-type-checked: Additional strict rules require type information.
  • Stylistic rule configurations:
    • stylistic: Stylistic rules you can drop in without additional configuration.
    • stylistic-type-checked: Additional stylistic rules that require type information.

You can see the equivalent code changes in#5251.

Description of Breaking Changes

In other words, there arethree changes being proposed here:

  • Reworking what's existing in the existingrecommended,recommended-requiring-type-checking, andstrict configs: thisis a breaking change
  • Adding newstrict-type-checked,sylistic, andstylistic-type-checked configurations: thisis not a breaking change
  • Renamingrecommended-requiring-type-checking torecommended-type-checked: thiswould be a breaking change, exceptrecommended-requiring-type-checking, will be left as an alias for its new config name,recommended-type-checked.
    • We'll eventually breaking change away the old alias - perhaps in v7.

Thanks@shawnmcknight for a greatTwitter discussion on phrasing the breaking changes!

You must be logged in to vote

Replies: 2 comments 6 replies

Comment options

Could I suggest a change?

Is it possible we could get astrict-requiring-type-checking that includesrecommended-requiring-type-checking, and thenstrict would only includerecommended?

I'd love to extend thestrict recommendation, but right now for performance purposes we try not to connect rules that require type information to the IDE. If I could just extendstrict for the IDE and then addstrict-requiring-type-checking for the check scripts, it would cut down on our maintenance a ton.

I know it's a separate request, but I wanted to mention it here first since it's in direct opposition to half of this change.

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

JoshuaKGoldbergNov 27, 2022
Maintainer Author

I wish we could thread existing discussion comments, since migrating from issues puts them all root-level 🙃 but responding in#6019 (reply in thread): yes!

@JoshuaKGoldberg
Comment options

JoshuaKGoldbergDec 8, 2022
Maintainer Author

@aaronadamsCA I merged this into the OPand so will hide this thread. [Edit: unhid so folks can see this context] Thanks! 😄

Comment options

I definitely agree that splitting outstrict-requring-type-checking might be a good idea.
One of the reasons we separatedrecommended andrecommended-requiring-type-checking is so that people could chose to lint with type information in specific situations for perf reasons.

So i think it might make sense to continue this theme forstrict as well?

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

JoshuaKGoldbergNov 27, 2022
Maintainer Author

Agreed, I really like this. I've also been noodling on getting a less verbose name. The closest I've come up with has been*-type-checked. So that makes four configs:

  • recommended
  • recommended-type-checked
  • strict
  • strict-type-checked

I'll update the PR.

@bradzacher
Comment options

do we want to rename the configs? that seems like a pretty major breaking change that would impact most of our users.
Personally I'm averse to such sweeping breakages unless there's significant value to be gained - I don't think that saving one word is a big enough win to justify.

@JoshuaKGoldberg
Comment options

JoshuaKGoldbergNov 27, 2022
Maintainer Author

I like renaming them to draw attention to big shifts, like going from three configs to six. It forces users to take stock of their configs. Especially ifstrict ends up being weaker in v6 than it is now, for having type-checked rules moved tostrict-type-checked.

To mitigate pain, I'd propose we:

  • Leaverecommended-requiring-type-checking alive as an alias forrecommended-type-checked
  • Go through and send PRs to major packages (we can use Sourcegraph to query for them!)
  • Make it log a console warning in v7
  • Remove it in v8
@JoshuaKGoldberg
Comment options

JoshuaKGoldbergDec 8, 2022
Maintainer Author

Chatted privately - we're 👍 to go for now!

I merged this into the OPand so will hide this thread. Edit: unhid so folks can see this context.

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Labels
package: eslint-pluginIssues related to @typescript-eslint/eslint-pluginbreaking changeThis change will require a new major version to be releasedaccepting prsGo ahead, send a pull request that resolves this issuepreset config changeProposal for an addition, removal, or general change to a preset config
3 participants
@JoshuaKGoldberg@aaronadamsCA@bradzacher
Converted from issue

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


[8]ページ先頭

©2009-2025 Movatter.jp