search

apollographql / graphql-subscriptions

:newspaper: A small module that implements GraphQL subscriptions for Node.js
MIT License
1.59k stars 133 forks source link

Change methods/types from AsyncIterator to AsyncIterable #194

Closed stephentuso closed 2 years ago

stephentuso commented 5 years ago

After opening #192, I looked into it more and saw that graphql-js is actually expecting an AsyncIterable, not AsyncIterator, to be returned from the subscribe function: source.

This PR updates the types, methods, and docs to reflect that. This is a breaking change. As mentioned in https://github.com/leebyron/iterall/issues/49, TS can't recognize $$asyncIterator as Symbol.asyncIterator, so type assertions had to be used in a few places

apollo-cla commented 5 years ago

@stephentuso: Thank you for submitting a pull request! Before we can merge it, you'll need to sign the Meteor Contributor Agreement here: https://contribute.meteor.com/

grantwwu commented 5 years ago

So as I'm understanding it, AsyncIterables are objects which you can get an asyncIterator out of using the $$asyncIterator function. So technically our asyncIterators should always have been an asyncIterator in the first place?

So this is a breaking change in the sense that we're changing the types, but all working implementations should already be AsyncIterables, right?

grantwwu commented 5 years ago

Also, this seems like it's related to https://github.com/apollographql/graphql-subscriptions/pull/147 ...

stephentuso commented 5 years ago

Oops - didn't do my homework on this one. Yes, this is basically the same as #147 without the changes re. #143. It is breaking because types and method names changed, you are correct though, it looks like all the implementations are already returning AsyncIterables.

Seems like I should probably just close this in favor of #147?

Akxe commented 4 years ago

How about this would look into implementing #223 (generic types) too?

hwillson commented 2 years ago

It looks like the work here has been superseded by https://github.com/apollographql/graphql-subscriptions/pull/232 (which is coming in 3.0. Let us know if anything is missing, and sorry we didn't get to this contribution sooner!