search

iamturns / eslint-config-airbnb-typescript

Airbnb's ESLint config with TypeScript support
MIT License
1.05k stars 98 forks source link

feat: Replace Airbnb 'default-case' rule with @typescript-eslint/swit… #321

Closed satouriko closed 7 months ago

satouriko commented 1 year ago

feat: Replace Airbnb 'default-case' rule with @typescript-eslint/switch-exhaustiveness-check

iamturns commented 1 year ago

Thank you for the PR @satouriko !

It looks like the default-case rule ensures all switch statements have a default case. Whereas the @typescript-eslint/switch-exhaustiveness-check rule ensures that all types within a union are within the switch. If the switch has a default case, then this rule does not apply.

The rules don't seem to be a 1 to 1 match, so I'm not sure about replacing one with the other. What do you think?

satouriko commented 1 year ago

In practice I would say, it's a much more common scene to use switch with an enum/union type expression, and in this case, default-case is not helpful (not able to warn you for non-exhaustiveness), and annoying (forcing you to add useless default branch). On the other hand, hardly would someone ever "forget" to add a default case while really "switching" on a infinite union type. This rule is practically useful while using a type defined by someone else, forgetting to cover the newly added cases when the type extends, and the "default case" would shadow this issue. So it's our team's (and probably many other team's) agreement to off default-case and prefer switch-exhaustiveness-check.

I've also seen issues like https://github.com/typescript-eslint/typescript-eslint/issues/2959 seeking to use default-case where switch-exhaustiveness-check doesn't apply, so I understand the worries & reasons not to replace it.

iamturns commented 7 months ago

Hi again @satouriko 👋

Given the goal of this ESLint config is to simply enhance Airbnb's rules with TypeScript support, I don't think this setting belongs. It feels different to all the other rules that are disabled and enabled.

There's a section in the README titled I wish this config would support [...] which explains a bit more.

iamturns commented 7 months ago

In an effort to clear all open PRs, I'm closing this one due to the above comment and the age of the PR. Thanks again @satouriko !