Cannot unbind event listener if initially bound with module name.

[matthew-andrews]

If you set an event listener and specify the module name as the second parameter you cannot unbind it.

function onEventName() {
  console.log('my event');
}
layout.on('eventname', 'strawberry', onEventName);

// This will not unbind the above event:
layout.off('eventname', 'strawberry', onEventName);

// And neither will this:
layout.off('eventname', onEventName);

layout.module('strawberry').fire('eventname');
// Expected => nothing
// Actual => 'my event' appears in console log

(You can copy this code and run it on http://ftlabs.github.io/fruitmachine/examples/todo/)

[wilsonpage]

Ah! Guessing that is because internally we wrap the user's callback and bind with a different function. This means that off can't find a matching callback :(

On Thu, Apr 3, 2014 at 2:55 PM, Matt Andrews notifications@github.comwrote:

If you set an event listener via:-

function onEventName() { console.log('my event');}layout.on('eventname', 'strawberry', onEventName); // This will not unbind the above event:layout.off('eventname', 'strawberry', onEventName); // And neither will this:layout.off('eventname', onEventName); layout.module('strawberry').fire('eventname');// Expected => nothing// Actual => 'my event' appears in console log

(You can copy this code and run it on http://ftlabs.github.io/fruitmachine/examples/todo/)

Reply to this email directly or view it on GitHubhttps://github.com/ftlabs/fruitmachine/issues/73 .

[matthew-andrews]

I think this should be relatively easy to fix by maintaining a listener map.

Something like:

var listenerMap = {};

exports.on = function(name, module, cb) {
  var l;

  // cb can be passed as
  // the second or third argument
  if (arguments.length === 2) {
    cb = module;
    module = null;
  }

  // if a module is provided
  // pass in a special callback
  // function that checks the
  // module
  if (module) {
    if (!listenerMap[name]) listenerMap[name] = [];
    l = listenerMap[name].push({
      orig: cb,
      cb: function() {
        if (this.event.target.module() === module) {
          cb.apply(this, arguments);
        }
      }
    });
    cb = listenerMap[name][l-1].cb;
  }
  events.prototype.on.call(this, name, cb);
  return this;
};

exports.off = function(name, module, cb) {

  // cb can be passed as
  // the second or third argument
  if (arguments.length === 2) {
    cb = module;
    module = null;
  }

  if (module) {
    listenerMap[name] = listenerMap[name].filter(function(map, index) {

      // If a callback provided, keep it
      // in the listener map if it doesn't match
      if (cb && map.orig !== cb) {
        return true;

      // Otherwise remove it from the listener
      // map and unbind the event listener
      } else {
        events.prototype.off.call(this, name, map.cb);
        return false;
      }
    });
  } else {
    events.prototype.off.call(this, name, cb);
  }

  return this;
};

This would also give us support for:-

layout.off('eventname', 'strawberry');

(Disclaimer: Not tested any of the above)