Skip to content
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

Improve Typescript typings to allow explicit typing #206

Merged
merged 2 commits into from
Apr 23, 2019
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 23 additions & 22 deletions test/typescript/typings.ts
Original file line number Diff line number Diff line change
@@ -1,24 +1,25 @@
// relative path uses package.json {"types":"types/index.d.ts", ...}

import Aedes = require ('../..')
import aedes = require ('../..')
import { Client, AuthenticateError } from '../..'
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should really expose the server as aedes.Server.

Basically we should add to aedes.js:

aedes.Server = aedes

In this way you can do:

import { Server, Client, AuthenticateError } from 'aedes'

This should not be a semver-major change, but just a facility for those using ESM and Typescript.

Copy link
Contributor Author

@rafsawicki rafsawicki Jun 25, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mcollina what do you think about using default property instead?

Aedes.default = Aedes

This way the main function could be imported using default import syntax, while still exposing types if needed

import aedes, { Client, AuthenticateError } from 'aedes'  
// or Server, whatever default export name someone prefer

In fact, aedes can already be used this way with Babel or even with Typescript with esModuleInterop setting enabled. Unfortunately, by default TS module loader expects default property to be present.

Copy link
Contributor Author

@rafsawicki rafsawicki Jun 26, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately, from what I've checked there is no way to change typings in a way that would allow both - current require syntax and default import in Typescript. This means that there would be a breaking change for people that currently use aedes in TS without esModuleInterop enabled.

I still think that default import is the way to go, especially for discoverability - when looking at JS examples, I think it's more obvious that const aedes = require('aedes') translates to import aedes from 'aedes' than to import { Server } from 'aedes', especially since with Babel a default import already works just fine.

If you don't want to introduce breaking changes or don't like default import syntax, I'll change it the way you suggested.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would prefer the way I suggested, as it’s an additive change and it does not require a change in ts configuration.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I might have not explained it clearly - it wouldn't require ts configuration change, but change in the way it's currently imported. In any case, I added the Server property as you suggested.

import { IPublishPacket, ISubscribePacket, ISubscription, IUnsubscribePacket } from 'mqtt-packet'
import { createServer } from 'net'

const aedes = Aedes({
const broker = aedes({
concurrency: 100,
heartbeatInterval: 60000,
connectTimeout: 30000,
authenticate: (client, username: string, password: string, callback) => {
authenticate: (client: Client, username: string, password: string, callback) => {
if (username === 'test' && password === 'test') {
callback(null, true)
} else {
const error = new Error() as Error & { returnCode: number }
const error = new Error() as AuthenticateError
error.returnCode = 1

callback(error, false)
}
},
authorizePublish: (client, packet: IPublishPacket, callback) => {
authorizePublish: (client: Client, packet: IPublishPacket, callback) => {
if (packet.topic === 'aaaa') {
return callback(new Error('wrong topic'))
}
Expand All @@ -29,7 +30,7 @@ const aedes = Aedes({

callback(null)
},
authorizeSubscribe: (client, sub: ISubscription, callback) => {
authorizeSubscribe: (client: Client, sub: ISubscription, callback) => {
if (sub.topic === 'aaaa') {
return callback(new Error('wrong topic'))
}
Expand Down Expand Up @@ -57,70 +58,70 @@ const aedes = Aedes({
}
})

const server = createServer(aedes.handle)
const server = createServer(broker.handle)

aedes.on('closed', () => {
broker.on('closed', () => {
console.log(`closed`)
})

aedes.on('client', client => {
broker.on('client', client => {
console.log(`client: ${client.id} connected`)
})

aedes.on('clientDisconnect', client => {
broker.on('clientDisconnect', client => {
console.log(`client: ${client.id} disconnected`)
})

aedes.on('keepaliveTimeout', client => {
broker.on('keepaliveTimeout', client => {
console.log(`client: ${client.id} timed out`)
})

aedes.on('connackSent', client => {
broker.on('connackSent', client => {
console.log(`client: ${client.id} connack sent`)
})

aedes.on('clientError', client => {
broker.on('clientError', client => {
console.log(`client: ${client.id} error`)
})

aedes.on('connectionError', client => {
broker.on('connectionError', client => {
console.log('connectionError')
})

aedes.on('ping', (packet, client) => {
broker.on('ping', (packet, client) => {
console.log(`client: ${client.id} ping with packet ${packet.id}`)
})

aedes.on('publish', (packet, client) => {
broker.on('publish', (packet, client) => {
console.log(`client: ${client.id} published packet ${packet.id}`)
})

aedes.on('ack', (packet, client) => {
broker.on('ack', (packet, client) => {
console.log(`client: ${client.id} ack with packet ${packet.id}`)
})

aedes.on('subscribe', (subscriptions, client) => {
broker.on('subscribe', (subscriptions, client) => {
console.log(`client: ${client.id} subsribe`)
})

aedes.on('unsubscribe', (subscriptions, client) => {
broker.on('unsubscribe', (subscriptions, client) => {
console.log(`client: ${client.id} subsribe`)
})

aedes.subscribe('aaaa', (packet: ISubscribePacket, cb) => {
broker.subscribe('aaaa', (packet: ISubscribePacket, cb) => {
console.log('cmd')
console.log(packet.subscriptions)
cb()
}, () => {
console.log('done subscribing')
})

aedes.unsubscribe('aaaa', (packet: IUnsubscribePacket, cb) => {
broker.unsubscribe('aaaa', (packet: IUnsubscribePacket, cb) => {
console.log('cmd')
console.log(packet.unsubscriptions)
cb()
}, () => {
console.log('done unsubscribing')
})

aedes.close()
broker.close()
152 changes: 78 additions & 74 deletions types/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,81 +4,85 @@ import { IPublishPacket, ISubscribePacket, ISubscription, IUnsubscribePacket } f
import { Duplex } from 'stream'
import EventEmitter = NodeJS.EventEmitter

declare enum AuthErrorCode {
UNNACCEPTABLE_PROTOCOL = 1,
IDENTIFIER_REJECTED = 2,
SERVER_UNAVAILABLE = 3,
BAD_USERNAME_OR_PASSWORD = 4
declare namespace aedes {
export enum AuthErrorCode {
UNNACCEPTABLE_PROTOCOL = 1,
IDENTIFIER_REJECTED = 2,
SERVER_UNAVAILABLE = 3,
BAD_USERNAME_OR_PASSWORD = 4
}

export interface Client extends EventEmitter {
id: string
clean: boolean

on (event: 'error', cb: (err: Error) => void): this

publish (message: IPublishPacket, callback?: () => void): void
subscribe (
subscriptions: ISubscription | ISubscription[] | ISubscribePacket,
callback?: () => void
): void
unsubscribe (topicObjects: ISubscription | ISubscription[], callback?: () => void): void
close (callback?: () => void): void
}

export type AuthenticateError = Error & { returnCode: AuthErrorCode }

export type AuthenticateCallback = (
client: Client,
username: string,
password: string,
done: (err: AuthenticateError | null, success: boolean | null) => void
) => void

export type AuthorizePublishCallback = (client: Client, packet: IPublishPacket, done: (err?: Error | null) => void) => void

export type AuthorizeSubscribeCallback = (client: Client, subscription: ISubscription, done: (err: Error | null, subscription?: ISubscription | null) => void) => void

export type AuthorizeForwardCallback = (client: Client, packet: IPublishPacket) => IPublishPacket | null | void

export type PublishedCallback = (packet: IPublishPacket, client: Client, done: () => void) => void

export interface AedesOptions {
mq?: any
persistence?: any
concurrency?: number
heartbeatInterval?: number
connectTimeout?: number
authenticate?: AuthenticateCallback
authorizePublish?: AuthorizePublishCallback
authorizeSubscribe?: AuthorizeSubscribeCallback
authorizeForward?: AuthorizeForwardCallback
published?: PublishedCallback
}

export interface Aedes extends EventEmitter {
handle: (stream: Duplex) => void

authenticate: AuthenticateCallback
authorizePublish: AuthorizePublishCallback
authorizeSubscribe: AuthorizeSubscribeCallback
authorizeForward: AuthorizeForwardCallback
published: PublishedCallback

on (event: 'closed', cb: () => void): this
on (event: 'client' | 'clientDisconnect' | 'keepaliveTimeout' | 'connackSent', cb: (client: Client) => void): this
on (event: 'clientError' | 'connectionError', cb: (client: Client, error: Error) => void): this
on (event: 'ping' | 'publish' | 'ack', cb: (packet: any, client: Client) => void): this
on (event: 'subscribe' | 'unsubscribe', cb: (subscriptions: ISubscription | ISubscription[] | ISubscribePacket, client: Client) => void): this

publish (packet: IPublishPacket & { topic: string | Buffer }, done: () => void): void
subscribe (topic: string, callback: (packet: ISubscribePacket, cb: () => void) => void, done: () => void): void
unsubscribe (
topic: string,
callback: (packet: IUnsubscribePacket, cb: () => void) => void,
done: () => void
): void
close (callback?: () => void): void
}
}

interface Client extends EventEmitter {
id: string
clean: boolean

on (event: 'error', cb: (err: Error) => void): this

publish (message: IPublishPacket, callback?: () => void): void
subscribe (
subscriptions: ISubscription | ISubscription[] | ISubscribePacket,
callback?: () => void
): void
unsubscribe (topicObjects: ISubscription | ISubscription[], callback?: () => void): void
close (callback?: () => void): void
}

type AuthenticateCallback = (
client: Client,
username: string,
password: string,
done: (err: Error & { returnCode: AuthErrorCode } | null, success: boolean | null) => void
) => void

type AuthorizePublishCallback = (client: Client, packet: IPublishPacket, done: (err?: Error | null) => void) => void

type AuthorizeSubscribeCallback = (client: Client, subscription: ISubscription, done: (err: Error | null, subscription?: ISubscription | null) => void) => void

type AuthorizeForwardCallback = (client: Client, packet: IPublishPacket) => IPublishPacket | null | void

type PublishedCallback = (packet: IPublishPacket, client: Client, done: () => void) => void

interface AedesOptions {
mq?: any
persistence?: any
concurrency?: number
heartbeatInterval?: number
connectTimeout?: number
authenticate?: AuthenticateCallback
authorizePublish?: AuthorizePublishCallback
authorizeSubscribe?: AuthorizeSubscribeCallback
authorizeForward?: AuthorizeForwardCallback
published?: PublishedCallback
}

interface Aedes extends EventEmitter {
handle: (stream: Duplex) => void

authenticate: AuthenticateCallback
authorizePublish: AuthorizePublishCallback
authorizeSubscribe: AuthorizeSubscribeCallback
authorizeForward: AuthorizeForwardCallback
published: PublishedCallback

on (event: 'closed', cb: () => void): this
on (event: 'client' | 'clientDisconnect' | 'keepaliveTimeout' | 'connackSent', cb: (client: Client) => void): this
on (event: 'clientError' | 'connectionError', cb: (client: Client, error: Error) => void): this
on (event: 'ping' | 'publish' | 'ack', cb: (packet: any, client: Client) => void): this
on (event: 'subscribe' | 'unsubscribe', cb: (subscriptions: ISubscription | ISubscription[] | ISubscribePacket, client: Client) => void): this

publish (packet: IPublishPacket & { topic: string | Buffer }, done: () => void): void
subscribe (topic: string, callback: (packet: ISubscribePacket, cb: () => void) => void, done: () => void): void
unsubscribe (
topic: string,
callback: (packet: IUnsubscribePacket, cb: () => void) => void,
done: () => void
): void
close (callback?: () => void): void
}

declare function aedes (options?: AedesOptions): Aedes
declare function aedes (options?: aedes.AedesOptions): aedes.Aedes

export = aedes