Tests to typescript #229
Tests to typescript #229
Conversation
| } | ||
|
|
||
| export async function getAddressFromOptions({ ethereum, privateKey } = {}) { | ||
| export async function getAddressFromOptions({ ethereum, privateKey }: { ethereum?: EthereumConfig, privateKey?: any} = {}) { |
There was a problem hiding this comment.
We should probably get something like this into Config.ts then use the same types across the app e.g. here and Signer:
type PrivateKeyAuth = {
privateKey: string
ethereum?: never,
apiKey?: never,
}
type ProviderAuth = {
ethereum: ExternalProvider | JsonRpcFetchFunc
privateKey?: never,
apiKey?: never,
}
/**
* deprecated: Please create an ethereum identity.
*/
type APIKeyAuth = {
apiKey: string
privateKey?: never,
ethereum?: never,
}
type AnonymousAuth = {
privateKey?: never,
apiKey?: never,
ethereum?: never,
}
type Authenticated = PrivateKeyAuth | ProviderAuth | APIKeyAuth
type AuthOptions = Authenticated | AnonymousAuthMaybe for another PR, your call.
There was a problem hiding this comment.
Good point. It would be great if e.g. ts-toolbelt would have a good helper for this.
There was a problem hiding this comment.
yeah it might, worth looking into
| streamId: stream.id, | ||
| last: published.length, | ||
| }) | ||
| }, () => {}) |
There was a problem hiding this comment.
Hm, this shouldn't be required.
I also notice that resendSubscribe's last param is actually mislabeled as onMessage when it should be onFinally.
There was a problem hiding this comment.
Changed type Subscriptions.add, Subscriptions.subscribe and Subscriptions.resendSubscribe to MaybeAsync<(err?: any) => void>
test/unit/StubbedStreamrClient.ts
Outdated
| return new Stream(null, { | ||
| // eslint-disable-next-line class-methods-use-this | ||
| async getStream() { | ||
| return new Stream(null as any, { |
There was a problem hiding this comment.
Did a small change to the implementation and now there is less casting and disabled warnings.
| } | ||
| } | ||
| // publisherId is the hash of 'username' | ||
| // @ts-expect-error |
There was a problem hiding this comment.
If it's not too much effort maybe add additional comment after // @ts-expect-error explaining what it's erroring on? Might make these more likely to get fixed if the comment gives a clue as to what's wrong at a glance.
e.g.
// @ts-expect-error additional commentingThere was a problem hiding this comment.
I'll make another PR where most of these will be fixed, and the rest commented.
timoxley
left a comment
timoxley
left a comment
There was a problem hiding this comment.
👍 Let's get this merged asap.
| import config from './config' | ||
| import { Stream } from '../../src/stream' | ||
| import { Subscription } from '../../src' | ||
| import { PublishRequest } from 'streamr-client-protocol/dist/src/protocol/control_layer' |
There was a problem hiding this comment.
blegh, ideally shouldn't have dist/src in any import, I'd consider that stuff "private".
For now you might have to grab it like this:
import { ControlLayer } from 'streamr-client-protocol'
const { PublishRequest } = ControlLayer
But if we update protocol to 8.0.0 you should be able to do this as of streamr-dev/streamr-client-protocol-js#53:
import { PublishRequest } from 'streamr-client-protocol'| describeRepeats('GapFill with resends', () => { | ||
| let expectErrors = 0 // check no errors by default | ||
| let publishTestMessages: (n?: number, opts?: any) => Promise<[message: any, request: any][]> | ||
| let publishTestMessages: ReturnType<typeof getPublishTestMessages> |
Convert most of tests to TypeScript.