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

feat: enable withFilter ResolverFn to return Promise #197

Conversation

janhalama
Copy link

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
Copy link

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

@grantwwu
Copy link
Contributor

grantwwu commented Sep 3, 2019

Isn't this a breaking change?

@simonsterckx
Copy link

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
Copy link
Author

@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
Copy link

@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
Copy link
Contributor

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
Copy link
Member

It sounds like this was addressed in #220 (which is coming in 3.0). Let us know if not - thanks for this contribution (and sorry for the delay!).

@hwillson hwillson closed this Nov 25, 2021
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

Successfully merging this pull request may close these issues.

6 participants