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

Allow custom signalling classes #11

Open
Tails opened this issue May 18, 2020 · 2 comments
Open

Allow custom signalling classes #11

Tails opened this issue May 18, 2020 · 2 comments

Comments

@Tails
Copy link

Tails commented May 18, 2020

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))

@dmonad
Copy link
Member

dmonad commented May 18, 2020

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
Copy link

byrond commented Jan 2, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants