slack-bot-api icon indicating copy to clipboard operation
slack-bot-api copied to clipboard

message event handling

Open leviwheatcroft opened this issue 9 years ago • 2 comments

Hey, I've been using slackbots a bit lately, it's a solid module and I like your coding style. I'd never seen vow before, but I've found it a pleasure to use after bluebird & friends..

Anyhow, I've been using a simple listen method, I haven't bothered to put it in a PR, because I'm not sure if you'd be interested in incorporating it into your module.. but if it matches your vision for slackbots just let me know and I'll submit a PR, but if this doesn't float your boat that's no drama - I just thought I'd ask.

I think it ticks a few boxes in that it allows you to create listeners which are clean and readable.

The listen method I'm using looks something like this..

  bot.listen = function(mask, handler) {
    bot.on('message', function(event) {
      let match
      if (!event.user || event.user == bot.userId) {
        return
      }
      match = _.every(mask, (value, property) => {
        if (_.isRegExp(value)) {
          return event[property].match(value)
        }
        return event[property] === value
      })
      if (!match) {
        return
      }
      handler(event)
    })
  }

Which allows you to add listeners like so:

  bot.listen(
    {
      type: 'message',
      text: /:star:/
    },
    function(message) {
      ...
    }
  )

or ...

  bot.listen(
    {
      type: 'reaction_added',
      reaction: 'star'
    },
    function(event) {
      ...
    }
  )

leviwheatcroft avatar Jul 23 '16 23:07 leviwheatcroft

I like it. My only qualm is subscriptions should be defined globally and iterated over once (and checked as needed) rather than every .listen attaching a new event subscription. Make sense? (As it stands, 3 calls to .listen incur a cost of three .on(...) bindings.)

It would be nice to have something mimic routing in express though. Some place you can apply bindings in a more concise manner with fewer checks/re-checks/etc.

Other than that, almost reminiscent of the good 'ol days and mIRC. ;-) (on *:TEXT:*hello*:#: .msg $user World.)

bchristie avatar Aug 02 '16 22:08 bchristie

It would be easy enough to push these listeners onto a list, then then subscribe to the event only once, and run through the list your listeners.. but would that really improve performance significantly? I mean, subscribing directly to the event is pretty much exactly the same, when the event is thrown node will iterate through the subscribers.

Routing in express is a nice declarative syntax which we all know and love, but I'm not sure it would really work here because messages et cetera aren't ordered the way a url is. Different parameters are already sorted out in the event, so you can directly filter on type or whatever. If we came up with some convention whereby a single string could be used as a mask to filter events, then that's something you'd have to learn / understand to use this module, whereas the mask object in it's current form is kind of immediately obvious.

leviwheatcroft avatar Aug 13 '16 23:08 leviwheatcroft