typescript-eslint
typescript-eslint copied to clipboard
Enhancement: [use-unknown-in-catch-callback-variable] Option to only check inline functions
Before You File a Proposal Please Confirm You Have Done The Following...
- [X] I have searched for related issues and found none that match my proposal.
- [X] I have searched the current rule list and found no rules that match my proposal.
- [X] I have read the FAQ and my problem is not listed.
My proposal is suitable for this project
- [X] I believe my proposal would be useful to the broader TypeScript community (meaning it is not a niche proposal).
Link to the rule's documentation
https://typescript-eslint.io/rules/use-unknown-in-catch-callback-variable/
Description
I propose an option to only check on inline functions, i.e. .catch((foo) => bar)
and not .catch(baz)
. If baz
accepts any
, it's either from an external library or will be flagged by --noImplicitAny
/no-explicit-any
.
Use case: I have a few instances of .catch(console.error)
in my codebase to log errors without throwing. console.error
is typed as (...data: any[]): void
, and as such this rule triggers an error.
Fail
Promise.resolve().catch((error) => error) // Preserve old behavior
Pass
Promise.resolve().catch((error: unknown) => error) // Preserve old behavior
Promise.resolve().catch(console.log) // Would previously error
Promise.resolve().catch(foo) // Would previously error
function foo(bar: any) { }
Additional Info
No response
Note: a related idea came up in the rule proposal, see https://github.com/typescript-eslint/typescript-eslint/issues/7526#issuecomment-1907664555
I personally don't have a strong opinion. Several points in favor and several points against spring to mind. We'll see what others say.
yeah I'm personally -1 on this given that the usecase of specifically passing an "external" function that accepts any
is rare.
This is also trivially worked-around from the user side (eg switching to an explicit arrow .catch((arg: unknown) => console.error(arg))
).
Now-a-days I think most 3rd-party things are going to accept unknown
instead of any
, and within your own codebase you can always switch the fn to unknown
.
it's either from an external library or will be flagged by --noImplicitAny/no-explicit-any
Assuming that you have that compiler option turned on or that you have that rule turned on. It's not uncommon to not use the latter -- a lot of people don't love how restrictive it is.
FWIW the only pain I had with adopting this rule is when I'm passing error-tracking utilities.
redisClient.connect().catch(winston.error);
redisClient.connect().catch(console.error);
redisClient.connect().catch(Sentry.captureException);
NONE OF THESE work.
So, I'm a big fan. If you are not implementing this function there's not much you can do anyway.
Having thought more thoroughly about this, I am +1 to the request as well. Passed in handlers will never have implicitly any
parameters, which is the real problem with supplying a function expression as a handler; the parameter doesn't require an annotation because it's deduced as any
I don't feel strongly enough about this to want to push back against multiple 👍s. I'll defer to the rest of the team's majority. 🙂
Kirk and I both hold the thought that this is fine:
function handler(x: any) {
// ...
}
Promise.reject().catch(handler);
It's still any
but it's explicit. If one turns off no-explicit-any
, I don't see why we should report this one but not any other blatantly unsafe things going on.
3 months later and this request has received 2 community reactions. With the team split and low community support, I think we can take this as signal to reject this request.
There are documented workarounds:
- wrapping the calls with explicit
.catch((arg: unknown) => cb(arg))
- patching the types for the 3rd party lib locally to use
unknown
And this is not a super common usecase given the low volume of community engagement. As per our workflow guidelines - we are taking this as signal that this check is not important to the broader community and are thus rejecting the proposal. Thanks for filing!