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

[typescript] Relax default-case rule #6906

Closed
NeoLegends opened this issue Apr 25, 2019 · 4 comments · Fixed by #6937
Closed

[typescript] Relax default-case rule #6906

NeoLegends opened this issue Apr 25, 2019 · 4 comments · Fixed by #6937

Comments

@NeoLegends
Copy link

react-scripts v3.0.0


Hey!

We're using the new ESLint-linting for TypeScript. Works great so far, but a TS feature makes a lint obsolete:

const fn = (input: 'foo' | 'bar'): string => {
  switch (input) {
    case 'foo':
      return 'a';
    case 'bar':
      return 'b';
    // `input` can't have any other value than `foo` or `bar`
    // (so this switch is guaranteed to be exhausive), yet ESLint
    // complains with `default-case`
  }
}

In this case the switch is safe because TS guarantees exhausiveness, yet ESLint complains about a missing default-case. However, in this case it's actually detrimental to add a default case, because you're actively using the compiler's exhaustiveness-analysis for correctness. And you won't be warned anymore when extending the range of valid parameter values and forgetting the appropriate switch case.

As far as I can see it, there are three options:

  1. Disable the lint for TS projects
  2. Hook into tsc to validate the switch's exhausiveness and conditionally apply the lint
  3. Do nothing and leave the lint as is

I consider options 1 and 2 to be valid, and I'd prefer not to go with 3, because it removes some of TS type system's power.

@ianschmitz
Copy link
Contributor

Hey @NeoLegends. What you're saying seems reasonable. Likely option 1 makes the most sense but i'll put some thought into it and get back to this. Thanks for reporting!

@ianschmitz
Copy link
Contributor

Seems the best option for now is to disable the default-case ESLint rule when using TypeScript, and suggest users use the noFallthroughCasesInSwitch instead.

@nmain
Copy link

nmain commented Apr 29, 2019

Is number 2 not viable? I'm not very familiar with the linter's internals. Typescript does compute the needed information (in the hypothetical missing default case, the type of the switch expression would be never if the case is not needed), so that would be the safest option.

@ianschmitz
Copy link
Contributor

ianschmitz commented Apr 30, 2019

Number 2 could be implemented in typescript-eslint as a rule. I don't know what value it would bring over the TypeScript compiler option. But this is outside the scope of CRA.

Regardless for now we have a solution that isn't a breaking change.

@lock lock bot locked and limited conversation to collaborators May 8, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants