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

Possibility to remove Events from EventEmitter #79

Open
tomhatzer opened this issue Nov 21, 2015 · 7 comments
Open

Possibility to remove Events from EventEmitter #79

tomhatzer opened this issue Nov 21, 2015 · 7 comments
Milestone

Comments

@tomhatzer
Copy link

Hi guys!

I'm still working with coffea and I really like it :)

As I can see, there are 3 events that get created by the ".on()" method.

eg.

function parseCmd(event) {

}
client.on('command', parseCmd);

Normally I could call

client.removeListener('command', parseCmd);

and the event would be gone, but not here. If I do execute the removeListener, nothing happens and the events stay where they are. Nothing gets removed.

Do you have an idea how I can remove the events properly?

Thanks!

Tom

@omnidan
Copy link
Member

omnidan commented Nov 22, 2015

Thanks for finding this, @tomhatzer

We use eventemitter3 which should be 100% compatible with the node event api.

Can you post a gist with example code so I can easily reproduce the issue? 😁

@tomhatzer
Copy link
Author

There are 2 problems from my point of view :)

In file lib/plugins/command.js on line 16-24 you build a custom object that includes a few different variables together with the callback function:

            utils.emit(irc, event.network, 'command', {
                "channel": event.channel,
                "user": event.user,
                "message": event.text,
                "cmd": cmd,
                "args": message,
                "isAction": event.isAction,
                "tags": event.tags ? event.tags : []
            });

In file lib/utils.js on line 73-75 3 events get created (3 times!) with a custom event_obj:

    var ret1 = instance.emit(event, err, event_obj);
    var ret2 = instance.emit(network + ":" + event, err, event_obj);
    var ret3 = instance.emit('event', event, err, event_obj);

Based on those 2 parts of code, it's impossible to remove binds for commands, because I can only guess what network it is bound to and what the contents of event_obj are.

I helped myself by creating a wrapper class for the internal nodejs EventEmitter that does nothing more than bind itself to message and command events and lets plugins bind to the Events from the wrapper class.

  self.init = function() {
    client.on("message", function(event) {
      self.emit("message", event);
    });

    client.on("command", function(event) {
      self.emit("command", event);
    });
  };

Long story short, my wrapper for this: https://gist.github.com/tomhatzer/73b24404195afe79d97d

Example code for bind/unbind:

var cmdHelper = require("CommandHelper.js");
var cb = function(event) {
    console.log(event);
};
cmdHelper.on('command', cb);
cmdHelper.removeListener('command', cb);

@omnidan
Copy link
Member

omnidan commented Nov 24, 2015

@tomhatzer Glad you figured out a workaround 😁 I don't understand how dispatching the event three times makes it impossible to remove listeners.

If you listen to on('command'), you can remove the listener for that, it's totally separate from the network:command event. It's like listening to on('command') vs on('connect') - you can still remove the listener for either event.

@tomhatzer
Copy link
Author

@omnidan :D The listeners for the internal EventEmitter need to be removed using .removeListener('command', callback); Doing this would work usually, but not in that case because I don't have the exact callback function that is used by eventemitter3. Also I don't have the exact context object for calling the eventemitter3 removeListener, which makes it impossible for me to remove the listeners for the 3 binds. There's always 1 variable thats missing or wrong (especially when working with multiple irc servers).

I created a plugin system that gives me the possibility to reload every plugin file on the fly (by removing the require cache entry for that file and loading it again). Which works now thanks to the CommandHelper class :D

Everything else works as intended :)

@omnidan
Copy link
Member

omnidan commented Nov 24, 2015

@tomhatzer how do you not have the exact callback function?

function callback (event) {
  event.reply('do something')
}

client.on('command', callback)
client.removeListener(callback)

That should work fine. You don't bind it three times, I only emit three times, and if you don't listen to network: or event it's not even emitted by the eventemitter. It really shouldn't affect anything.

As I said, a gist of a piece of code to reproduce this issue would be nice. :)

@tomhatzer
Copy link
Author

@omnidan I... I'm sorry, I mixed things up with the emits. :(

Hmmm, you all the removeListener with only 1 parameter.
I always called it with client.removeListener('command', callback);

@omnidan
Copy link
Member

omnidan commented Nov 24, 2015

@tomhatzer Sorry, that was my mistake! You do have to call it with the event name, but it should still work fine like this:

function callback (event) {
  event.reply('do something')
}

client.on('command', callback)
client.removeListener('command', callback)

You won't have to unsubscribe from all three events (command, network:command and event), unless you subscribed to more than one of these.

@omnidan omnidan added this to the 0.6 milestone Feb 11, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants