yjs / y-webrtc

WebRTC Connector for Yjs
MIT License
458 stars 111 forks source link

Allow custom signalling classes #11

Open Tails opened 4 years ago

Tails commented 4 years ago

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.

https://github.com/yjs/y-webrtc/blob/5fffe6b74bfbf3e53449dd193312533f20afa367/src/y-webrtc.js#L595

dmonad commented 4 years ago

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 8 months ago

@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?