Make a setupFunction return an object

[quintstoffers]

See #10 for previous communication. Solves #7.

This change requires a setup function to return an object containing the properties filter and channelOptions. Both are optional, so no breaking changes in that regard. However, there is no backwards compatibility for the "old style" function-type return value, so existing usages need to be updated to return an object with filter property.

Code example taken from #10, adapted to object-style return value:

{
    setupFunctions: {
        // Taken from GitHunt.
        commentAdded: (options, args) => ({
            commentAdded: {
                filter: comment => comment.repository_name === args.repoFullName
            }
        }),
        // Proposed structure.
        commentUpdated: (options, args) => ({
            commentUpdated: {
                // These options could be passed to PubSub.
                channelOptions: {
                    repoFullName: args.repoFullName
                },
                // Technically redundant since we specified repoFullName above and thus should
                // only receive relevant messages. However, it's here for completeness' sake
                filter: comment => comment.repository_name === args.repoFullName
            }
        })
    }
}

Note that I added a dev dependency to sinon to be able to assert channelOptions are passed into pubsub.subscribe.

[helfer]

@quintstoffers thanks!

@davidyaha do you think you could review this PR?

[davidyaha]

Will do.

[davidyaha]

Other than those two remarks I've it looks good!

[davidyaha]

Can be done on line 159 like so let {channelOptions = {}, filter} = trigger;

[quintstoffers]

Whoops, will fix!

[davidyaha]

There is no actual need for this trigger constant anymore

[davidyaha]

Oh also, it might be a good idea to create an inteface for the returned object on the trigger function.
something like:

interface TriggerConfig {
  channelOptions?: Object
  filter?: Function
}

[quintstoffers]

@davidyaha: Thanks for your input! I removed the unnecessary variable and defined setupFunctions and children using Interfaces.

[davidyaha]

You'r very welcome! I saw you decided not to use the es6 style default values, is it on purpose?

Thanks for your work so far! I would like to use the improved API :)

[quintstoffers]

I'm not a big fan of the default values when destructuring because I often find the default values are too complex and I end up with a lump of hard to read code. I didn't give it a good thought for that reason, but it does look perfectly fine in hindsight:

// Deconstruct the trigger options and set any defaults
const {
    channelOptions = {},
    filter = () => true,
} = triggerMap[triggerName];

As a bonus the variables can now be const, which is nice. I'll update the PR in a minute!

[quintstoffers]

@helfer: I think were done here 😄 .

[helfer]

Thanks a lot! @quintstoffers @davidyaha

[glasser]

FYI, GitHunt-API (the best way to understand how to use Apollo and graphql-subscriptions) has not yet been updated with this API change.