Repository navigation
Generalize Kite transports & deprecate ws property #40
Description
Current kite client implementation is tightly coupled with websocket transform, the transport object it's using is assigned to kite.ws property. But we already have 2 different servers:
websocketsockjs
Since client implementation of these 2 servers are identical, only changing url to ws://someurl to http://someurl satisfies the kite initialization/connection. This works until we want to support another transport protocol which has different api characteristics (e.g: webrtc).
We are already accepting transformOptions and transformClass via options on Kite initialization step. I propose extending this feature with following steps:
- change the property name to
transport - generalize the construction of
transportby defining aKiteTransportinterface, and simply initialize giventransportClasswith giventransportOptionswithout any explicit check like we currently do here:
class Kite extends Emitter {
// ...
connect() {
const { url, transportOptions, transportClass } = this.options
// first initialize the transport
this.transport = new transportClass(url, transportOptions)
// assign event handlers
this.transport.addEventListener(Event.open, ...)
// then connect to it.
this.transport.connect()
}
// ...
}- Following is the
KiteTransportinterface i imagined
import { Event } from 'kite.js/lib/constants'
declare type TransportEvent =
| Event.OPEN
| Event.CLOSE
| Event.MESSAGE
| Event.ERROR
| Event.INFO
declare interface TransportEventResponder {
[Event.OPEN]: () => void,
[Event.CLOSE]: (event: Object) => void,
[Event.MESSAGE]: ({ data: Object }) => void,
[Event.ERROR]: (error: Object) => void,
[Event.INFO]: (info: Object) => void
}
declare interface KiteTransport {
uri: string;
options: Object;
responder: TransportEventResponder;
constructor(uri, options);
addEventListener(eventName: TransportEvent, handler: Function): void;
connect();
disconnect(reconnect: bool);
send(message: string): void;
}- Corresponding
WebSocketTransportclass would be like the following:
// ws-transport.js
import { KiteTransport } from 'kite.js'
import { Event } from 'kite.js/lib/constants'
import WebSocket from 'ws'
export default class WebSocketTransport extends KiteTransport {
constructor(url, options) {
// assigns url, and options to `this`.
super(url, options)
this.responder = null
}
connect() {
this.responder = new WebSocket(this.url)
}
addEventListener(eventName, handler) {
this.ready(() => {
this.responder.addEventListener(eventName, handler)
})
}
disconnect(reconnect) {
if (this.responder) {
this.responder.close()
this.responder = null
}
if (reconnect) {
this.connect()
}
}
send(message) {
if (!this.responder) {
throw new Error(`responder is not set`)
}
this.responder.send(message)
}
}and to initialize a kite using this transport i would use it just as before:
import { Kite } from 'kite.js'
import WebSocketTransport from './ws-transport'
const kite = new Kite({
url: 'ws://localhost',
transportClass: WebSocketTransport
})- Once we complete this transition we can extend
Kiteapi to accept atransportsoption allowing us to register multiple transports (please note these code snippets are just imaginary code as a brain storm, not sure how feasible it would be to implement these):
const kite = new Kite({
transports: [{
transport: WebSocketTransport,
options: wsOptions
}, {
transport: SockJsTransport,
options: sockJsOptions
}]
})
kite.connect()
// maybe passing url with `connect()` call?
// kite.connect(someUrlComingFromSomewhereElse)The aim here is to allow us using multiple transports without touching any of the existing kite code. I am still not sure how url would work in the scenario where we have multiple transports.
To sum up, since ws is used internal-only, (1) replacing it with a transport property can be done without introducing a breaking change. (2) extending kite api to accept multiple transports should be done once we are satisfied with the api and the way it's gonna work, therefore it implicitly depends on (1).
In the future i see several kite-server-<transport> packages exporting both Server classes and their Transport classes to be used by clients.
@gokmen thoughts?
Activity
gokmen commentedon Jun 26, 2017
This sounds great,
this.wsis not a great fit forsockjsusage I agree. and +1k for renaming it totransportbut we need to make sure that existing users of kite doesn't rely on that property like https://github.com/koding/koding/blob/master/client/app/lib/kite/kodingkite.coffee#L59 it's a thing that can be fixed easily but there might be some other usages. Instead I think it's time for to make major changes like this in 2.0 branch.