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

Embed Starscream 3 #53

Merged
merged 1 commit into from
Nov 11, 2021
Merged

Embed Starscream 3 #53

merged 1 commit into from
Nov 11, 2021

Conversation

FZambia
Copy link
Member

@FZambia FZambia commented Nov 10, 2021

Relates #48

Looking at Starscream issue tracker migration to StarScream 4 can result in some connection problems. This requires a bit more consideration in general. Maybe there is a better alternative these days?

Since we already have dependency conflicts mentioned in #48 I think for now we can embed Starscream 3 to SwiftCentrifuge.

Removed all public and open access level identifiers.

@fedulvtubudul
Copy link
Contributor

There is an official WebSocket implementation from Apple https://developer.apple.com/documentation/foundation/urlsessionwebsockettask. Possible reason to avoid it for now is that it is iOS 13+ compatible, while centrifuge-swift is declared to support iOS 9+.

So in ideal world I would suggest isolating WebSockets transport into an abstraction with several possible implementations like Starscream 3 for those who needs older systems support or URLSession's WebSockets for those who are ready to drop everything below iOS 13.

@FZambia
Copy link
Member Author

FZambia commented Nov 11, 2021

Yeah, I think moving to the official WS implementation is a good thing. Starscream 4 already does this internally - so I'd say we can use it eventually, but need to research a bit. Maybe there is an actively developed fork of it.

@FZambia FZambia merged commit 79b5f5a into master Nov 11, 2021
@FZambia FZambia deleted the embed_starscream_3 branch November 11, 2021 07:22
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.

2 participants