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

Swift Concurrency Support #100

Conversation

NeedleInAJayStack
Copy link
Member

@NeedleInAJayStack NeedleInAJayStack commented Jul 14, 2022

This adds support for Swift concurrency by adding new top-level async public APIs.

It also adds a Swift concurrency-based EventStream. This allows us to directly test the subscription functionality within this package, and allows clients to use GraphQL subscriptions without coding custom EventStreams (which typically required drivers like GraphQLRxSwift)

Long term, we probably want to remove the EventStream workaround and just use the swift concurrency types by default. This has been left as a future task though, because it would require a breaking API change and require all clients to be using swift versions that support concurrency.

@paulofaria
Copy link
Member

Hey, J! This all looks great. Thank you for finally looking into async/await support. I'm really out of time these days. If you have some time, it would be nice if you took a look at the work I have in GraphQLSwift/Graphiti#71. Besides async/await, it also adds support for client and server directives, which I think it's pretty neat. To implement it, I had to do some work here, which is in the same branch name feature/async-await. One issue I had was with the visitor/reducer. I see there is some outstanding PRs working on something related? If you're interested, would be nice to consolidate all of these efforts. Take a look at the branches and let me know what you think. I also had some ideas about improving the API and bring it much closer to the GraphQL SDL, but those would be mostly cosmetic, so we can postpone that, for sure.

@paulofaria
Copy link
Member

If you have no interest in looking into that, that's cool too. 😄

@paulofaria
Copy link
Member

By the way, feel free to merge the PR and tag a new release. 😊

@NeedleInAJayStack NeedleInAJayStack merged commit 3071750 into GraphQLSwift:master Jul 14, 2022
@NeedleInAJayStack
Copy link
Member Author

Oh woah, adding directives like that looks awesome!! I'll have a look over the next week or two for sure!

@d-exclaimation
Copy link
Member

Sorry to suddenly jump here. I was planning to add support to the new ConcurrentEventStream to my library, Pioneer, but I found some issues with the current implementation of .mapStream, which I explained in #101. I have proposed a solution for it in #102 but it needs to be reviewed.

@NeedleInAJayStack
Copy link
Member Author

No need to be sorry at all, @d-exclaimation. This work was my first foray into Swift async/await, so I'm glad to have someone watching my back 😄. I'll jump on and review for sure.

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