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

signalingOptions #529

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

thegobot
Copy link

No description provided.

@jianjunz
Copy link
Collaborator

jianjunz commented Jan 17, 2022

The ctor of ConferenceClient accepts a signaling channel object, which allows custom configuration or even implementation for signaling.

export const ConferenceClient = function(config, signalingImpl) {

@thegobot
Copy link
Author

thegobot commented Jan 17, 2022

The ctor of ConferenceClient accepts a signaling channel object, which allows custom configuration or even implementation for signaling.

export const ConferenceClient = function(config, signalingImpl) {

I understand, but why should I make my own implementation if I need to change 1 parameter (path)? I use nginx to proxy

@jianjunz
Copy link
Collaborator

The signaling options may not work for all signaling implementations. It may conflict with signalingImpl in ConferenceClient. Consider two cases here:

  1. Another new signaling protocol is added, e.g.: WebTransport. The signalingOptions may apply to a specific signaling protocol only.
  2. A user provided signaling module, which could also based on socket.io, doesn't support this option, so it could potentially be ignored.

If I understand correctly, the value of signalingOptions applies to a specific signaling implementation. It's better to be an argument of the signaling implementation's ctor. Then use this signaling implementation to create a ConferenceClient.

@thegobot
Copy link
Author

The signaling options may not work for all signaling implementations

The end user must know (and choose in advance) the implementation. Options he applies to a known implementation, obviously

@jianjunz
Copy link
Collaborator

The signaling implementation could be undefined, SDK choses one. It's socket.io at this time, but it could be another one in the future. There is no guarantee that undefined signaling implementation is always the socket.io one.

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

Successfully merging this pull request may close these issues.

None yet

2 participants