-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
Configs: Also set parser: "@typescript-eslint/parser" in common configs? #8149
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
Comments
We do set it in the base config which all configs extend 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 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 |
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.
Would you still consider the warding worth it post #8146? Personally, I wouldn't. |
Oh, and #8150: if we can roughly guarantee folks aren't pulling the potential bullshittery, I suspect the downsides would be much lessened. |
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!!!
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" |
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 ineslint-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. 😄
The text was updated successfully, but these errors were encountered: