feat: ephemeral peer channels #6
Conversation
webxdc.d.ts
Outdated
|
|
||
| /** | ||
| * Set a listener for _ephemeral_ status updates. | ||
| * Own status updates are not received. |
There was a problem hiding this comment.
Does this trigger a regular message to announce the peer-address?
Does this wait on anything?
IOW: the docs are not clarifying what a caller can expect in terms of how connectivity happens (same question for setEphemeralUpdateListener.
There was a problem hiding this comment.
Desktop explicitly calls send_gossip_advertisement to send out an advertisement with the current IP address. The async call resolves when at least one peer is connected and the swarm is operatable.
webxdc.js
Outdated
| // browsers. In an actual webxdc environment (e.g. Delta Chat messenger) this | ||
| // file is not used and will automatically be replaced with a real one. | ||
| // See https://docs.webxdc.org/spec.html#webxdc-api |
There was a problem hiding this comment.
this addition does not relate to the PR topic. Why not as a PR to the hello-repo which would diverge as soon as we merge this PR here?
There was a problem hiding this comment.
Hello repo also has a PR. For people who only look at this repo it might be interesting to have a mock implementation as well. The true source is linked so a divergence is acceptable imo.
webxdc.js
Outdated
There was a problem hiding this comment.
this file is wrong, this repo is only about types. if you add it here bundlers will do mess with it.
There was a problem hiding this comment.
From a usability perspective, I find it way better to have a working implementation of webxdc.js . It's always easier to just delete code instead of trying to find a good implementation. Maybe it's also enough to just add a reference to the hello repository implementation.
There was a problem hiding this comment.
make an extra package for that, including it here will not have the effect you want. this package is types only
There was a problem hiding this comment.
having an extra package for the emulator would also make syncing & updating it easier, see webxdc/vite-plugins#2 which also includes the emulator.
currently the emulator lives in the hello repo, the hello repo is a minimal example how to develop apps without npm packages, so it is kinda outside of the rest.
Still this package is only for types and the webxdc.js is only a dummy for the bundlers to make them compile (and not complain about a missing file), as it clearly(?) states in its comment. If you put an emulator there it will be bundled and thus never replaced by the host (deltachat or cheogram), because the bundler inlines it and changes its name. so you would have a non working app, not what you want.
|
I fixed most issue I raised here in #8, please review that and merge it into this pr. |
Fixes for the ephemeral_peer_channels pr (#6)
webxdc.d.ts
Outdated
| @@ -101,7 +101,7 @@ interface Webxdc<StatusPayload, EphemeralPayload = any> { | |||
| * Send an ephemeral update to another peer. | |||
| * @param payload Data that can be serialized with `JSON.stringify`. | |||
| * | |||
| * @returns resolves when (TODO is it when it is sent or registered by core?) | |||
| * @returns A promise that resolves when a peer connection is established. Ephemeral messages can then be sent with `sendEphemeral` | |||
There was a problem hiding this comment.
are you paying attention? this is sendEphemeralUpdate, your edit seems like it was meant for setEphemeralUpdateListener
webxdc.d.ts
Outdated
| active: boolean | ||
| trashed: boolean |
There was a problem hiding this comment.
is one of these private? or "readonly"?
There was a problem hiding this comment.
already marked locally
There was a problem hiding this comment.
just tried it out in the ts playground, seems like typescript needs an explicit private or protected keyword, it is not like rust where everything is private by default. maybe there is a compiler flag for this, but I wouldn't bet on that.
|
On Sun, May 12, 2024 at 09:32 -0700, Sebastian Klähn wrote:
@Septias commented on this pull request.
> serial?: number
): Promise<void>;
+
+ /**
+ * Join a realtime channel.
+ */
+ joinRealtimeChannel(): RealtimeListener;
This should probably even return an error to make misusage more clear
maybe yes -- we would also need to adapt spec then.
`null` is also fine IMO -- if you try to use it to call "setListener" or "send" it will create an error, right?
… --
Reply to this email directly or view it on GitHub:
#6 (comment)
You are receiving this because you commented.
Message ID: ***@***.***>
|
|
Yeah, but that error will be much less helpfull |
Co-authored-by: WofWca <[email protected]>
Part of 5346