Skip to content

WebSocket wrapper + WebSocket example update #442

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

Merged
merged 4 commits into from
May 6, 2020

Conversation

MartinKavik
Copy link
Member

Resolves #8

@MartinKavik MartinKavik mentioned this pull request May 5, 2020
11 tasks
Copy link

@thedodd thedodd left a comment

Choose a reason for hiding this comment

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

Looking awesome! Any thoughts on implementing an optional reconnect pattern? I’ve implemented this pattern a year or so ago when I was hacking on Gloo a bit. Happy to grab that code and share it here if you like the idea.

@MartinKavik
Copy link
Member Author

Any thoughts on implementing an optional reconnect pattern?

See https://github.com/MartinKavik/seed/pull/1/files.

I decided to add streams::backoff and use it explicitly in the app with web sockets - that way we can integrate reconnecting also for GraphQL, fetch and other APIs/libs. I think the explicitness + customizability + universality is a good tradeoff for a bit more boilerplate.

@thedodd What do you think?

@thedodd
Copy link

thedodd commented May 6, 2020

Solid.

IMHO, one of the more fundamental libraries needs to build a higher-level WebSocket abstraction, something directly on top of WebSys but which provides the async/await & optional reconnect behavior so that it could be cleanly encapsulated. I would suggest Gloo, but I had some not-so-great experiences trying to get things done there.

I had a working implementation previously, but it was based on fut01 (back in the day). Hopefully I'll have the time to just extract that code and publish as a standalone crate. If it is good, we could potentially bring it under the Seed org as well. Thoughts?

@MartinKavik
Copy link
Member Author

just extract that code and publish as a standalone crate. If it is good, we could potentially bring it under the Seed org as well

How it differs from the code in this PR?
I would rather have one ultimate solution included directly in Seed repo, but external crate for experiments and potential candidate for integration into Seed would be nice - please add the crate into Seed Awesome list once ready.

@thedodd
Copy link

thedodd commented May 6, 2020

Yea, I haven't touched the code in quite some time, but it is still there: https://github.com/thedodd/gloo/tree/49-websocket/crates/websocket I'll try to get it resurrected and in its own crate here soon.

@thedodd
Copy link

thedodd commented May 6, 2020

@MartinKavik the code that I linked to (which is incomplete), fully encapsulates the reconnect logic. It exposes an option to configure reconnects, and then exposes the connection state as a stream or directly readable value. Basically no boilerplate for the user.

We certainly don't need that here, but I figured I would at least bring it up because I had been hacking on it before.

@MartinKavik
Copy link
Member Author

fetch, websocket and others are basically slim wrappers for browser API, written only for Seed. I can imagine we'll add additional functionalities (like built-in reconnection, logging, etc.) and make them standalone in the future - but I suggest to wait and watch how those libs are used in the wild by users.

@MartinKavik MartinKavik merged commit 85fb352 into seed-rs:master May 6, 2020
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.

WebSocket support & example
2 participants