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

Web support #53

Closed
wants to merge 6 commits into from
Closed

Web support #53

wants to merge 6 commits into from

Conversation

r-oman-G
Copy link

@r-oman-G r-oman-G commented Sep 2, 2021

Create IWebSocket interface with similar methods in dart:html and dart:io WebSocket. This is have not a big difference between. Test's work normally. Trade-offs noticed: dart:html WebSocket should not use 'protocols' because in this case server will not response. And dart:html WebSocket should use binaryType = 'arraybuffer';

@FZambia
Copy link
Member

FZambia commented Sep 2, 2021

@r-oman-G thanks, looks very promising! Could you please add a detailed pr description to describe things you done and maybe any trade-offs you noticed?

@r-oman-G
Copy link
Author

r-oman-G commented Sep 4, 2021

@FZambia hi, thanks for fast answer. Update pull request

@FZambia
Copy link
Member

FZambia commented Sep 5, 2021

Trade-offs noticed: dart:html WebSocket should not use 'protocols' because in this case server will not response.

Did you find the underlying reason of this? Would be nice to use centrifuge-protobuf subprotocol to avoid using ?format=protobuf in URL.

Also, plz run dartfmt -w lib/ test/ as CI currently fails.

@r-oman-G
Copy link
Author

r-oman-G commented Sep 6, 2021

Did you find the underlying reason of this? Would be nice to use centrifuge-protobuf subprotocol to avoid using ?format=protobuf in URL.

yes, if we do it when we create webSocket then server will not response. Or response something like this "server can't response, try again". I'm try it many times, but anywey dart:html WebSockets can't read our request, if we use protocol during creation WebSockets

Also, plz run dartfmt -w lib/ test/ as CI currently fails.

Okay, I we'll do it tomorrow

@FZambia
Copy link
Member

FZambia commented Sep 7, 2021

"server can't response, try again". I'm try it many times, but anywey dart:html WebSockets can't read our request, if we use protocol during creation WebSockets

On the server side you are using Centrifugo v2 right? Subprotocol only handled by Centrifugo v3 - maybe thats the reason.

@r-oman-G
Copy link
Author

r-oman-G commented Sep 8, 2021

"server can't response, try again". I'm try it many times, but anywey dart:html WebSockets can't read our request, if we use protocol during creation WebSockets

On the server side you are using Centrifugo v2 right? Subprotocol only handled by Centrifugo v3 - maybe thats the reason.

maybe you right, cause i'm test centrifugo only with one server. By the way we use centrifugo v2

@FZambia
Copy link
Member

FZambia commented Sep 19, 2021

Also, plz run dartfmt -w lib/ test/ as CI currently fails.

Still actual. @r-oman-G are you still interested in getting this merged?

@r-oman-G
Copy link
Author

r-oman-G commented Sep 19, 2021 via email

@FZambia
Copy link
Member

FZambia commented Sep 19, 2021

Thx, no rush - just wanted to understand the status.

@FZambia
Copy link
Member

FZambia commented Oct 19, 2021

There was a problem in CI which resulted in error, need to merge master – and tests should pass.

@FZambia FZambia mentioned this pull request Nov 19, 2021
@adimshev
Copy link

Any updates?

@FZambia
Copy link
Member

FZambia commented Sep 2, 2022

This is still awesome to have, but needs to be addressed over latest version of SDK. So whether this PR should be updated or a new one created.

@FZambia FZambia mentioned this pull request Jan 20, 2023
@FZambia
Copy link
Member

FZambia commented Jan 29, 2023

Closing in favour of #73

@FZambia FZambia closed this Jan 29, 2023
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

3 participants