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

Loosening a bit the rules for better DX #207

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

edgardz
Copy link

@edgardz edgardz commented Jan 22, 2024

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.

@ThaNarie
Copy link
Member

ThaNarie commented Jan 22, 2024

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 :)

@reindernijhoff
Copy link
Member

Can you upvote pull request? (asking for a friend)

@nswaldman
Copy link

nswaldman commented Jan 23, 2024

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).

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.

@edgardz
Copy link
Author

edgardz commented Jan 23, 2024

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

@ThaNarie
Copy link
Member

ThaNarie commented Jan 23, 2024

@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 :) )

because they are super annoying and most devs I talked to hate them

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.

How would you like to proceed? I can create a PR per rule

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.

@leroykorterink
Copy link
Collaborator

@edgardz I finally had some time to create discussions for the rules. Please see the discussions page.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants