NET-177: Add TypeScript support #199
Conversation
…the same hash was already imported" error
NET-177 TypeScriptify streamr-client-javascript
Convert the JS-client from TypeScript. The highest priority is to convert all methods that are publicly available to the end-users. |
timoxley
left a comment
timoxley
left a comment
There was a problem hiding this comment.
A few minor things, but overall lgtm 👍 good work
| } | ||
|
|
||
| async getOrCreateStream(props: { id?: string, name?: string }) { | ||
| return this.streamEndpoints.getOrCreateStream(props) |
There was a problem hiding this comment.
So all these wrapped functions should probably just forward the exact signature of the functions they're wrapping, rather than manually duplicating the type definition in multiple places:
getOrCreateStream(...args: Parameters<StreamEndpoints['getOrCreateStream']>): ReturnType<StreamEndpoints['getOrCreateStream']> {
return this.streamEndpoints.getOrCreateStream(props)
}Might be able to parameterise this and make a helper type FnWrapper<T, N> = ??? but that's beyond my ability at this point.
There was a problem hiding this comment.
Alternatively, these could be mixins, either using the class factory method they describe here or the "alternative" method, which would be pretty close to how it was set up before: https://www.typescriptlang.org/docs/handbook/mixins.html
| import Stream, { StreamProperties } from './stream' | ||
| import { ExternalProvider, JsonRpcFetchFunc } from '@ethersproject/providers' | ||
|
|
||
| export interface StreamrClientOptions { |
There was a problem hiding this comment.
This should go into ./Config.ts and could probably be largely inferred from defaults.
| dataUnion: null, // Give a "default target" of all data union endpoint operations (no need to pass argument every time) | ||
| tokenAddress: '0x0Cf0Ee63788A0849fE5297F3407f701E122cC023', | ||
| minimumWithdrawTokenWei: '1000000', // Threshold value set in AMB configs, smallest token amount to pass over the bridge | ||
| // @ts-expect-error |
There was a problem hiding this comment.
Yeah these should probably be undefined or set to some default value or fix the type to be | null for now, IMO better than // @ts-expect-error
Added basic TypeScript support to the project. Converted most of the public methods to TypeScript.
TODO:
Todometa type: need to define valid types for those before 5.x is published@ts-expect-error: need to check if some of the TS warnings are actually bugsnull|undefinedStreamrClientpublic methods and fields