Pothos subscriptionField + withFilter are breaking the type inference of subscription arguments

[zeuscronos]

I have this very simple dummy project using: Pothos. You can download it from here.

On file: /src/graphql/schema.ts I have the following simple code:

import { withFilter } from 'graphql-subscriptions';
import builder, {
  AppMutationFieldBuilder,
  AppQueryFieldBuilder,
  AppSubscriptionFieldBuilder,
} from './builder';

const NOTIFICATION_CHANNEL_MESSAGE = 'CHANNEL_MESSAGE';

export const GqlMessageEvent = builder.simpleObject('MessageEvent', {
  fields: (t) => ({
    channel: t.string(),
    message: t.string(),
  }),
});

// Queries
// prettier-ignore
{
  builder.queryType({});
  builder.queryField('health', (t: AppQueryFieldBuilder) =>
    t.field({
      type: 'String',
      resolve: () => 'OK',
    })
  );
}

// Mutations
// prettier-ignore
{
  builder.mutationType({});
  builder.mutationField('sendMessage', (t: AppMutationFieldBuilder) =>
    t.field({
      type: 'Boolean',
      args: {
        channel: t.arg.string({ required: true }),
        message: t.arg.string({ required: true }),
      },
      resolve: (_, { channel, message }, context) => {
        context.pubsub.publish(NOTIFICATION_CHANNEL_MESSAGE, {
          messageEvent: { channel, message },
        });
        return true;
      },
    })
  );
}

const IGNORE_MESSAGE_EVENTS = false;
const shouldIgnoreMessageEvents = new Promise((resolve) =>
  resolve(IGNORE_MESSAGE_EVENTS)
);

// Subscriptions
// prettier-ignore
{
  builder.subscriptionType({});
  builder.subscriptionField('messageEvent', (t: AppSubscriptionFieldBuilder) =>
    t.field({
      type: GqlMessageEvent,
      args: {
        channel: t.arg.string({ required: true }),
      },
      subscribe: withFilter(
        (_, __, context) => context.pubsub.asyncIterator([NOTIFICATION_CHANNEL_MESSAGE]),
        async ({ messageEvent }, { channel }) => {
          return (messageEvent.channel === channel) && (!(await shouldIgnoreMessageEvents));
        }
      ),
      resolve: ({ messageEvent }) => messageEvent
    })
  );
}

export default builder.toSchema();

My Problem: Looks like Pothos and withFilter function are not working well together. Then I'm having the 2 issues below:

Issue 1: For the subscription field: messageEvent I'm getting these 2 TypeScript errors:

• Typescript Error 1: Type 'ResolverFn' is not assignable to type 'Subscriber<{}, InputShapeFromFields<{ channel: InputFieldRef<string, "Arg">; }>, any, unknown>'. Type 'AsyncIterator<any, any, undefined>' is not assignable to type 'MaybePromise<AsyncIterable>'.

• Typescript Error 2: Type '({ messageEvent }: { messageEvent: any; }) => any' is not assignable to type 'Resolver<unknown, InputShapeFromFields<{ channel: InputFieldRef<string, "Arg">; }>, any, { channel: string; message: string; }, unknown>'. Types of parameters '__0' and 'parent' are incompatible. Type 'unknown' is not assignable to type '{ messageEvent: any; }'.

Issue 2: The channel argument in the second parameter of withFilter (which is a function) is not actually recognized as an argument when just above I used: channel: t.arg.string({ required: true }). However if I get rid of: withFilter and just use the code below, then channel is recognized as an argument with type: string:

subscribe: (_, { channel }, context) => {
  console.log(channel);
  return context.pubsub.asyncIterator([NOTIFICATION_CHANNEL_MESSAGE]);
}

[hayes]

The way the withFilter method is typed makes inference impossible, which pothos can't fix for you.

I would probably just not use it, and do something like this instead:

builder.subscriptionField('messageEvent', (t: AppSubscriptionFieldBuilder) =>
    t.field({
      type: GqlMessageEvent,
      args: {
        channel: t.arg.string({ required: true }),
      },
      subscribe: async function*(_, { channel }, context) {
        const iter = context.pubsub.asyncIterator([NOTIFICATION_CHANNEL_MESSAGE]);

        for await (const payload of iter) {
          if ((payload.messageEvent.channel === channel) && (!(await shouldIgnoreMessageEvents))) {
            yield payload;
          }
        }
      },
      resolve: ({ messageEvent }) => messageEvent
    })

[hayes]

I didn't test this, but hopefully that will get you going in the right direction

[zeuscronos]

That was the code I initially had but even though it works as expected I had troubles when doing unit tests involving multiple cycles of subscriptions/desubscriptions. I got this warning...

(node:12788) MaxListenersExceededWarning: Possible EventEmitter memory leak detected. 11 CHANNEL_MESSAGE listeners added to [EventEmitter]. Use emitter.setMaxListeners() to increase limit
(Use node --trace-warnings ... to show where the warning was created)

... which is a clear concern for me because I'm afraid of leaving event listeners around in the server corresponding to subscriptions that already finished.

Alternatively you can do this other simple experiment with your proposed code as indicated in the screenshot below, by doing 11 cycles of Subscriptions/Desubscriptions...

... then you will get the warning I mentioned above.

Looks like with that code, if you don't yield the event (because the condition), then even though if the subscription is finished the event listener gets stuck around causing possible memory leaks which is something we don't want.

However, when you use withFilter(...) you don't get that warning. Then I'm assuming that the logic inside: withFilter(...) handles better the situation when the event is not yielded.

[hayes]

I can reproduce the issue you are talking about, but I think this is a bug outside the scope of Pothos. I did notice that I wasn't seeing the same warnings when using yoga instead of apollo.

Pothos just passes the resolve and subscribe functions into the generated schema. I think the place to fix this would be for graphql-subscriptions to improve the types for their withFilter function, so that it can be used properly, but there may also be something interesting happening inside of apollo.

I added some additional logging and what I was seeing was that wrapping the for loop in a try/finally block, it didn't appear to be triggering the return on the async iterator, which is what probably clears up listeners.

If you have anymore insight into whats going on, I'm definitely interested, but I don't think this is something that can be fixed inside pothos, since pothos doesn't really control how these iterators are invoked by the server