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

.stopListening can cause callbacks added via .on to be removed as well #4210

Open
moloko opened this issue Mar 5, 2019 · 2 comments
Open

Comments

@moloko
Copy link

moloko commented Mar 5, 2019

OK so I'm not sure if this is really a bug or just something that needs clarifying in the documentation... but here goes.

I have some code that uses a mixture of .on and .listenTo to setup event handlers. This is because I want to use .stopListening to remove all the event handlers added via .listenTo whilst keeping those that I added via .on.

What I found is that if you pass this as the the context to .on then calling .stopListening will also cause the event handlers added via .on to be removed as well - when I would expect that calling .stopListening would remove only those event handlers added via .listenTo (or .listenToOnce).

I have set up an example of this in codepen. The idea is that you should be able to:

  • click 'setup events' to start it listening to the 'change' button being clicked
  • click 'destroy events' to stop it from listening to the 'change' event (via .stopListening), and then
  • click 'setup events` again to start it listening to the change button again

But what actually happens is that the call to .stopListening removes all the event handlers.

As you can see in the commented out code, not passing this as the context (but instead using Function.bind to set it) means works fine i.e. the call to .stopListening then only removes event handlers added via .listenTo.

@paulfalgout
Copy link
Collaborator

So this is effectively how listenTo works

listenTo = function(obj, eventName, callback) {
  obj.on(eventName, callback, this);
}

So stopListening is similar such that it offs the context of this.

on and listenTo are not different event bus's but listenTo is in effect sugar for safer syntax.

Perhaps there is room to improve the docs, though the stopListening documentation says "remove just the events it's listening to on a specific object" which is precisely what is happening. I'm not sure there is any reason to assume from the docs that what is made through on and listenTo are different "events" Also as you've seen not specifying the listeners context removes the specified object and binding solves the issue.

@moloko
Copy link
Author

moloko commented Mar 6, 2019

@paulfalgout thanks for clarifying the functionality, that is kinda what I suspected was going on TBH which is why I didn't assume it was a bug.

I do think some clarification in the docs would help. The part you refer to actually reads:

or be more precise by telling it to remove just the events it's listening to on a specific object

Which, as I understand it, refers to when you pass an object to it i.e. view.stopListening(model) - which is not what I'm doing.

I think it could use some clarification where it says

call stopListening with no arguments to have the object remove all of its registered callbacks

The word 'registered' links to the docs for .listenTo, which naturally leads one to assume that a 'registered callback' is one you added via .listenTo.

Perhaps it would be better to link to a proper definition of 'registered callback'? Something like: "a registered callback is one that is added via .listenTo or .listenToOnce - or via .on or .off when context is specified"?

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

3 participants