[camelcase] Remove rule from recommended config

[NMinhNguyen]

I'd like to request that the camelcase rule is removed from the recommended config to match ESLint: https://eslint.org/docs/rules/camelcase

Prior art: #651 and #729.

Repro

{
  "rules": {
    "@typescript-eslint/<rule>": ["<setting>"]
  }
}

// your repro code case

Expected Result

Actual Result

Additional Info

Versions

package	version
@typescript-eslint/eslint-plugin	2.12.0
@typescript-eslint/parser	2.12.0
TypeScript	3.7.3
ESLint	6.7.2
node	10.18.0
npm	6.13.4

[bradzacher]

As mentioned in #651 (comment)

  "@typescript-eslint/camelcase": "error",
     style rule.
     It is best practice to enforce some naming convention (eslint doesn't have a snake_case rule).
     Most codebases follow this style.
     Nobody has complained, so I think it's staying.

camelcase has been in recommended config since the recommended config was added for this plugin. I put forward that comment for commenting, and nobody suggested directly the rule be removed, just that stylistic rules be removed in general.

We removed the majority of the stylistic ones in #729, but I chose to leave a few of the less opinionated ones in. I'm happy going a bit more opinionated with the recommended set, considering it only takes a single line in your config to override any rule you don't like.

---

Reasoning as to why it's in the recommended set:

Every single JS/TS codebase I've seen uses camelCase naming.
Considering that there is no other naming convention rule in core eslint, and none have been suggested in this plugin yet (soon I'll have #1318, but that support is purely because I based the impl off of another impl which included it), it's pretty good evidence that either people use and enforce camelcase, or they don't enforce one at all.

Doing a quick google reveals two eslint plugins concerned with snake case [1] [2], which have a combined total of less than 300 weekly downloads. Which further supports my above assertion.

---

As per our docs on configs, please provide reasoning as to why you think it should be removed.

[NMinhNguyen]

@bradzacher thanks for your reply. There are several references as to why enforcing any sort of stylistic rules isn’t deemed best practice and I believe Create React App’s ESLint config was probably one of the first key players to promote this idea.

• https://twitter.com/dan_abramov/status/1086215004808978434?s=20
• https://twitter.com/dan_abramov/status/1036279035805028352?s=20
• https://twitter.com/dan_abramov/status/907238496850542592?s=21
• https://twitter.com/ryanflorence/status/965993401618391040?s=20
• https://twitter.com/kentcdodds/status/974300162985201664?s=20

nobody suggested directly the rule be removed

Issue #651 actually mentions the removal of camelcase under Stylistic rules. Further to that, there is an excellent comment by @brainkim that also mentions camelcase: #651 (comment)

There are certain API’s that have underscores in them such as UNSAFE_componentWillMount or unstable_createResource. Yes, I do realise that they’re unsafe and kind of exceptions but still, I don’t think producing a linting error when using such API’s necessarily helps you catch bugs.

[brainkim]

@NMinhNguyen Psst. Since I was pinged. If you’re struggling with typescript-eslint’s recommended ruleset, here’s something that I realized once I actually compared the eslint’s ruleset and typescript-eslint’s ruleset. The only rule which actually overlaps is no-unused-vars. In other words you can use:

"rules": {
  "@typescript-eslint/no-unused-vars": "error",
  "no-unused-vars": "off",
}

along with the default eslint:recommended rules and call it a day. I audited the rules under the framework I describe in the linked issue for rules which catch bugs or whatever and there are actually very few which aren’t “opinionated” and the ones which catch additional bugs often require more configuration via manually setting a tsconfig which must include the files you’re linting. In other words, it really isn’t worth it. These days I just set the rules to above and go on my merry way.

[bradzacher]

< twitter links >

All of these reference style rules in general, and they are more talking about rules like indent, and func-call-spacing. They all say you should use prettier. I VERY STRONGLY agree with this sentiment.

I generally put no time into rules which do things prettier already does better.
This is one of the reasons I've never spent the (large amount of) time required to fix our indent rule - prettier is a much better alternative.
I don't block the community however, so if people want to submit PRs to add them, I'm not going to block them though.

Prettier, however, does not mutate the AST by design, which means it will never enforce a naming convention, so in this department prettier doesn't count.

For dan's case - at FB we don't use the camelcase rule, because the codebase (many, many years ago) used jshint, which didn't have a naming rule. Then when the codebase was migrated to eslint, there were many non-camelcase names, so the rule was noisy. Then nobody bothered to ever go and clean it up. Though all the code I've worked on there informally uses camelcase naming conventions, just without the rule (people just do it naturally).

https://github.com/facebook/fbjs/blob/e85f39ae37e9650191e832c1d1ed8f02c272e9d1/packages/eslint-config-fbjs/index.js#L285-L286

There are certain API’s that have underscores in them such as UNSAFE_componentWillMount or unstable_createResource

They are outliers to the general case.
If your specific use case requires that you use those names, then this is the use case of adding the one line to your config, and manually configuring the rule. Which is the entire point of the allow option.

What about angular users, or vue users, or plain node users? Why should they not be recommended the rule because react has some experimental, unstable APIs named with underscores?

Issue #651 actually mentions the removal of camelcase under Stylistic rules...

Sorry, I was exaggerating when I said no one. What I mean by no one is "a mostly insignificant number of people".
Including reactions, in total I see ~5 people who have specifically asked for camelcase's removal. That's 5 users out of our 2.5mil weekly downloads.
Ofc we won't ever see that many users coming through our github issues, so I don't expect to see those numbers.

For something that has community support, the numbers I'm looking for are more like ~30+ users. The highest number of reactions on an issue was ~70, and the currently opened highest is ~40, so ~30 is something I see as having a decent community drive.
~5 users is not enough for me to consider a breaking change for.

---

In other words you can use ... along with the default eslint:recommended rules and call it a day

Sorry, I didn't quite understand your comment - are you saying the only rule of our plugin you use is no-unused-vars?

---

I'm very much okay having a few opinionated rules in the recommended set, considering how simple it is for people to disable them, should they not like them.
Just like how we try and set sensible defaults for rules, but sometimes you need to reconfigure the rule based on your specific use case.

[NMinhNguyen]

@brainkim thanks so much for the advice! That's definitely useful to know 🙂

@bradzacher

I'll quote things that I think are applicable to camelcase:

Linters are there to help you find mistakes in your code.

eslint-config-react-app has no stylistic rules

Doesn’t enforce any stylistic rules, only real mistakes.

If eslint:recommended, eslint-config-react-app and other such configs do not enable camelcase, why should @typescript-eslint/recommended? Is it really this preset's responsibility to enforce/recommend a particular naming convention? If your concern is it's a breaking change, then let's at least consider it for inclusion in the next breaking change release. I could even try and get votes if that would help build a stronger case?

Why should they not be recommended the rule

Given the above, I'd ask the opposite question - why should they be recommended a stylistic rule? If a team wants to enforce a certain naming convention, wouldn't it be better for them to opt in to it?

Sorry, I didn't quite understand your comment - are you saying the only rule of our plugin you use is no-unused-vars?

I interpreted that to mean he simply uses eslint:recommended without @typescript-eslint/recommended.

[bradzacher]

eslint-config-react-app is a different beast though.. It's enabled as part of create-react-app, and CRA is designed as a zero-config framework, so much so that adding your own config requires you to eject from the framework itself.

So eslint-config-react-app has to be as un-opinionated as possible, or else people will want to eject from the framework to disable the rules they don't agree with (and then it's a pain to un-eject, so you get a fragmented userbase...).
IIRC they only very recently added the ability to override the config with custom configs.

This makes it hard to compare our recommended set with that of eslint-config-react-app.

The eslint recommended set is a fair comparison, and one people bring up often. They choose to include zero stylistic rules. It's a valid standpoint, and I adopted it to a degree (I disabled most of the layout rules, and I want to disable them all in 3.0). There were just a few "stylistic rules" that I think tread the line between style and best practices, and as I believe they are a best practice, I left them in.

The community thinks the same or similar as well, because the original recommended set was built off of some relatively widely used community configs (it was an amalgamation of 3 or 4 community configs).
We were much smaller back then (about a year ago, about 3% of the user base we have now (85k vs 2.5m)), but there was a discussion amongst some 5-10 people and everyone was happy with the end result.

why should they be recommended a stylistic rule?

See the section in my original comment. Everyone does this standard anyway, so IMO it's good to enforce it from the get-go. Or else you end up in FB's case where you enable it too late and it's too hard to migrate your codebase to turn it on, and then you just rely upon code reviews to loosely enforce the standard.

I've always been of the opinion that being a little opinionated is good. If people hate it, it's a 1 line config to turn it off.
People love the opinionated airbnb set (~1.5m weekly downloads), and it leads to a lot of people tweaking it afterwards because it's opinionated, and in tweaking it, they end up learning more about eslint. That was how I learned about eslint, and from that I created my own config that I use in my personal works.

I interpreted that to mean he simply uses eslint:recommended without @typescript-eslint/recommended.

I think there are a number of rules that we offer that catch bugs / potential issues, and there are a few that are a little opinionated, but can again often catch potential issues. But it's up to you if you use them. We offer two configs, recommended and recommended-requiring-type-checking, so you can use recommended rules without the typechecking parser config.

I think there are a number of eslint rules that would be recommended, if they had type information built in. A few of those have made it in here (like require-await).

[NMinhNguyen]

People love the opinionated airbnb set (~1.5m weekly downloads)

It seems like many of the Twitter profiles I shared above actually discourage using it :) Ryan Florence put it this way:

In my experience meeting literally thousands of developers across the world, the airbnb lint config has caused way more harm than good. It's not standard, its obtuse. If you have any control over it, get rid of it.

A sane set of rules is react-app + prettier.

And although CRA is zero-config, the ESLint config that they do have has been widely recommended in the community because it does what a linter is supposed to do which is catch bugs.

IMO it's good to enforce it from the get-go

I did read the relevant portion in your original comment, however I still don’t think it’s good to recommend because it’s a stylistic choice and to me it doesn’t seem like it has anything to do with TypeScript or catching bugs. Given that it makes certain API’s more annoying to use (you can also imagine some REST API’s that return objects with kebab case properties), doesn’t help catch bugs, I don’t think it should be recommended.

At this point I feel like I am starting to repeat the reasoning as to why I think it should be removed. Do you know what can be done to have it removed? Is it the number of votes?

[bradzacher]

It seems like many of the Twitter profiles I shared above actually discourage using it

They do, but they are speaking as people that actively understand everything to do with linting and all the lint rules on offer.

Speaking as someone who has been given the daunting task of setting up a lint config for a codebase (when nobody, not even myself understood linting), the airbnb config is great. Plug it in, adjust a few rules and it works perfectly. There's a reason it still has 1.5m weekly downloads.

Now a days, I see that it is okay, but very opinionated. Some things are done better by other tools (like prettier), some parts I don't agree with. I think for a lot of devs setting up eslint, it's a great basis from which to build and learn about what you and your company's style actually is.
I think it's much better to go "I don't agree with that, I'll change it", because that's something you actually do. Rather than "we do X all the time, is there a lint rule for it?" (as this question is rarely actually asked, when it should be).

it does what a linter is supposed to do which is catch bugs.

I disagree with this statement. That is just a piece of what linters are for.

A linter is much broader than just bugs.
It's about reducing the cognitive load for engineers - you don't need to think about what you're doing is right, because the linter will help tell you.
It's about automating code reviews - reducing the burden on reviewing engineers because they don't have to think about if variables are named to conventions, or if a function should be marked as async.

Linters are also about enforcing code style and telling people how to do things. At FB we have a lot of custom internal lint rules which do things from telling people not to use deprecated APIs (and giving alternatives), to ensuring imports are ordered in the same way.

If for you all a linter is about is catching bugs, then our recommended set is not for you, and I don't intend it to be you.

Happy to accept a PR from anyone if they want to create a small, targeted set about only catching bugs, but the recommended set won't be it. But nobody has wanted to do this yet.

Do you know what can be done to have it removed? Is it the number of votes?

As I mentioned earlier - if there is significant response from the community, then it's something we maintainers will consider and discuss. If not, then sorry, it's staying in.

[NMinhNguyen]

Correct me if I’m wrong but it seems like the definition of recommended seems to differ between the official ESLint config and the official TypeScript one? Perhaps controversial, but maybe something like @typescript-eslint/opinionated would be a less contentious name? I’ve spoken to several people and they also had this expectation that they’d be able to drop in @typescript-eslint/recommended and it would match eslint:recommended to then find some legacy code written using non-camelCase variable names.

Linters are there to help you find mistakes in your code.

This was Dan’s quote and a sentiment that seems to be shared by many other thought leaders in the space like Kent Dodds, Ryan Florence etc.

[josgraha]

Great discussion and thanks so much for the history @bradzacher, very interesting background here. FWIW I had a similar experience using an internal closed API with kebab case. I migrated our linting system from tslint to eslint-typescript and reading some article opted into eslint-typescript:recommended. For the most part this was a great experience because eslint-typescript catches so many more code quality issues. After fixing some issues and disabling others I raised a tech debt ticket to fix all the ‘eslint-disable’ instances. But the one that stood out was this camelCase rule which was an exception. My OCD kicked in and this made me question my expectation of a linter. I decided that a linter should help us find code quality issues and nothing more. Makes it easy to mark a ticket as done and to definitively say “tech debt paid”. There are lots of tools out there as you mentioned to help us on our path stylistically. Luckily as Brian pointed out I can opt out of a contrived rule set and go with the base eslint recommended rule set. Sorry about the length of this story but I hope it helps provide some context of a real world use case and potential OCD friction :)

[tswaters]

This plugin's config is in a special sort of gray area whereby it needs to toggle certain eslint rules off, and turn on it's own for anything to work at all.

I'm looking through and it seems the choices of config are recommended which includes a slew of opinionated style-based lint rules, all which includes that and more... and eslint-recommended, which disables certain eslint rules that tsc will catch.

Is there a config that matches eslint:recommended, but with the appropriate eslint-typescript rules replaced, and only the "this is an error you probably didn't mean to do this" rules enabled?

Could one be added?

( re: naming, it would be exceedingly confusing to have an additional config that mirrors eslint:recommended and wasn't named recommended, so +1 to @NMinhNguyen's recommendation of using opinionated vs recommended )

[bradzacher]

We are a community run project with volunteer maintainers. The maintainers are mostly here to oversee the direction, triage issues, review and merge PRs, and maintain the parser internals (which are usually too extensive for new contributors to jump into).

As mentioned in my previous comment:

Happy to accept a PR from anyone if they want to create a small, targeted set about only catching bugs, but the recommended set won't be it. But nobody has wanted to do this yet.

We recommend that you use a rule to enforce a naming convention. Which is why it's in the recommended set.

If someone wants to PR a strict ruleset which "only catches errors", then I'm happy to review and merge it.

If someone wants to PR to enhance the eslint-recommended config so that it switches the eslint:recommended rules with our extension version, then I'm happy to review and merge it.

[bradzacher]

See proposed recommended changes in #1423