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

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

Closed
Overtorment opened this issue Jun 12, 2020 · 6 comments
Closed

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

Overtorment opened this issue Jun 12, 2020 · 6 comments
Labels
feature request New feature or request

Comments

@Overtorment
Copy link

We are slowly migrating to this plugin (pr here BlueWallet/BlueWallet#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
Copy link
Owner

@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?

@Rapsssito Rapsssito added the feature request New feature or request label Jun 12, 2020
@Rapsssito Rapsssito changed the title API adapters API adapters (react-native-tcp, net, tls) Jun 12, 2020
@Overtorment
Copy link
Author

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
Copy link
Author

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
Copy link
Owner

Rapsssito commented Jun 16, 2020

@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
Copy link
Author

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

@github-actions github-actions bot added the stale label Jul 22, 2020
Repository owner deleted a comment from github-actions bot Jul 22, 2020
@Rapsssito Rapsssito removed the stale label Jul 22, 2020
@github-actions github-actions bot added the stale label Aug 22, 2020
Repository owner deleted a comment from github-actions bot Aug 22, 2020
@Rapsssito
Copy link
Owner

Moved to #41.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants