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

Introduce experimental real time channels #77

Closed
wants to merge 18 commits into from
Closed

Introduce experimental real time channels #77

wants to merge 18 commits into from

Conversation

hpk42
Copy link
Contributor

@hpk42 hpk42 commented Apr 22, 2024

Superseeds #75

this PR introduces an experimental realtime API for webxdc apps.
The corresponding implementation in Delta Chat is at deltachat/deltachat-core-rust#5346

src-docs/spec/sendRealtimeData.md Outdated Show resolved Hide resolved
src-docs/spec/sendRealtimeData.md Outdated Show resolved Hide resolved
**Note that this API is experimental and not fully settled (April 2024)**

Send `Uint8Array` data to other realtime peers for this app.
You must first call [`setRealtimeListener`](./setRealtimeListener.md)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know of any other web APIs that behave this way where an event listener has to be added first before any corresponding events can be emitted.

I can't think of any use-cases where you'd want to send but not listen for responses, but this seems like something that could be very frustrating for any devs that overlook this warning.

I assume the reason is that you'd like to avoid instantiating the ephemeral messaging stack unless it's required. Perhaps that stack could be launched on sending a message as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good point -- i had thought about it but one iteration before the one you commented, we had setRealtimeListener return a promise that resolved only when a first peer is connected. So there was a way to do a sendRealtimeData after waiting for the promise to resolve and be optimistic that at least one peer will see it.
Now, that we removed the promise, we could as well trigger the setup of the realtime machinery on sendRealtimeData as well even though it's almost guaranteed the sent data will not land anywhere if we are just starting the machinery. @link2xt what do you think? You were the most critical about the promise IIRC.

Copy link
Contributor Author

@hpk42 hpk42 Apr 22, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

after some more thought, i addressed the comment and made the two methods symmetric in the sense that they both implicitely trigger the setup of realtime connections.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think there is no need to even specify when the stack is initialized. You can just call both methods whenever you want and API should do something meaningful, i.e. try to initialize the connection instead of dropping the message right away.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if we keep connection setup totally implicit i wonder how would an app be able to ever force disconnection from peers? Ensuring that no code in an app still tries to send data may not be easy which would implicitely set it up again. I think it's better to have some explicit way to manage connectivity. If we also want to avoid having a dependency of top-level send and receive APIs then we should consider this API:

const realtimeSession = webxdc.joinRealtimeSession(receiveCallback)
and then having realtimeSession.sendData() and realtimeSession.disconnect() methods.
There would be no way to send data or leave a session without first joining, and it's natural that after disconnect the session is invalidated and you need to start fresh to reconnect and send/receive data.
This API design gives both explicit control over connectivity, and does not need a reminder to only send data after you registered a receive-listener.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here is my last comment implemented in a separate PR: https://github.com/webxdc/website/pull/78/files

@hpk42
Copy link
Contributor Author

hpk42 commented Apr 24, 2024

closing this, as #77 is now the head runner and includes the commits of this PR here.

@hpk42 hpk42 closed this Apr 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants