Subscription

[NeedleInAJayStack]

Adds subscription support using RxSwift observables. Implementation mirrors graphql-js.

[paulofaria]

Amazing work @NeedleInAJayStack. Thank you very much. I think we should provide this as separate module. I think this would be better since this opens up the possibility of other reactive stream implementations like OpenCombine and ReactiveSwift.

[NeedleInAJayStack]

Thanks @paulofaria! I think providing it as a separate module makes sense, and it will be pretty easy to split out. A few questions:

• Any suggestions on package names? I'm thinking GraphQLSubscriptions, but I could be more specific about it using RxSwift if you'd like.
• Should we have a separate subscription package for Graphiti too?

[paulofaria]

I think it's best to be specific and go with the package/module name GraphQLRxSwift. About Graphiti, yeah, we should do the same, package/module name GraphitiRxSwift. The repository names can be GraphQL+RxSwift and Graphiti+RxSwift. I invited you to the team. Let me know if you are able to create repositories. You can copy the README, LICENSE files, etc from this repo and adjust to your liking. Thanks again, @NeedleInAJayStack.😄

[NeedleInAJayStack]

Hey @paulofaria, I removed the RxSwift dependency from this pull request by wrapping all the required functionality in an EventStream class. That way, we can keep the subscription functionality in this repo (since it requires some lower-level private methods like collectFields), while not requiring RxSwift directly. I've also changed the Graphiti pull request to use EventStream and remove the RxSwift dependency.

I've created a new GraphQLRxSwift repo that contains an RxSwift-specific implementation of EventStream. Implementations for other reactive stream packages should be pretty trivial.

Hopefully that's acceptable! Definitely happy to discuss if you like.Thanks!

[NeedleInAJayStack]

Hey @paulofaria, let me know if there's anything else you need from me on this.

[paulofaria]

Sorry @NeedleInAJayStack. Been super busy. Will check before the end of the week!

[NeedleInAJayStack]

No worries @paulofaria - I know how it goes. Just wanted to make sure you weren't waiting on me for anything. Thanks again!

[paulofaria]

LGTM! Sorry for the delay!

[paulofaria]

Whys is merging still blocked? Looks like all tests are passing and I approved the request.

[NeedleInAJayStack]

@paulofaria Thanks for reviewing!! I'm not sure why it's not merging. I do see that the 'macos-latest' test doesn't seem to be completing... Maybe that's what's holding it up?

It's weird because I don't see this macos-latest test specified in the github build.yml. Do you have any ideas on why GitHub thinks this test needs to run?

[NeedleInAJayStack]

@paulofaria, I tried to figure out why merging wasn't working today, but I don't think I have permissions to see the automatic merging configuration, and I can't see any reason why the checks are waiting for macos-latest (since it's not defined in the build.yml file).

The pull request does say "This pull request can be automatically merged by project collaborators". Are you able to merge it?

Thanks again for your review!

[paulofaria]

I'll just go ahead and force merge it. Thank you very much.