removeListener undocumented, doesn't work?
james-criscuolo opened this issue · 6 comments
Hello,
I'm updating from 2.4.0 to 7.0.0, and it appears removeListener no longer works. I am confident I'm passing the same callback to both on and removeListener, but:
console.error("XXX REMOVING", ref.listenerCount(event));
ref.removeListener(event, callback);
console.error("XXX AFTER REMOVING", ref.listenerCount(event));
within the removeListener function returns the same number. I notice the function is undocumented, which leads me to believe this might be known.
Upon looking at the on function, it appears that the callback is wrapped in another function, which will make this not work, unless that function is saved and tracked. I used to do these calls directly on ev, and while I haven't tried, it looks like I'll probably still be able to on __ev.
Small update, confirmed just using __ev.on and __ev.removeListener works.
removeAllListeners() is called for you on close, so I didn't expose them.
However I'm probably missing the removal on error as well, I just noticed.
Also you should listenerCount() on __ev to verify; on and removeListener are still available and they wrap __ev for you.
The listenerCount call on __ev correctly decrements, and at this point I'm going forward with skipping the wrapper functions. I also realized I said this "no longer works," but as I wasn't using it before, I realize it's possible it never worked.
I need the ability to remove listeners mid-call, so the removeAllListeners on close (and soon error, maybe), will not help. If I wasn't clear above, the summary is that due to the way the on wrapper works, I do no think the removeListener wrapper will ever work in conjunction with it. The removeListener wrapper is fine on its own, but the on wrapper wrapping your callback makes that callback unremovable.
due to the way the on wrapper works, I do no think the removeListener wrapper will ever work in conjunction with it. The removeListener wrapper is fine on its own, but the on wrapper wrapping your callback makes that callback unremovable.
Couldn't agree more. Looks like this is in need of a major overhaul. The whole point of the thing was to add tracing capabilities, but that shouldn't come at the cost of breaking the event-emitter pattern / allowing leaks.
The FreeSwitchResponse object probably should inherit from eventEmitter (might as well use eventemitter2 while we're at it), and once should probably be renamed onceAsync (to keep in line with bluebird's promisify pattern naming conventions). If the object doesn't inherit, then on, emit, removeListener, removeAllListeners should be simple wrappers (but might as well inherit, really).
(There is a need for an overhaul of the client as well due to #44, so we're due for a new major.)
Integrated in v8.0.0