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

Return type of pubsub.asynctIterator should include AsyncIterable #261

Open
hayes opened this issue Aug 4, 2022 · 2 comments
Open

Return type of pubsub.asynctIterator should include AsyncIterable #261

hayes opened this issue Aug 4, 2022 · 2 comments

Comments

@hayes
Copy link

hayes commented Aug 4, 2022

the pubsub.asyncIterator method currently is typed as returning an AsyncIterator. While this is accurate, it misses an important detail:

The subscribe function of a graphql field expects an AsyncIterable, not an AsyncIterator. At runtime this works because the PubSubAsyncIterator also implements the AsyncIterable interface. Unfortunately this is hidden by the explicit return type in pubsub.asyncIterator Which is typed as returning an AsyncIterator, not a PubSubAsyncIterator, and does not include the [Symbol.asyncIterator] property.

This library is very commonly used when implementing graphql subscriptions, and I believe this has resulted in it becoming common to mis-type the expected return type of subscribe functions (for example in nexus)

This should be a relatively easy, and backwards compatible fix

@kefniark
Copy link

kefniark commented Sep 14, 2022

An ugly hack for anyone else running into it

You can redefine your own withFilter and ResolveFn, with as described above AsyncIterator => AsyncIterable

import { withFilter as withFilterOriginal, FilterFn } from 'graphql-subscriptions'

type ResolverFn<TSource = any, TArgs = any, TContext = any> = (
  rootValue?: TSource,
  args?: TArgs,
  context?: TContext,
  info?: any
) => AsyncIterable<any>

export const withFilter = withFilterOriginal as unknown as (
  asyncIteratorFn: ResolverFn,
  filterFn: FilterFn
) => ResolverFn

It's a really annoying issue, because it's a pure typing problem and it's probably a one liner to fix here

@Aeolun
Copy link

Aeolun commented Aug 14, 2023

For those not interested in switching to a whole different server to fix a minor issue, I've published a fork here: Aeolun/graphql-subscriptions-continued that basically just releases the release-3.0 branch in this repository.

It doesn't update the dependent packages (e.g. graphql-redis-subscriptions), but if they feel like it they can switch.

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

No branches or pull requests

3 participants