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

Flag to type 'catch' variables as 'unknown'. #41016

Closed
DanielRosenwasser opened this issue Oct 9, 2020 · 13 comments · Fixed by #41013
Closed

Flag to type 'catch' variables as 'unknown'. #41016

DanielRosenwasser opened this issue Oct 9, 2020 · 13 comments · Fixed by #41013
Assignees
Labels
Breaking Change Would introduce errors in existing code Committed The team has roadmapped this issue Fix Available A PR has been opened for this issue Suggestion An idea for TypeScript

Comments

@DanielRosenwasser
Copy link
Member

In TypeScript 4.0, we allowed users to to annotate catch variables with : unknown; however, it'd be nice if we could have that be the default.

I could imagine a flag like --useUnknownInCatchVariables to switch the default type to be unknown.

// @useUnknownInCatchVariables: true

try {
  // ...
}
catch (e) {
  e.toUpperCase(); // error
}

Alternatively, I could also imagine a much broader flag that also types parameters as unknown as well.

@MartinJohns
Copy link
Contributor

@DanielRosenwasser You might want to proofread the title.

@DanielRosenwasser
Copy link
Member Author

GitHub seems to be having UI glitches for me or something....

@DanielRosenwasser DanielRosenwasser changed the title Flag to tur Flag to type 'catch' variables as 'unknown'. Oct 9, 2020
@AviVahl
Copy link

AviVahl commented Oct 10, 2020

Default in tsc --init please? :)

@treybrisbane
Copy link

@DanielRosenwasser will this flag be included in strict?

@gerardolima
Copy link

I believe this should be the default behavior, actually. The inferred type for err in catch (err) { } should be unknown, which it is, instead of any which is unsafe and highly discouraged.

unknown is safer and provides better semantics; I really don't understand why this is not the default for tsc.

@MartinJohns
Copy link
Contributor

@gerardolima TypeScript usually does not introduce breaking changes without a very good reason, which I'd argue this is not. It's in the same realm with the strict compiler flags, which all are breaking changes and are opt-in.

@gerardolima
Copy link

Only code that rely on wrong assumptions (and any) would brake, @MartinJohns. As extensively demonstrated, exceptions in JS can be just anything and that's why no assumption should be valid on their types by default. This semantic is aligned with the TS type unknown, instead of any. Obviously, this is my view on the subject and that may not be aligned with TS team.

@DanielRosenwasser
Copy link
Member Author

DanielRosenwasser commented May 5, 2021

So through a personal Twitter survey, current results show that many people have voted for unknown as their preference in catch clause variables. There's definitely a bias there, but the preference between any and unknown skews higher towards unknown than I would've expected.

That said, there are two things worth mentioning:

  • I don't know if people really understand the pain that would come with unknown in catch variables.
  • A lot of people have specified a preference for Error or some form of checked exceptions, which is not necessarily at odds with this, but worth considering in case this conflicts with future work.

@samhh
Copy link

samhh commented May 6, 2021

I suspect you could broadly split TypeScript devs into two camps, those who're migrating from JavaScript or otherwise just want a barely-typed experience, and those who want their type system to be as safe as possible thus want to avoid any. For example, I'd be surprised if anyone wanted unknown catch clause variables but didn't also want JSON.parse to return unknown.

Totally anecdotal, but in codebases I've worked on where we've made use of try/catch for control flow we seldom ever treated it as anything other than unknown, it was generally just a value to propagate, be that to the console, an external logging service, etc. Even with a flag like this enabled the exceptions to the rule could easily be overridden with assertions.

@ritschwumm
Copy link

ritschwumm commented May 6, 2021

i suspect the camp migrating from javascript is on the decline - at least in my experience nobody starts a project in plain JS any more when it's possible to use TS instead. and - having just migrated a medium-sized project from JS to TS - i'd still be glad for any help the typechecker can give me to make things right while i'm at it. even if it takes a bit more effort.

@MartinJohns
Copy link
Contributor

@ritschwumm There are many projects out there that don't support strict type checks, and new ones are still created each day. Two examples I recently had to suffer from are Angular (which just recently started supporting strictNullChecks (released Sep 2016), disabled by default) and NestJS (which does not support strict type checks).

So I wouldn't say the only issue are projects migrating from JS.

@gerardolima
Copy link

@samhh the following code fails with @typescript-eslint rules due to no-unsafe-assignment; that's what I want to avoid. Variables typed as any are obviously handy, but they are unsafe and removing that liability is the whole point of TypeScript. So, in my opinion, the behaviour that infers types as any should be the optional ones.

try { throw new Error('any-error') } catch(err) { console.log({ err }) }

@phiresky
Copy link

phiresky commented May 8, 2021

Just want to mention there's a typescript-eslint rule to disallow inferring of the catch type (to enforce using catch(err: unknown)): no-implicit-any-catch.

Would be great to have it integrated in typescript though.

@DanielRosenwasser DanielRosenwasser added Committed The team has roadmapped this issue and removed In Discussion Not yet reached consensus labels May 14, 2021
@DanielRosenwasser DanielRosenwasser added this to the TypeScript 4.4.0 milestone May 14, 2021
@DanielRosenwasser DanielRosenwasser self-assigned this May 14, 2021
@typescript-bot typescript-bot added the Fix Available A PR has been opened for this issue label May 14, 2021
@DanielRosenwasser DanielRosenwasser added the Breaking Change Would introduce errors in existing code label Jun 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Breaking Change Would introduce errors in existing code Committed The team has roadmapped this issue Fix Available A PR has been opened for this issue Suggestion An idea for TypeScript
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants