-
Notifications
You must be signed in to change notification settings - Fork 89
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
WebRTC connectivity guide #377
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
I just made Vercel previews public, so anyone opening this PR should be able to preview |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is great. Some thoughts:
- Nothing here is browser specific, all of this applies to using the WebRTC transport to dial browsers from Node.js so it might be worth mentioning that?
- Dialling browsers from non-browsers is only possible in js-libp2p right now, that's not really touched on here
- A lot of time is spent discussing how to automatically discover the listener peer's address, including using GossibSub to do so
- It's rare to want to dial a specific peer for no reason, normally you want to dial a peer because they have something you want like an IPFS block, in which case you would have found their address during a search for providers
- GossipSub is a big dep that causes the node to do a lot of work, it's inclusion comes with a lot of overhead
- Language/diagrams could be simplified by using roles (e.g.
Dialer
/Listener
) instead ofPeer 1
/Peer 2
- The role of the
relay
/bootstrapper
becomes a bit overloaded since it's used for connection establishment but also peer discovery- Maybe it could be two separate nodes to make it clear that the peer discovery part isn't a hard dependency of establishing the connection, it's just a way for the dialler to not have to copy/paste the multiaddr of the listener?
- Alternatively peer discovery could be moved out of scope here and covered elsewhere?
- The name "bootstrapper" is a bit confusing since what we normally refer to as "bootstrappers" are nodes run to enable populating routing tables
- The context switch between js and go for the relay is a bit jarring, also considering people reading this will like be JS people, but it is covered at the end of the article.
- It's a little odd that we recommend using go-libp2p for the relay because it has WebTransport, but WebTransport is still pretty broken in browsers. WebSockets, although tedious to configure, do work very well and are available everywhere. Not sure there's a good answer here.
This is pretty cool. I never thought about it this way, but will definitely weave this into the guide.
Do you think Relay is a better term? Which term would you recommend here?
Fair point. I'll give writing the relay in node.js with WebSockets. Since the guide is only running in localhost, we don't need a certificate, so this may work well. |
Co-authored-by: Alex Potsides <alex@achingbrain.net>
@achingbrain Mind giving this another review. I've addressed all your feedback and think this is ready to be merged. |
Great job!! I cleaned up passive voice as best as I could.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is great!! I just cleaned up some passive voice for clarity.
LGTM. Send it. |
This PR adds a new guide on WebRTC with js-libp2p for browser-to-browser connectivity.
TODO
Tasks