search

murat-dogan / node-datachannel

WebRTC For Node.js and Electron. libdatachannel node bindings.
Mozilla Public License 2.0
280 stars 52 forks source link

Expose WebSocket functionality of libdatachannel? #233

Closed ptesavol closed 3 months ago

ptesavol commented 3 months ago

Hi

Would it be possible to also expose the WebSocket functionality of libdatachannel in node-datachannel? There are the well-known websocket packages for nodejs (ws, websocket, and uWebSockets), but they all have their own shortcomings when it comes to heavy-duty use cases with hundreds of connections per node.

ptesavol commented 3 months ago

I just created a pull request that brings the WebSocket functionality of libdatachannel into node-datachannel: https://github.com/murat-dogan/node-datachannel/pull/235. This might require a little refactoring and more testing, but let me know what you think.

murat-dogan commented 3 months ago

I'm sorry for not getting back to you sooner. This is a good idea.

For me PR is ok. Just small notes.

ptesavol commented 3 months ago

There is a lot of copy-paste code, but using inheritance seems to be a complex topic when using Node-API, so I didn't go there yet: https://mmomtchev.medium.com/c-class-inheritance-with-node-api-and-node-addon-api-c180334d9902

murat-dogan commented 3 months ago

Hello @ptesavol For me it is ok. Just please solve the typo

ptesavol commented 3 months ago

Hi @murat-dogan Sorry, which typo do you mean? For some reason I cannot see your notes in the PR discussion

murat-dogan commented 3 months ago

Please check conversation tab. There is a typo and conflicts.

ptesavol commented 3 months ago

Unfortunately on my machine I cannot see anything in the conversation tab of https://github.com/murat-dogan/node-datachannel/pull/235. Could you do the small fixes before merging, that might be the easiest way to go?

murat-dogan commented 3 months ago

I merge the PR. Thanks.