georapbox / angular-PubSub

Angular 1.x implementation of the Publish–Subscribe pattern.
MIT License
32 stars 13 forks source link

subscriber callback being called n-times after publish #3

Closed walhow closed 7 years ago

walhow commented 8 years ago

For each subscriber I add, I get back that many hits to each callback. For example, if I have 3 subscribing directives, each directives callback gets hit 3 times, for a single publish. Any idea how to fix this? The tokens are unique. So if one of my directives had a controller called historyCtrl, and I use the console statements from the readme, the tokens are unique: 0, 3, 6

I was really hoping each subscriber would just be called once. Thanks!

walhow commented 8 years ago

Found the problem. Needed to implement an 'on destroy' method, to clean up the listeners. My directives were being initialized multiple times, hence the multiple responses. Maybe sending an id with the subscriptions could be used to prevent multiple calls to the same listener?

georapbox commented 8 years ago

Hi,

This is a common issue with all one page apps. Things need to be cleared when leaving a view. This could also happen if you had subscribed to a DOM event or in case of Angular if you had set up a watcher. You would have to make sure that you unsubscribed every time our scope is destroyed. My point is that even if I passed to the callback an id, if you did't unsubscribe the listeners you would end up with many subscriptions every time your directives initialized. And this would cause memory leaks. That's why unsubscribe exists in the first place. Please correct me if I misunderstood something of what you said above. I would also like to hear more about how you would try to not triggering many times the event using an id passed to the callback.

Thanks

walhow commented 8 years ago

You've got a good point there, the ID is not a good approach, unsubscribe is better. Now I've got an issue trying to figure out how to fire the destroy event for the directives, when the user changes the 'page'. Note: found the issue, I just needed the .on listener in the link definition.