Add (incomplete) Typescript definitions #134
Conversation
|
Changes Unknown when pulling 0f38d10 on andrew8er:feature/typescript-definitions into ** on mtth:master**. |
types/index.d.ts
Outdated
| export type CodecTransformer = (buffer: Buffer, callback: () => void) => Buffer; // TODO | ||
|
|
||
| export interface CodecOptions { | ||
| deflate: CodecTransformer; |
There was a problem hiding this comment.
Is 8 spaces per tab a strong convention for typings? If not, can we switch to 2 for consistency with the other files?
There was a problem hiding this comment.
I have no strong feelings on file indentations, although I think that two spaces do not provide enough visual contrast.
types/index.d.ts
Outdated
|
|
||
| export type Callback<V, Err = any> = (err: Err, value: V) => void; | ||
|
|
||
| export type CodecTransformer = (buffer: Buffer, callback: () => void) => Buffer; // TODO |
There was a problem hiding this comment.
It'd probably be easier to extend if we omit partial types entirely. Can you remove this and all other non-complete types in the file?
There was a problem hiding this comment.
The CodecTransformer definition is almost correct, as far as I can see it, only the callback definition might be lacking an argument. I think, the definition has a certain value, because programmers will provide those functions and thus can be sure they are defined correctly.
types/index.d.ts
Outdated
| } | ||
|
|
||
| export interface Decoder { | ||
| on(type: 'metadata', callback: (type: Type.Type) => void): this; |
There was a problem hiding this comment.
Does this mean that TypeScript is able to support different callback types for different events?
types/index.d.ts
Outdated
| export function parse(schemaOrProtocolIdl: string, options?: any): Service.Protocol | Type.Type; // TODO | ||
| export function readProtocol(protocolIdl: string, options?: ReaderOptions): Service.Protocol; | ||
| export function readSchema(schemaIdl: string, options?: ReaderOptions): Type.Schema; | ||
| // TODO streams |
There was a problem hiding this comment.
Unless you're certain that they are exhaustive, can we remove the commented TODOs?
There was a problem hiding this comment.
I am actually not sure if these are exhaustive, hence the TODO…
types/index.d.ts
Outdated
|
|
||
| interface ClientOptions { | ||
| buffering: boolean; | ||
| cache: any; |
There was a problem hiding this comment.
The cache option is deprecated, can we remove it?
types/index.d.ts
Outdated
| createServer(options?: ServerOptions): Server; | ||
| message(name: string): any; | ||
| type(name: string): any; | ||
| inspect(name: string): any; |
There was a problem hiding this comment.
I'm not sure it's useful to include this method, it's an override just for pretty-printing in the REPL.
types/index.d.ts
Outdated
| } | ||
|
|
||
| function compatible(client: Client, server: Server): boolean; | ||
| function forProtocol(protocol: Protocol): Service; |
There was a problem hiding this comment.
This might be too restrictive. It's sometimes useful to instantiate services from inlined JavaScript objects, which I think this would prevent, right?
types/index.d.ts
Outdated
| // TODO types | ||
|
|
||
| export namespace Type { | ||
| type Schema = string; |
There was a problem hiding this comment.
Schemas can also be JavaScript objects (for example {type: 'map', values: 'int'}).
There was a problem hiding this comment.
The Js object could also be completely typechecked. Have a look at https://github.com/joewood/avro-typescript/blob/master/src/model.ts
Maybe we could extract a common Ts definition for Avro schemas for use in booth projects.
|
Changes Unknown when pulling be9936c on andrew8er:feature/typescript-definitions into ** on mtth:master**. |
These are probably wrong and/or incomplete. They may serve as a starter though.