Change detach to interface #575
Conversation
Pull Request Test Coverage Report for Build 2771
💛 - Coveralls |
backkem
force-pushed
the
detach-interface
branch
from
dac8f77
to
d01de14
Compare
This is in preparation of a JS/WASM shim.
backkem
force-pushed
the
detach-interface
branch
from
d01de14
to
be88c86
Compare
| @@ -328,7 +328,7 @@ func (d *DataChannel) ensureOpen() error { | |||
| // Please reffer to the data-channels-detach example and the | |||
| // pions/datachannel documentation for the correct way to handle the | |||
| // resulting DataChannel object. | |||
| func (d *DataChannel) Detach() (*datachannel.DataChannel, error) { | |||
| func (d *DataChannel) Detach() (datachannel.ReadWriteCloser, error) { | |||
There was a problem hiding this comment.
I'm actually not sure what to say about this change. I probably not know enough of the context. The pattern here is to keep a reference to the webrtc.DataChannel for datachannel properties and use the interface only for read/writes? (I'm not doing that in my code right now). For example, I see the data channel has a StreamIdentifier() that AFAICT we are going to loose. I'm not worried about the StreamIdentifier itself, but seems reasonable to me that the datachannel will hold properties we may want to query.
There was a problem hiding this comment.
Good question.
First off, my original intent for adding the Detach API was to get access to the Read/Write way of using the datachannel since it makes more sense in Go than OnMessage among other reasons. I didn't necessarily want to provide the ability to influence the data channel otherwise since hat can potentially mess with pions/webrtc functionality.
Now for my current goal: I built some functionality on top of this Detach API (backkem/go-libp2p-webrtc-direct) and I'd like to be able to target WASM as well for browser support. Therefore, I'd like to make the Detach API work in WASM as well. This change allows me to create a shim around the browser JS OnMessage API that implements the datachannel.ReadWriteCloser and hopefully keep all these implementation details out of the higher level implementation.
This is in preparation of a JS/WASM shim.