-
Notifications
You must be signed in to change notification settings - Fork 26
chore(js-client)!: Simplify/optimize js-client and update README [fixes DXJ-490] #366
Conversation
* limitations under the License. | ||
*/ | ||
|
||
export type Node = { peerId: string; multiaddr: string }; |
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.
@folex really doesn't like the name Node in this context. I pretty much agree. If we move it here maybe we should name it differently. Also we discussed that maybe exposoing only multiaddrs would be enough and a simple function to get peerid out of them (seems like js people would rarelly need peerId directly, I don't know)
Also since now CLI generates a file that contains all these adresses - maybe there is no reason to expose this from the client at all anymore, since you would have to change your source code each time you switch environments, but with CLI controlling this part - you wouldn't have to do that and would only have to import one file
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.
Agree with everything except exposure. This type must be public, because js-client accepts it in public methods.
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.
Maybe we need first of all discuss the problem of sourcing info about the nodes before the merge.
If we agree to distribute the info across the repos?
README.md
Outdated
```javascript | ||
// Passing 1 kras network config from ./dist/network.js above | ||
window.Fluence.connect({ | ||
multiaddr: "/dns4/0-kras.fluence.dev/tcp/9000/wss/p2p/12D3KooWSD5PToNiLQwKDXsu8JSysCwUt8BVUJEqCHcDe7P5h45e", |
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.
In the future, addresses may be changed. And as I discovered @nahsi used to store all currently implemented addresses in the repo file: https://github.com/fluencelabs/fluence-network-environment/blob/main/src/index.ts
Maybe we should link the source info of @nahsi in any places we can?
ps. In other places where the same comment should be written, I will not write the one, but I want to consider the same comment
packages/core/js-client-isomorphic/src/worker-resolvers/node.ts
Outdated
Show resolved
Hide resolved
* limitations under the License. | ||
*/ | ||
|
||
import { Worker } from "threads/master"; |
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.
import { Worker } from "threads/master"; | |
import type { Worker } from "threads/master"; |
No description provided.