mightyiam / eslint-config-love

A TypeScript ESLint config that loves you
MIT License
765 stars 65 forks source link

Desired outcome: candidate rules are tested on some actual code prior to their addition #1841

Open mightyiam opened 3 days ago

mightyiam commented 3 days ago

@darkbasic I've had this in mind for a while. I think that even the least automated workflow might be reasonable, especially in cases where the impact of the rule is not clear. But for all new rules such a workflow would shed light on impact, which is a benefit. But considering how much time I spend on this project, I might be more inclined to test only for some rules.

If you or anyone wishes to help, then please pour your thoughts in here. Perhaps design a tool. Perhaps find an existing one.

darkbasic commented 3 days ago

You want rules you don't agree with to never be released?

Nope, that's not the point. The point is that the rules I don't agree with are usually highly contested among many other tyescript-eslint users/developers. That lead me to think that these rules simply didn't have enough real world testing done before being published in a stable recommended config.

I have many other rules I don't agree with (like the lack of trailing commas which make git rebases more difficult and error prone) but they are not fundamentally broken and I can either live with them or disable them.

You believe that more testing would decrease the frequency of such occurrences?

I'm hundred percent sure about that. It's just a matter of enabling a new rule in a big enough codebase and spending half an hour going through all the occurrences to notice if it does more harm than good. I don't have the presumption to think I'm always right so I usually talk with other developers (either on Discord or among colleagues) about the cases I think are problematic to see if I'm missing a piece of the puzzle and sometimes I do. Lately I unfortunately find too many occurrences where everybody is against a certain rule which means that these rules have been probably rushed out too quickly.

You suggest some additional testing scheme?

My biggest fear is that the audience for a "testing" version of this package might not be big enough to catch issues within a small timeframe. Projects with top notch CI/CD which include renovate among other things could potentially catch these issues early on, but chances are they would probably skip testing versions altogether. Where renovate isn't in charge of updating dependencies I usually skip several versions between each upgrade, so a testing version wouldn't help either. Increasing the testing time would block stable releases, which should still occur for rules that are deemed safe. What could help is configs IMO. The codebase sticks to the recommended config where you are 100% sure that either of the following is true:

  1. The rule doesn't provide any type safety benefit but it also never gets in your way.
  2. Most of the times the rule doesn't get in your way but it provides plenty enough type safety benefits to overcome the hurdle.

This is especially important if you have git hooks that force everybody to adhere to the linting rules in order to commit. But each version should also ship a testing config that enables additional rules which are suspected to be controversial. At this point CI has a chance to also run the testing config and report issues early on. You could bake controversial rules in the testing config for months without blocking stable releases.

mightyiam commented 2 days ago

I have many other rules I don't agree with (like the lack of trailing commas which make git rebases more difficult and error prone) but they are not fundamentally broken and I can either live with them or disable them.

This config doesn't go into formatting at all for that matter. Aren't you using a formatter in conjunction with this config?

I wouldn't expect users to participate in testing an experimental configuration.

While you seem upset about the release of controversial rules, the release of controversial rules may actually be a practical strategy. Early adopters provide feedback, rules are sometimes adjusted, progress is made. Would this be the cause of embarrassment among "supporters" of this config within teams?

darkbasic commented 2 days ago

This config doesn't go into formatting at all for that matter. Aren't you using a formatter in conjunction with this config?

Oh my bad, I use it in conjuction with eslint-prettier and I vaguely remembered that eslint-config-standard-with-typescript did have some formatting rules but I'm probably mistaken.

I wouldn't expect users to participate in testing an experimental configuration

Most won't, but those who care about esling-config-love not breaking their application will have a peak from time to time. Being able to stage changes for as long as months will make sure that some real world testing will be eventually done.

While you seem upset about the release of controversial rules, the release of controversial rules may actually be a practical strategy. Early adopters provide feedback, rules are sometimes adjusted, progress is made.

The problem with this approach is that almost each and every release is a major release and these tend to happen very frequently. If major bumps were reserved for potentially controversial changes and happened only every couple of months this could have worked.

Would this be the cause of embarrassment among "supporters" of this config within teams?

It actually is unfortunately. Reality is that if I give the team freedom to update eslint-config-love I risk either finding the app full of shit or having to stop doing whatever I'm doing to assess the problem and revert the rules.

This is not what I envision for eslint-config-love: it should be a collection of established best practices and as such it should be viable even for someone who cannot discern between good rules and bad rules. If we allow it to be a testing ground those who use it to learn will have a very bad time.

mightyiam commented 2 days ago

Oh my bad, I use it in conjuction with eslint-prettier and I vaguely remembered that eslint-config-standard-with-typescript did have some formatting rules but I'm probably mistaken.

It did: #1572

The problem with this approach is that almost each and every release is a major release and these tend to happen very frequently. If major bumps were reserved for potentially controversial changes and happened only every couple of months this could have worked.

The way versioning works is documented in the readme:

Any change that might require a user to make changes beyond upgrading this package is considered major. For example, rule addition are obviously major. It is expected that most version bumps will be major.

This is not what I envision for eslint-config-love: it should be a collection of established best practices and as such it should be viable even for someone who cannot discern between good rules and bad rules. If we allow it to be a testing ground those who use it to learn will have a very bad time.

Yeah, okay. I share this vision of it being "established". Or at least thought of and tested to a reasonable degree. But to help users who lack discernment, I don't currently have good ideas. The readme says:

As with any ESLint configuration, some ad-hoc disabling of rules is expected. It is further expected that the strict nature of this configuration would more frequently require the disabling of rules.

Users need to have discernment for this. Not to mention the rest of the engineering they are generally tasked to perform.

darkbasic commented 2 days ago

some ad-hoc disabling of rules is expected

I'm fine with that, but it should be limited to some. Several rules are doing more harm than good recently IMO.

Users need to have discernment for this. Not to mention the rest of the engineering they are generally tasked to perform.

Good luck having someone who just started learning Typescript being able to discern things that sometimes are hard even for experienced developers (example: the implications of not assigning a variable in Typescript).

Typescript is hard, there is no way around it. Sometimes I give up because things are too hard to implement and tend to break between major versions of Typescript. I cannot blame someone who just started, they need to get all the help that we can provide from good tooling.

otusweb commented 11 hours ago

I agree with @darkbasic on having a bleeding edge config and a recommended config that is trailing behind. I ended here because of the prefer-destructuring (which I feel does not bring much type safety and seems to be getting in the way from what I see in our codebase).

mightyiam commented 9 hours ago

If you really want this, I have some questions about it. But I don't want to reuse this particular issue.