apollographql / graphql-subscriptions

:newspaper: A small module that implements GraphQL subscriptions for Node.js
MIT License
1.59k stars 133 forks source link

Update with-filter.ts #103

Closed srtucker22 closed 7 years ago

srtucker22 commented 7 years ago

I think this is might be what I’m looking for.

Let’s say my GQL sub is something like:

type Subscription {
    # Subscription fires on every message added
    # for any of the groups with one of these groupIds
    messageAdded(groupIds: [Int]): Message
  }

Right now, my withFilter resolver might look like this:

  messageAdded: {
      subscribe: withFilter(
        () => pubsub.asyncIterator(‘messageAdded’),
        (payload, args, ctx) => {
          return myFilter(payload, args, ctx);
        },
      ),
  }

That’s pretty cool, but this means that every subscriber to messageAdded will get a message and then filter for whether to publish to client based on the myFilter function. Not very scalable :/

The most obvious way to fix this to me is to publish more distinct topics:

      subscribe: withFilter(
        (payload, args, ctx) => {
          const topics = getTopicsFromArgs(args);
          return pubsub.asyncIterator(topics);
        },
        (payload, args, ctx) => {
          return myFilter(payload, args, ctx);
        },
      ),

Now pubsub.publish will publish something like messageAdded.1 for a message added to Group 1, and only users subscribed to messageAdded.1 will receive the message and run it through myFilter. But currently asyncIteratorFn doesn't have access to the args, context, etc to make distinct topic(s) like messageAdded.1

So while this fix is a small bit of code, I think it could make withFilter a much more scalable helper function :)

srtucker22 commented 7 years ago

doesn't seem like this code is affecting the build failing :/

NeoPhi commented 7 years ago

Can you rebase this PR against the most recent master to see if it resolves the issue.

srtucker22 commented 7 years ago

We cool now?