-
Notifications
You must be signed in to change notification settings - Fork 177
sdks/ts: replace any with proper types in SDK core #2857
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
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 |
|---|---|---|
|
|
@@ -320,25 +320,25 @@ function flatten<T, E, E2>(this: Result<Result<T, E>, E2>): Result<T, E | E2> { | |
| else return this.val; | ||
| } | ||
|
|
||
| function assertErrorInstanceOf<T, C extends abstract new (..._: any) => any>( | ||
| function assertErrorInstanceOf<T, C extends abstract new (..._: unknown[]) => unknown>( | ||
|
Contributor
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. Again not much value add. It doesn't provide extra safety, but not "against" this change just because it kind of look better than previous version
Contributor
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. Any is not unknown. Sometimes |
||
| this: Result.Ok<T>, | ||
| constructor: C, | ||
| ): Result.Ok<T>; | ||
| function assertErrorInstanceOf<E, C extends abstract new (..._: any) => any>( | ||
| function assertErrorInstanceOf<E, C extends abstract new (..._: unknown[]) => unknown>( | ||
| this: Result.Err<E>, | ||
| constructor: C, | ||
| ): Result.Err<E & InstanceType<C>>; | ||
| function assertErrorInstanceOf<T, E, C extends abstract new (..._: any) => any>( | ||
| function assertErrorInstanceOf<T, E, C extends abstract new (..._: unknown[]) => unknown>( | ||
| this: Result<T, E>, | ||
| constructor: C, | ||
| ): Result<T, E & InstanceType<C>>; | ||
| function assertErrorInstanceOf<T, E, C extends abstract new (..._: any) => any>( | ||
| function assertErrorInstanceOf<T, E, C extends abstract new (..._: unknown[]) => unknown>( | ||
| this: Result<T, E>, | ||
| constructor: C, | ||
| ): Result<T, E & InstanceType<C>> { | ||
| if (this.isOk()) return this; | ||
|
|
||
| if (this.val instanceof constructor) return this as any; | ||
| if (this.val instanceof constructor) return this as Result<T, E & InstanceType<C>>; | ||
|
|
||
| throw new TypeError(`Assertion failed: Expected error to be an instance of ${constructor.name}.`); | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -233,6 +233,6 @@ export function fallibleTransaction<Out, Err>( | |
| * ``` | ||
| * | ||
| */ | ||
| export type OperationErrors<T extends Operation<any, any, any>[]> = { | ||
| [K in keyof T]: T[K] extends Operation<any, any, infer Err> ? Err : never; | ||
| export type OperationErrors<T extends Operation<unknown, unknown, unknown>[]> = { | ||
|
Contributor
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. This doesn't give any extra safety, but I am not totally against this change |
||
| [K in keyof T]: T[K] extends Operation<unknown, unknown, infer Err> ? Err : never; | ||
| }[number]; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -70,7 +70,7 @@ const handlers: { [K in TsType['kind']]: Handler<K> } = { | |
| config: unsupported('Config'), | ||
| }; | ||
|
|
||
| function unsupported(kind: string): Handler<any> { | ||
| function unsupported<K extends TsType['kind']>(kind: string): Handler<K> { | ||
| return ({ scopeName, parameterInScope }) => | ||
| Either.left( | ||
| `Unsupported type \`${kind}\`` + | ||
|
|
@@ -79,7 +79,7 @@ function unsupported(kind: string): Handler<any> { | |
| ); | ||
| } | ||
|
|
||
| function unsupportedWithHint(kind: string, hint: string): Handler<any> { | ||
| function unsupportedWithHint<K extends TsType['kind']>(kind: string, hint: string): Handler<K> { | ||
|
Contributor
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. 👍 |
||
| return ({ scopeName, parameterInScope }) => | ||
| Either.left( | ||
| `Unsupported type \`${kind}\`${scopeName ? ` in ${scopeName}` : ''}` + | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -16,7 +16,7 @@ import * as util from 'node:util'; | |
| import { NameOptionTypePair } from '../types/analysedType'; | ||
|
|
||
| // type mismatch in tsValue when converting from TS to WIT | ||
| export function typeMismatchInSerialize(tsValue: any, expectedType: string): string { | ||
| export function typeMismatchInSerialize(tsValue: unknown, expectedType: string): string { | ||
|
Contributor
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. If you think about this, it's not unknown. its |
||
| return `Type mismatch. Expected type \`${safeDisplay(expectedType)}\`, got \`${safeDisplay(tsValue)}\``; | ||
| } | ||
|
|
||
|
|
@@ -30,23 +30,23 @@ export function typeMismatchInDeserialize(nodeTag: string, expectedType: string) | |
| } | ||
|
|
||
| // Missing keys in tsValue when converting from TS to WIT | ||
| export function missingObjectKey(key: string, tsValue: any): string { | ||
| export function missingObjectKey(key: string, tsValue: unknown): string { | ||
| return `Missing key '${key}' in ${safeDisplay(tsValue)}`; | ||
| } | ||
|
|
||
| // tsValue does not match any of the union types when converting from TS to WIT | ||
| export function unionTypeMatchError(unionTypes: NameOptionTypePair[], tsValue: any): string { | ||
| export function unionTypeMatchError(unionTypes: NameOptionTypePair[], tsValue: unknown): string { | ||
| const types = unionTypes.map((t) => t.name); | ||
| return `Value '${safeDisplay(tsValue)}' does not match any of the union types: ${types.join(', ')}`; | ||
| } | ||
|
|
||
| export function enumMismatchInSerialize(enumValues: string[], tsValue: any): string { | ||
| export function enumMismatchInSerialize(enumValues: string[], tsValue: unknown): string { | ||
| return `Value '${safeDisplay(tsValue)}' does not match any of the enum values: ${enumValues.join(', ')}`; | ||
| } | ||
|
|
||
| // unhandled type of tsValue when converting from TS to WIT | ||
| export function unhandledTypeError( | ||
| tsValue: any, | ||
| tsValue: unknown, | ||
| typeName: string | undefined, | ||
| message: string | undefined, | ||
| ): string { | ||
|
|
@@ -57,6 +57,6 @@ export function unhandledTypeError( | |
| return error + (message ? `${message}` : ''); | ||
| } | ||
|
|
||
| export function safeDisplay(tsValue: any): string { | ||
| export function safeDisplay(tsValue: unknown): string { | ||
| return util.format(tsValue); | ||
| } | ||
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.
Again no much safety introduction.
But this version is not harming. Hence approving