yjs/y-webrtc

Allow custom signalling classes

Tails opened this issue · 2 comments

Tails commented

I have a websocket connection that expects a JSON-RPC envelope (Holochain). To support this I need to wrap the message that is sent over the websocket in the SignalingConn class. It would be great if the class would be defined as a constructor argument so it would not be needed to override the WebrtcProvider.connect(). Alternatively the SignalingConn class could be returned by a getSignalingConnectionClass() hook that can be overrided more easily.

const signalingConn = map.setIfUndefined(signalingConns, url, () => new SignalingConn(url))

I'm not a big fan of constructor magic. I wouldn't be able to guarantee that your custom signaling conn is properly managed.

I believe in this case the right approach is to extend the connect method.

// Your custom signaling conns should be maintained in a separate map:
const holoSignalingConns = new Map()

class HoloWebrtcProvider {
  connect () {
    super.connect()
    const holoSignalingConn = map.setIfUndefined(holoSignalingConns, 'your custom endpoint definition', () => new HoloSignalingConn())
      this.signalingConns.push(holoSignalingConn)
  }
}

This would also allow you to use HoloSignalingConn and pure SignalingConn in combination.

byrond commented

@dmonad we have been trying to do something similar and ran into some issues. We created #62 as a possible refactor to allow us to swap in a different signaling server implementation. Would you be willing to review that approach and provide feedback and whether this is something that could be merged into y-webrtc?