-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
ref: Refactor some more any
types
#14546
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
Conversation
size-limit report 📦
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Generally lgtm but two places with weird gut feeling.
@@ -78,8 +78,7 @@ export class ServerRuntimeClient< | |||
/** | |||
* @inheritDoc | |||
*/ | |||
// eslint-disable-next-line @typescript-eslint/no-explicit-any | |||
public captureException(exception: any, hint?: EventHint, scope?: Scope): string { | |||
public captureException(exception: unknown, hint?: EventHint, scope?: Scope): string { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this might be breaking in a weird way but I can't think of an example right now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this should be fine as accepting stuff as unknown like this will generally work for anything. The problematic part is replacing a return value of any
with unknown
, or with function args taking ...args: unknown[]
vs ...args: any[]
.
const colno = column; | ||
const lineno = line; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
m: why are we removing the isNaN
check here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
because it should not actually be needed, I think? window.onerror = function()
should be supported everywhere in all the browser we support, and is typed consistently, so these should be proper numbers (or undefined) always?
6d45ce7
to
bc0b5d3
Compare
bc0b5d3
to
11bcde2
Compare
Getting rid of some more
any
types.