feat: replace iterall with native Symbol.asyncIterator + fix return types#232
feat: replace iterall with native Symbol.asyncIterator + fix return types#232hwillson merged 5 commits intoapollographql:release-3.0from
Conversation
|
Since this is a clean slate, would you consider changing the input types for |
|
@rh389 Yours could be merged afterwards (if any is ever merged 😅). Doing more stuff in one pr might decrease the chance of it getting merged, so I would like to keep it like that for now 😇 |
|
Yeah, sorry I don't mean to hijack - just that this PR introduces |
|
@glasser I noticed you recently pushed to this repository. Is this repository actively maintained, should new maintainers, or should a deprecation notice be added? A lot of pull requests are still undressed, including this one... |
|
It is not particularly maintained. It is a dependency of Apollo Server 2, for not particularly good reasons: AS does not actually use any code from it directly, but it just re-exports all of its exports. Because of that, as part of making AS2 support I'm tracking figuring out what to do with these projects in apollographql/apollo-server#5141 as part of the AS3 release process. |
|
I don't know whether you saw my comment over here: #240 (comment) Maybe this package should be deprecated and instead a EventEmitter abstraction for different systems such as redis pubsub or mqtt could be used. |
|
@glasser You could have considered this for the v2.0.0 release... It is a shame that the typings are still wrong :/ |
|
@n1ru4l This package is not actively maintained. 2.0.0 was the bare minimum to get it to build with graphql@16. |
BREAKING fixes the type annotation of the abstract class PubSubEngine. According to the TypeScript type-defintion a `PubSubAsyncIterator` instance is actually a `AsyncIterableIterator` instead of an `AsyncIterator`. The typing of `PubSubAsyncIterator` is way more convenient as it allows iterating over it with the `for await (const foo of iterator) { doSth() }` syntax, which is super handy for filtering or mapping (See https://gist.github.com/n1ru4l/127178705cc0942cad0e45d425e2eb63 for some example operators).
…ymbol.asyncIterator (see apollographql/graphql-subscriptions#232)
…ymbol.asyncIterator (see apollographql/graphql-subscriptions#232)
BREAKING fixes the type annotation of the abstract class PubSubEngine. According to the TypeScript type-defintion a
PubSubAsyncIteratorinstance is actually anAsyncIterableIteratorinstead of anAsyncIterator. The typing ofPubSubAsyncIteratoris way more convenient as it allows iterating over it with thefor await (const foo of iterator) { doSth() }syntax, which is super handy for filtering or mapping (See https://gist.github.com/n1ru4l/127178705cc0942cad0e45d425e2eb63 for some example operators).This should be able to get merged once Node.js 10 runs out of LTS. See https://nodejs.org/en/about/releases/
Technically this project already dropped Node.js 10 support when removing the Node.js 10 CI build.
Closes #192