-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
chore(lint): prefer 'unknown' to 'any', fix lint warnings #18488
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
base: develop
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| module.exports = { | ||
| extends: ['../.eslintrc.js'], | ||
| rules: { | ||
| // tests often have just cause to do evil | ||
| '@typescript-eslint/no-explicit-any': 'off', | ||
| }, | ||
| }; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,5 +2,5 @@ | |
| * Just an Error object with arbitrary attributes attached to it. | ||
| */ | ||
| export interface ExtendedError extends Error { | ||
| [key: string]: any; | ||
| [key: string]: unknown; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. m: This type is publicly exported. I think this is a (subtle) breaking change, right? Update: I found a few more changes below with the same issue. I think in most cases the affected types are only supposed to be used on objects users would pass to the SDK (in which case, it's on us to do more through checking). But nothing holds them back from also working with said objects in which case TS could break their builds after updating. I'm leaning towards avoiding this kind of subtle breakage (but definitely fixing it in v11) but also happy to be convinced otherwise :D WDYT? |
||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,7 +4,7 @@ import type { QueryParams } from './request'; | |
| * Data extracted from an incoming request to a node server | ||
| */ | ||
| export interface ExtractedNodeRequestData { | ||
| [key: string]: any; | ||
| [key: string]: unknown; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. m: this is also exported, so same question as above |
||
|
|
||
| /** Specific headers from the request */ | ||
| headers?: { [key: string]: string }; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,7 +6,7 @@ import type { SpanAttributes } from './span'; | |
| * Context data passed by the user when starting a transaction, to be used by the tracesSampler method. | ||
| */ | ||
| export interface CustomSamplingContext { | ||
| [key: string]: any; | ||
| [key: string]: unknown; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. m: this is also exported, so same question as above |
||
| } | ||
|
|
||
| /** | ||
|
|
||
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.
l: Should we think about splitting this file up into type definitons and our functionality? My thinking is for type definitions the file-wide explicit any disabling makes sense but it could let stuff slip through in the functions we export. WDYT?
(feel free to disregad, there might be a good reason I'm missing to disable it also for the exported functions)