Generalize Kite transports & deprecate ws property

[usirin]

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:

• websocket
• sockjs

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 transport by defining a KiteTransport interface, and simply initialize given transportClass with given transportOptions without 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 KiteTransport interface 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 WebSocketTransport class 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 Kite api to accept a transports option 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?

[gokmen]

This sounds great, this.ws is not a great fit for sockjs usage I agree. and +1k for renaming it to transport but 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.