Skip to content
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

Configs: Also set parser: "@typescript-eslint/parser" in common configs? #8149

Closed
2 tasks done
JoshuaKGoldberg opened this issue Dec 27, 2023 · 4 comments
Closed
2 tasks done
Labels
breaking change This change will require a new major version to be released package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin preset config change Proposal for an addition, removal, or general change to a preset config triage Waiting for maintainers to take a look

Comments

@JoshuaKGoldberg
Copy link
Member

JoshuaKGoldberg commented Dec 27, 2023

Before You File a Proposal Please Confirm You Have Done The Following...

Description

I, ah, never realized an ESLint config can also set a plugin! Or at least never thought to apply that feature to our configs. But that can be done, and is used in eslint-plugin-prettier.

Setting parser for users would remove a line from our getting started recommendation:

/* eslint-env node */
module.exports = {
  extends: ['eslint:recommended', 'plugin:@typescript-eslint/recommended'],
-  parser: '@typescript-eslint/parser',
  plugins: ['@typescript-eslint'],
  root: true,
};

IMO reducing config properties from 4 to 3 would be a nice little win for users. The ESLint/TypeScript ecosystem already has a reputation for high complexity. Anything we can do to reduce that IMO is good marketing as well as a better user experience.

Plus, per #6814 -> #8146, the tooling has really solidified on it being unexpected to use any parser other than @typescript-eslint/parser.

Impacted Configurations

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

Additional Info

Thanks @BPScott for informing me in https://github.com/JoshuaKGoldberg/dot-com/pull/140/files#r1435384958. 😄

@JoshuaKGoldberg JoshuaKGoldberg added package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin triage Waiting for maintainers to take a look breaking change This change will require a new major version to be released preset config change Proposal for an addition, removal, or general change to a preset config labels Dec 27, 2023
@JoshuaKGoldberg JoshuaKGoldberg changed the title Configs: Also set parser: "@typescript-eslint/parser" in recommended configs? Configs: Also set parser: "@typescript-eslint/parser" in common configs? Dec 27, 2023
@bradzacher
Copy link
Member

bradzacher commented Dec 27, 2023

We do set it in the base config which all configs extend

https://github.com/typescript-eslint/typescript-eslint/blob/main/packages/eslint-plugin/src/configs/base.ts

I personally always liked the user setting it explicitly because it means that it hard-overrides what any config might do - which prevents weird issues like the user doing extends: ["plugin:@typescript-eslint/recommended", "some-config-that-sets-another-parser"].

For example sometimes people extend the angular or Vue configs which can set other parsers as default and cause weirdness. Imagine if they extended a config that set the babel parser!!!

I think the one line in the example is worth warding against the potential bullshittery

@JoshuaKGoldberg
Copy link
Member Author

How often do folks set parsers other than ours for TypeScript code? I've never seen it in the wild, and only heard mention of it from #6814.

I think the one line in the example is worth warding against the potential bullshittery

Would you still consider the warding worth it post #8146? Personally, I wouldn't.

@JoshuaKGoldberg
Copy link
Member Author

Oh, and #8150: if we can roughly guarantee folks aren't pulling the potential bullshittery, I suspect the downsides would be much lessened.

@bradzacher
Copy link
Member

Would you still consider the warding worth it post #8146?

Yes because the problem is not type-aware rules. That's certainly one problem but the issue is that if someone uses eg babel's parser then they'll get "undefined behaviour" which is MUCH worse than an explicit error from our rules because you don't know its happening - you'll maybe get false positives as the AST is broken, or you might get crashes, or even worse you could get false negatives and not know anything is wrong!!!

How often do folks set parsers other than ours for TypeScript code?

Now-a-days not on purpose for the most part - but some people still try swapping it out for misguided reasons.

But most often I've seen it happen due to people doing things accidentally. I'm on mobile so search sucks - but recently there was an issue where someone was in the exact I mentioned - they'd extended a config which had set the babel parser as it was a JS config.

Considering the getting started guide is 4 lines I think it's fine for us to include it. It also serves as a nice point of education about how ESLint works - "we are providing configs, plugins and a parser for you"

@JoshuaKGoldberg JoshuaKGoldberg closed this as not planned Won't fix, can't repro, duplicate, stale Feb 4, 2024
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 12, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
breaking change This change will require a new major version to be released package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin preset config change Proposal for an addition, removal, or general change to a preset config triage Waiting for maintainers to take a look
Projects
None yet
Development

No branches or pull requests

2 participants