add optional wrtc support

[bukzor]

I wouldn't say wrtc is "easily added to dependencies", but it's entirely possible to build for those that really want it.

I've added it as an optional dependency, and updated the index-node.js to reflect the supported features.

[HenrikJoreteg]

@fippo any thoughts on this?

[fippo]

Looking good (the bundle needs to be updated though), but wouldn't that pull wrtc as a dependency even in the browser?

[bukzor]

I don't know what "the bundle" is (newbie here).

npm does install optional dependencies by default, but imo that's npm's fault. I'm told to use --no-optional skip any optional dependencies. https://www.npmjs.org/doc/files/package.json.html#optionaldependencies

Let me know if you find a better pattern for optional dependencies.

[bukzor]

Ah I now realize that you meant webrtcsupport.bundle.js. It seems to be updated already. To my understanding, it wouldn't include the -node code.

[bukzor]

The npm optionalDependencies I believe are most like the debian Suggests, while we want something more similar to Recommends.

https://www.debian.org/doc/debian-policy/ch-relationships.html

The best option for now is probably to take wrtc out of optionalDependencies and document the relationship elsewhere.

[bukzor]

After some lengthy discussion on #npm isaacs suggested moving this from optionalDependencies to the README, and if necessary, make another package that depends on both.

Agree?

[fippo]

ah... I think @legastero shows us how to do it in https://github.com/henrikjoreteg/getusermedia -- basically the browser and index entries in package json. That means there would be two different versions, one for node and the other for the browser, but as long as they're in the same package maintenance is easy.

[HenrikJoreteg]

As much as possible I always vote for the simplest solution. Isaac is probably right in this case. A simple other package that just extended this might be cleaner.

[omphalos]

Another option might be to allow the consumer of this library to pass in the wrtc module to webrtcsupport if they have/want it available. The advantage is that there would only be one library for webrtcsupport but the dependency wouldn't have to be baked into the package.

[fippo]

@omphalos isn't that now (since #19) solved by requiring wrtc prior to this module?

[omphalos]

@fippo I like that change but I don't see how that would help a Node process that wants to use wrtc. To me it looks like if I do require('webrtcsupport') in Node I always get index-node.js. (I might be missing something.) So I would think that there could be a change made to index-node.js that allows consumers of that file to optionally inject the wrtc dependency, if they want it.

[omphalos]

@fippo Actually I see now. Reading the linked issue I realize that the user can optionally call require('wrtc') or require('webrtcsupport') depending on what they want. That makes a lot of sense to me, and it's probably better than what I was suggesting.

[daviddias]

Hey there! I had to this externally in IPFS and although it does work, I would rather just count on this module to tell me the truth.

I actually added an extra clause in IPFS to check if the platform is Linux or Windows, so that the user explicitly says they want to use it, as it is typically cumbersome to install it in those runtimes. See: https://github.com/ipfs/js-ipfs/blob/master/src/core/components/init.js#L66-L78

Any hopes of getting this featured added in?