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

Remove dependency on starscream in favor of URLSession #3289

Open
pokryfka opened this issue Nov 8, 2021 · 6 comments
Open

Remove dependency on starscream in favor of URLSession #3289

pokryfka opened this issue Nov 8, 2021 · 6 comments
Labels
feature-request Request a new feature

Comments

@pokryfka
Copy link

pokryfka commented Nov 8, 2021

Starting from iOS13 URLSession supports web sockets.

Please consider removing dependency on starscream

@jgale
Copy link

jgale commented Nov 9, 2021

I second this request, for a minor reason:

  • I need the AWS Amplify SDK, and use it via SPM
  • AWS Amplify SDK depends on this package (aws-appsync-realtime-client-ios)
  • This package depends on starscream up to the next minor version from 3.3.1
  • That currently depends on swift-nio-zlib-support which is deprecated. This leads to a warning in my Xcode project that I can't suppress:
ignoring declared target(s) 'swift-nio-zlib-support' in the system package

At a minimum, perhaps it could be updated to the latest version of starscream which no longer depends on swift-nio-zlib-support, unless running on Linux.

@lawmicha
Copy link
Contributor

Hi @pokryfka and @jgale , we've upgraded to Starscream 4.0.4 and it removes swift-nio-zlib-support. I believe Starscream is building native websocket support as well. From what I can tell, Starscream will use native websockets with a conditional version check for iOS 13 in the future. This seems to make the migration to native websockets less compelling.

AWS Amplify SDK depends on this package (aws-appsync-realtime-client-ios)

This is true regardless of which plugins are used and is due to the Amplify library being a monorepo. I believe this can be fixed in the future if we move the API plugin (the only plugin that uses this package) to its own repo, and is impacting customers which aren't using DataStore/API at all, and still have to pull in the dependency.

We're open to this discussion and would like to ask, are there more reasons to migrate off of Starscream or additional details for the consideration to use native websockets directly?

@jgale
Copy link

jgale commented Apr 15, 2022

Thanks for the detailed reply @lawmicha. Now that it's been upgraded to Starscream 4.0.4, my swift-nio-zlib-support warning is gone so I am happy. Thank you!

I can't speak to @pokryfka 's opinions, I don't have any knowledge or experience with the different websockets libraries.

@pokryfka
Copy link
Author

pokryfka commented Apr 19, 2022

Hi @pokryfka and @jgale , we've upgraded to Starscream 4.0.4 and it removes swift-nio-zlib-support. I believe Starscream is building native websocket support as well. From what I can tell, Starscream will use native websockets with a conditional version check for iOS 13 in the future. This seems to make the migration to native websockets less compelling.

AWS Amplify SDK depends on this package (aws-appsync-realtime-client-ios)

This is true regardless of which plugins are used and is due to the Amplify library being a monorepo. I believe this can be fixed in the future if we move the API plugin (the only plugin that uses this package) to its own repo, and is impacting customers which aren't using DataStore/API at all, and still have to pull in the dependency.

We're open to this discussion and would like to ask, are there more reasons to migrate off of Starscream or additional details for the consideration to use native websockets directly?

Starscream using native websockets is one more reason to remove dependency on Starscream.

That is as long as you drop support for iOS12 and earlier.

Personally, with iOS16 (beta) to be announced in 1.5 months, I think requiring iOS13 and later is reasonable;
especially that it bring a lot of language changes which would make AppSync API more modern and much more convenient to use; I mean Combine and async/await with async functions and sequences (the latter was formally introduced in iOS15 but we have backward compatibility in Swift back to iOS13).

@pokryfka
Copy link
Author

Any chance to have it done? :-D

This is a major change and requires proper testing but the actual code change should be simple as you abstracted WSS client in AppSyncWebsocketProvider.

Other than reducing dependencies, it improves compatibility with other libraries and tools.
For example I use Proxyman for debugging network calls but AppSync subscription messages are not recorded as you use 3rd party library for networking.

@pokryfka
Copy link
Author

Please note aws-amplify/aws-appsync-realtime-client-ios#90

@atierian atierian transferred this issue from aws-amplify/aws-appsync-realtime-client-ios Oct 11, 2023
@thisisabhash thisisabhash removed the pending-triage Issue is pending triage label Jan 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request Request a new feature
Projects
None yet
Development

No branches or pull requests

4 participants