feat: enable withFilter ResolverFn to return Promise

apollographql / graphql-subscriptions

:newspaper: A small module that implements GraphQL subscriptions for Node.js

MIT License

1.59k stars 133 forks source link

feat: enable withFilter ResolverFn to return Promise #197

Closed janhalama closed 2 years ago

janhalama commented 5 years ago

Use case: asyncIteratorFn of withFilter is place where I want to authorize subscription, authorization should not be implemented in GraphQL resolvers, I want to delegate authorization logic to the business layer which is in our case implemented in another service, that is why I need to change result of ResolverFn to Promise<AsyncIterator> to be able to await for the result of the service call in asyncIteratorFn handler

emaciel10 commented 5 years ago

Any reason why this one hasn't been merged in? We feel like it would be really useful for our workflow as well!

grantwwu commented 5 years ago

Isn't this a breaking change?

simonsterckx commented 5 years ago

Would like to have this functionality too! We could just check if the return value is a promise before resolving it to ensure backward compatibility.

janhalama commented 5 years ago

@simonsterckx @grantwwu thanx for your comments, please see my latest update to this PR, I moved required functionality into new function withFilterAsync, withFilter function remains unchanged.

emaciel10 commented 5 years ago

@simonsterckx @grantwwu bump! Anything I can do to keep this PR moving? It would be great if we could get this functionality merged in

grantwwu commented 5 years ago

Hey, until I see more activity from Apollo with regards to helping me do code reviews and participating in the maintenance of this - there doesn't seem to have been a post in the #contributing channel of the Apollo Spectrum chat for several months now - I'm going to put my participation on hold here, for two reasons:

Even though I can merge stuff in, I can't release it on NPM. So there's no point in my merging it.
I recently left my job which uses GraphQL subscriptions. I'm going to be joining a company in November which most likely does not.

hwillson commented 2 years ago

It sounds like this was addressed in https://github.com/apollographql/graphql-subscriptions/pull/220 (which is coming in 3.0). Let us know if not - thanks for this contribution (and sorry for the delay!).