Rapsssito / react-native-tcp-socket

React Native TCP socket API for Android, iOS & macOS with SSL/TLS support.
MIT License
319 stars 81 forks source link

API adapters (react-native-tcp, net, tls) #51

Closed Overtorment closed 4 years ago

Overtorment commented 4 years ago

We are slowly migrating to this plugin (pr here https://github.com/BlueWallet/BlueWallet/pull/1223) instead of old @aprock react-native-tcp. So far so good! But we will test extensively.

I wanted to raise a concern that this package API does not conform not to nodejs net/tls nor to widely used @aprock react-native-tcp. It's no big deal, I wrote adapters, but this package cant work as a drop-in replacement out of the box. So I was thinking, maybe it's worth including adapters I wrote (https://github.com/BlueWallet/BlueWallet/pull/1223/files#diff-2054cc4c657c92ae593a544fe4dd140d) to this package so it can be a drop-in replacement?

Rapsssito commented 4 years ago

@Overtorment, thanks for your adapters! I will take a look at them. Could you please open a PR so the diff is more readable for me?

Overtorment commented 4 years ago

I can do this next week, or meanwhile you can take a look at my PR, and look for 2 files: blue_modules/net.js & blue_modules/tls.js

Overtorment commented 4 years ago

fyi, we migrated and rolling out to production. I see this package is releasing faster than I can update :-)

FYI had to do some changes in adapters https://github.com/BlueWallet/BlueWallet/blob/master/blue_modules/net.js https://github.com/BlueWallet/BlueWallet/blob/master/blue_modules/tls.js to make it work with 3.7.1 better

Rapsssito commented 4 years ago

@Overtorment, sorry about the releases, I am finally getting some free time to fix and update the package! My objective is to make it as similar as possible to the node net API.

The last update 4.0.0, might make it a lot easier for you. TcpSocket now extends EventEmitter, so there is no need to adapt methods like removeListener, addListener...

Overtorment commented 4 years ago

if it is not improving performance a lot or fixes some major issues I wont update, for now. busy with other stuff, but good job! will plan update on roadmap

Rapsssito commented 4 years ago

Moved to #41.