fix(docs): update webrtc instructions for node in faq #3183
Conversation
They were out of date and cause confusing errors to be thrown.
| }, | ||
| config: { | ||
| transport: { | ||
| WebRTCStar: { |
There was a problem hiding this comment.
It'd be really nice if libp2p could make these keys case-insensitive or throw when an unknown key is encountered - webRTCStar vs WebRTCStar is easy to get wrong.
There was a problem hiding this comment.
We will work on improving the config per libp2p/js-libp2p#576. It will probably change a lot the way we config libp2p. But referencing so that we have this in attention
There was a problem hiding this comment.
I just noticed in our config docs that we have this:
const transportKey = WebRTCStar.prototype[Symbol.toStringTag]Should we use it here to avoid confusions on where the naming comes from?
There was a problem hiding this comment.
Actually I'm not sure if that makes it any clearer. It would be vastly preferable to be able to just use [WebRTCStar.tag] the same as the peer discovery config key.
There was a problem hiding this comment.
We should rething that in the config issue for libp2p. From what I know, the tag was added for the discovery side of things. The transports do not include it, except for webrtc-star, as it is both transport and discovery.
Anyway, we will look into it in a future libp2p release
| @@ -88,8 +86,14 @@ const node = await IPFS.create({ | |||
| }, | |||
| libp2p: { | |||
| modules: { | |||
| transport: [wstar], | |||
| peerDiscovery: [wstar.discovery] | |||
There was a problem hiding this comment.
Do we want to remove discovery?
There was a problem hiding this comment.
From the old instructions you have to new up a WebRTCStar instance to get the discovery property, but now that throws an error about providing an upgrader.
Could you make a suggested change to correct the config here please?
There was a problem hiding this comment.
peerDiscovery: {
[WebRTCStar.tag]: {
enabled: true
}
}
FYI I also did #3092 a while ago that uses the tag's instead of the string to avoid errors like the case sensitive that you mentioned earlier, but this is only available for discovery.
There was a problem hiding this comment.
also PRd libp2p to use the tag in this missing doc: libp2p/js-libp2p#713
There was a problem hiding this comment.
Is #3092 ready for review/merging? It's been draft for ages.
achingbrain
changed the title
They were out of date and cause confusing errors to be thrown.