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

Use wrtc as a `peerDependency #8

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Conversation

piranna
Copy link

@piranna piranna commented Jan 12, 2021

Fix #7

@piranna
Copy link
Author

piranna commented Feb 11, 2021

Can this be reviewed and merged?

@t-mullen
Copy link
Owner

wrtc is an interface, one implementation of which is node-webrtc (the module available on NPM). There are others - including https://github.com/mappum/electron-webrtc. Changing this to a peer-dependency prevents devs from choosing an implementation.

Thanks for your interest in this project - please keep in mind it is a WIP and barely functions.

@piranna
Copy link
Author

piranna commented Feb 22, 2021

wrtc is an interface, one implementation of which is node-webrtc (the module available on NPM). There are others - including mappum/electron-webrtc.

Not like this... wrtc is a npm module, that node-webrtc is it's repo name. The interface is the W3C WebRTC API.

Changing this to a peer-dependency prevents devs from choosing an implementation.

Not really, since the project makes use of wrtc non-standard APIs. Once we get rid of them and use just only the W3C APIs, then it would make sense to allow to pass the actual implementation object as parameter, but then maybe we would need to change the project name to reflect that too.

Thanks for your interest in this project - please keep in mind it is a WIP and barely functions.

That's why I have proposed these changes, we are currently using them in production code :-)

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.

Use wrtc as a peerDependency
2 participants