type-graphql
type-graphql copied to clipboard
MichalLytek
Async Subscription topic filters
Describe the solution you'd like
I'd like to be able to put authorization checks inside the subscription topics option so that I can dynamically block or allow users on subscriptions. Right now my auth checks are asynchronous but the SubscriptionTopicFunc doesn't accept Promises.
Describe alternatives you've considered
It's possible to put the authorization checks in the filter method or in the subscription logic itself, however this means that the authorization checks will fire for every event instead of just once at subscription time which is much more expensive.
Additional context Add any other context or screenshots about the feature request here.
Unfortunately, it's not possible. graphql-subscriptions requires that the topics function return the AsyncIterator synchronously (ResolverFn).
https://github.com/apollographql/graphql-subscriptions/blob/master/src/with-filter.ts#L7-L8
Please open an issue on their repo about adding support for async ResolverFn in withFilter helper - then after deps update it will available and working without any changes on TypeGraphQL side.
Opened https://github.com/apollographql/graphql-subscriptions/issues/213
I'm currently looking into this limitation and the one from #617 myself and I don't quite understand the reliance on graphql-subscriptions here. They purely export the ResolverFn type, which is insufficient for the use case, as returning a Promise from the subscribe handler is generally supported.
When changing the definition for the ResolveFn in Subscription.ts, my code seems to behave as expected:
export declare type ResolverFn = (rootValue?: any, args?: any, context?: any, info?: any) => AsyncIterator<any> | Promise<AsyncIterator<any>>;
graphql-js will correctly await the provided handler.
The withFilter from their package is similarly limited. However, this can be worked around by simply providing a custom replacement.
As graphql-subscriptions is unmaintained and consumers like Apollo Server are moving to other libraries as well, I feel this should be addressed in type-graphql if at all possible.
Is there anything I'm missing here?
Is there anything I'm missing here?
Yes.
I don't quite understand the reliance on graphql-subscriptions here
TypeGraphQL was created 3 years ago. graphql-subscriptions was the most popular lib for subscriptions back then.
Introducing some other replacement, especially custom one, requires a lot of effort and is a breaking change for all the users.
That's fair, but the package seems to purely provide PubSub, withFilter and a few types. This issue seems to only exist due to an incomplete type and allowing an additional return type on ResolveFn shouldn't be a breaking change for any current consumers of type-graphql from what I can see.
Their PusbSub implementation and withFilter would still behave exactly the same way as before.
In my project I just do as any because it's working with async.
But without upstream fix I can't update TypeGraphQL code.
The library has been abandoned long ago. This is never going to happen. There is no reason to type subscribe in SubscribeOptions as ResolveFn, as graphql-subscriptions does not consume this function. Their ResolveFn type is only relevant for their consumption of the handler in withFilter. It may appear convenient to reuse it here, but what should dictate the correct type is graphql-js.
If that isn't compelling enough to change this behavior, then I'm probably out of luck. Depending on an abandoned library shouldn't be something to be happy about in the first place though.
I have in plans for vNext to reimplement subscriptions from scratch, both on API (decorators) level and inside internals. We now don't need graphql-subscriptions at all with well-supported async-iterators:
https://github.com/apollographql/graphql-subscriptions/issues/240#issuecomment-767568596
But this is a breaking change as now you have put redis-based pubSub and it's working. That's the promise of v1.x which I can't break.