-
Notifications
You must be signed in to change notification settings - Fork 1
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
Loosening a bit the rules for better DX #207
base: main
Are you sure you want to change the base?
Conversation
I wonder if we should have separate discussions (https://github.com/mediamonks/eslint-config/discussions/landing ?) for each of them, since PR view doesn't really support threaded (unless we use the comment thread for each rule in the diff as thread, but that's harder to later find back if we need to look at why we did certain things). Also, having everything in a single PR might make merging some of them early tricky when others are still under discussion, so might need to make separate PR(s) for those anyway. :) I haven't checked the rules myself, but are there any rules disabled that are auto-fixable? Those likely will have a different discussion compared to rules that you manually need to amend, or disable, since it's more a style preference. Anyhow, will have a more thorough look later :) |
Can you upvote pull request? (asking for a friend) |
This. As-is, this is a list of vague "this is annoying" statements without discourse and it's way too easy to have something slip through that'll bite us in the long run. |
Hey @nswaldman @ThaNarie How would you like to proceed? I can create a PR per rule if you guys would like to move on this way instead. And yes, I confirm the main reason for changing the rules is because they are super annoying and most devs I talked to hate them. By the way, I don't have access to https://github.com/mediamonks/eslint-config/discussions/landing |
@edgardz I've enabled discussions, and added an Announcement with some info on how we go about things https://github.com/mediamonks/eslint-config/discussions (feel free to also feedback on that :) )
I appreciate that you're mostly summarising here, but the motivations for changes should be a bit more elaborate :D Why do they hate them. Is it only because they are used to it, have they properly considered alternatives, etc.
I would suggest to indeed create a PR/Issue/Discussion with relevant link/information/motivation so we can discuss and document things on each individual basis :) I'm happy to leave this main one open as a reference, or to continue talking about the process around these things, since that's what it's already turned into at this point. |
@edgardz I finally had some time to create discussions for the rules. Please see the discussions page. |
Here is an initial list of rules we should consider disabling for better DX.
no-await-in-loop: sometimes we want/need do to things sequentially (like loading things in a specific order) and can leverage a for loop to do so. This rule introduces unnecessary verbosity.
arrow-body-style: this is just a style preference with no real impact in the result. I prefer to use the arrow function syntax when the body is a single expression, but I don't think it's worth enforcing it.
class-methods-use-this: this rule introduces unnecessary congnitive complexity. Sometimes the code is cleaner and easier to read when all methods are defined in the class, even if they don't use
this
.curly: this rule is too strict, and one of the most annoying. One liners can keep the code cleaner and easier to read.
no-implicit-coercion: introduces unnecessary verbosity
no-inline-comments: sometimes inline comments are useful to explain a specific line of code specially when the comment is really short.
no-multi-assign: again, single liners can keep the code cleaner and faster to read.
no-negated-condition: this rule is one of the top contenders for the most annoying rule. Sometimes it's easier to read a negated condition than the positive one.
no-nested-ternary: introduces unnecessary verbosity
prefer-destructuring: dot notation can be useful to determine where some value is coming from. It can also avoid variable name collisions. Also, forcing destructuring often introduces unnecessary verbosity.
prefer-named-capture-group: regex is already hard to read, this rule makes it even harder.
unicorn/consistent-function-scoping: introduces unnecessary verbosity
unicorn/no-array-for-each: super annoying. introduces unnecessary verbosity
unicorn/no-array-reduce: super annoying. introduces unnecessary verbosity
unicorn/prefer-array-find: this forbids this elegant way of filtering out nullish values:
array.filter(Boolean)
unicorn/prefer-module: this breaks inline require(), which can be really useful in some cases.
unicorn/prefer-number-properties: introduces unnecessary verbosity
unicorn/prefer-string-replace-all: introduces unnecessary verbosity
unicorn/prefer-string-slice: substr and substring work in slightly different ways, and sometimes we need to use one or the other.
unicorn/prevent-abbreviations: another huge contender for the most annoying rule.