Automated cleanup of Commands and Requests?

[rfbowen]

See: http://backbonejs.org/#Events-listenTo

In the case of Backbone's eventing system, listeners are registered on objects and cleaned up automatically on destruction. Such a feature would be a really nice addition to Backbone.Radio, removing the work involved in managing clean-up of every registered event, command, and request handler.

For example, in a view that needs to exchange information with a flash player:

var SomeView = Backbone.View.extend({
  initialize: function() {
    var flashChannel = Backbone.Radio.channel('flash');

    this.listenTo(flashChannel, 'tick', this.onTick);
    this.comply(flashChannel, 'play', this.play);
    this.reply(flashChannel, 'currentTime', this.currentTime);
  },
  onTick: ...
  play: ...
  currentTime: ...
});

If it simplifies matters, perhaps when needing to hand a channel in as an argument the functions can be given new names that indicate the intention to register them for later destruction - complyWith and replyTo, to go along with listenTo.

Lastly, we could wrap the remove function to include stopComplying and stopReplying functions, in addition to stopListening.

Thoughts?

[jamesplease]

I like this feature a lot – it makes a ton of sense to me. But I have some questions/concerns about the implementation.

listenTo works because it makes sense to mixin Backbone.Events with your objects, and listenTo is part of the Events API. But adding commands and requests makes less sense. There's not much reason to use any other API methods of Radio/Events directly on your Object.

This is bad because it makes Requests/Commands slightly less consistent with Events, which is something I'd like to avoid if possible.

So maybe we make separate helper mixins for these methods. I think it makes more sense to include those methods in Marionette than it does Radio.

But the real question is which Classes we mix them into. Right now Marionette doesn't modify Models/Collections. But these would be just as useful on those Classes as they would be views. In fact, if we didn't extend models and collections with these helpers, then people would probably become confused.

So yup, I'm not too sure what's best. Maybe it should be in a separate helper library.

[rfbowen]

I'm not sure I understand why mixing Requests/Commands in with objects is not sensible. Any given object I might have could certainly benefit from being able to reply to and/or issue commands and requests, no?

As far as what classes they should be mixed into, I'd apply the same pattern used to incorporate listenTo functionality. Being that the goal is to have these things consistent with events, it seems sensible to keep the experience of using them consistent.

[jamiebuilds]

Well... They're definitely useful in terms of Commands/Requests in the way that Events is, in the sense that you'd want that functionality on many objects. However, without the stopListening equivalent inside of Radio, it loses the cleanup that you have throughout Backbone. There's also no easy way of adding that functionality in a clean way.

In terms of channel communication, there's not much we can do for automated cleanup, simply because channels live forever, and you'd have to find a way to dereference the requests/commands/events at the correct time. We currently have the convience method reset. However, that's a brute force tactic that isn't going to work for every case.

The problem is that multiple objects can by replying or complying and each of those objects are created and destroyed at different times. It'd be nice to have a function that could remove commands/requests/events by context. ie.

channel.removeEverythingRelatedTo(this);

buuuut... I'm not sure how to go about implementing that.

[rfbowen]

From the annotated source it doesn't seem the implementation of listenTo/stopListening is out of reach for commands and requests - there would just need to be a similar registry kept for them. Admittedly I've only glanced over this code, so if there's a particular tidbit that renders it implausible I could have easily overlooked it.

If we are thinking of just Radio and not Marionette as a whole - there could be a mixin to use for the object of concern (for example, Backbone.View). The remove function (http://backbonejs.org/docs/backbone.html#section-131) could be wrapped up to also call the stopComplying and stopReplying methods after being executed - leveraging the internal registries of things being complied with and replied to by that object on calls to .complyWith(object, 'command', callback) and .replyTo(object, 'request', callback) - where it's possible for object to be a channel.

As far as channels, I haven't thought much about that. I'm okay with them persisting indefinitely - I'm more interested in being able to easily unwire all of the compliers, repliers, and listeners with easy calls and/or have them cleaned up when a view is cleaned up with a .remove(). This seems doable within the context of the object with a little bookkeeping internal to each object.

Let me know if I'm misunderstanding anything - I really do appreciate you taking the time to entertain my request!

[jamiebuilds]

So to be clear for this thread, we're not talking about adding something to the channel object itself.

myObject = _.extend({}, Backbone.Events);
// add some listeners...
myObject.stopListening(); // removes everything related to 'myObject' (even from other objects)

myObject.channel = Backbone.Radio.channel('myChannel');
// add some listeners...
myObject.channel.stopListening(); // removes everything related to 'myChannel' not 'myObject' (not including other objects)

What we want is a set of functions like this:

myObject = _.extend({}, Backbone.Radio.somethingToMixin);
myObject.listenTo(myChannel, 'myEvent', myObject.callback);
myObject.replyTo(myChannel, 'myRequest', myObject.callback);
myObject.complyTo(myChannel, 'myCommand', myObject.callback);

myObject.stopListening(); // removes everything related to 'myObject' (even from other objects)
myObject.stopReplying(); // ditto / (name conflict)
myObject.stopComplying(); // ditto / (name conflict)

@jmeas I'm also thinking that the current names stopReplying and stopComplying need to be changed because they aren't matching the functionality of stopListening.

on	once	off	trigger	listenTo	listenToOnce	stopListening
reply	replyOnce	?	request	replyTo	replyToOnce	stopReplying
comply	complyOnce	?	command	complyTo	complyToOnce	stopComplying

[jamesplease]

One thing to keep in mind here is that Requests and Commands are one-to-one, whereas Events is one-to-many. This matters because if we are to add a listenTo analogy, it wouldn't be compatible with using reply for that given request. This is unlike listenTo, which simply tacks on on additional handlers for each event. Consequently, if you attempted to replyFor to something that had already been set up with reply, it would need to replace the old one (which might be difficult to debug) or throw an error. But maybe that isn't such a bad idea.

@thejameskyle, in response to your suggestion that the method names aren't consistent: I disagree. Pub-sub implementations are interesting because of the use of the verbs on and off, which are opposites of one another. It's a limitation of my vocabulary (or perhaps the English language) that there's no direct analogy for Requests and Commands. Because of this we're stuck (for now) using the more verbose stopXing for even the 'regular' API. For the 'secondary' API, I think I'd prefer using the preposition for, giving us the complete API below:

on	once	off	trigger	listenTo	listenToOnce	stopListening
reply	replyOnce	stopReplying	request	replyFor	replyForOnce	stopReplyingFor
comply	complyOnce	stopComplying	command	complyFor	complyForOnce	stopComplyingFor

Regarding my comments above that mixing in Requests/Commands onto any object doesn't make sense...let me try to explain with an example.

// Extend the prototype of a model class
_.extend(MyModel.prototype, Radio.Requests);

// Create a new instance
var myModel = new MyModel();

// Set up a reply
myModel.reply('data', myModel.getData);

// ...later, in some other file we make a request
myModel.request('data');

That sort thing is what seems silly to me. To take advantage of the registered Request we've had to pull in the object directly. At this point, you could just as easily do myModel.getData(); directly. The whole point of Radio, I think, is to use a mediator (the channel) to keep your objects separated.

However, if we were to add replyFor and complyFor...then things change quite a bit!

myModel.replyFor(myChannel, 'data', myModel.getData);

That seems really nice to me, assuming stopReplyingFor is called during the cleanup process for you (which it would be).

I think my problem before was that I was feeling the tension of mixing an event system into an object (like a view or model) directly versus using it on a standalone mediator, like Channel. When you've got a Channel, it really only makes sense to use the listenTo and replyFor methods. And if what I said above made any sense, then mixing Requests and Commands on an object directly doesn't really do you much good. The conclusion that I drew, and what had me worried, is that there's no reason to have the 'first' API once you've got the 'second' for Requests and Commands.

In any event, I'm beginning to come around to adding these listenTo methods for Requests and Commands.

The one issue from before – and I guess one that's really just a concern for Marionette – still remains about what to do about mixing in Requests and Commands into the data Classes. Should we finally add Marionette.Model and Marionette.Collection? I'm not sure.

If we are thinking of just Radio and not Marionette as a whole - there could be a mixin to use for the object of concern (for example, Backbone.View).

Very true. For some reason, though, that just doesn't seem like something I'd want to include in Radio. If we went with something like that I'd want it to be in a separate repository or something. I'm not sure why other I feel this way other than the fact that I like how Radio is only dealing with messaging-related things right now, and not about how its implemented. But maybe I shouldn't be so worried about this.

From the annotated source it doesn't seem the implementation of listenTo/stopListening is out of reach for commands and requests.

You're absolutely right. It'd be pretty simple, really. I should clarify that my concerns aren't anything to do with the technical feasibility of adding this functionality. It was more about how it would be used, if it would be consistent, and so on.

[jamesplease]

Mmk...think I sold myself on this. It'll be in v0.8.0.

[jamiebuilds]

@jmeas since replyFor and complyFor aren't exactly 1-1 with listenTo (accepts a string as the first argument for a channel name), should there be listenFor or do we modify the behavior of listenTo?

[jamesplease]

You know, I was actually thinking that it shouldn't accept a string...just to keep things consistent and simple at the expense of some convenience.

[jamiebuilds]

Inconceivable!

[jamesplease]

v0.8.0 is the next version of Radio, and I'd like to release it in 6 weeks (Nov. 13th). I can most likely get around to this eventually, but if anyone wants to take a stab at opening a PR that'd be really awesome.

[jamesplease]

I'm on this :fist:

[jamesplease]

This will be released as 0.9.0

[jamesplease]

Almost done! Should be released this week...finally :)

[pascalpp]

Looks like I'm late to this discussion, but fwiw it seems to me that the 'off' equivalents should have 'off' in the name. e.g.

once => replyOnce / complyOnce
off => replyOff / complyOff

but maybe that ship has sailed if people are already using stopReplying / stopComplying..

[jamesplease]

@pascalpp, thanks for the suggestion! When we were figuring out the API, we definitely thought about trying to stay as closely as possible to Backbone.Events. One thing that bugged me about using the same exact verbs is that it makes the method names a bit awkward when taken out of context. If you remove Backbone.Events, and think about the Commands API in isolation, complyOff reads strangely to me. stopComplying, on the other hand, is correct grammar. A similar case can be made for the preposition to use for the listenTo equivalents. In some situations, though, it's even worse than reading awkwardly. Such an API can actively obscure what the method does, I think. Consider replyTo. That could just as easily be the method name for the 'regular' API, I think, where an object is responding to an event on itself. To me, replyFor communicates more clearly the fact that some obj is replying for some other target.

Although the naming of the methods is different, the current API is consistent with Events in behavior. Once the PR that solves this problem lands, each Events method will have an analogous method in Commands and Requests. This is a good thing, because all three systems are fairly similar. But if you were to compare the 3 against each other, Requests and Commands make sense to pair togetehr (for more: #186). Events has many differences when compared against those two systems, so it's a delightful consequence that by selecting an API that reads well, it also encodes this natural pairing, even if it is by chance.

There are definitely trade offs with either decision, to be sure, but I'm pleased with the API we've settled on. If you're unconvinced, then we should continue talking! Maybe you're onto something that I'm not picking up on.

[pascalpp]

@jmeas that makes sense. you're right that it's not good to force things to use the same semantics when the behavior under the hood is different.

[oskbor]

What is the status of this feature? I am looking forward to using it :)

[jamesplease]

WIP. The implementation in #157 needs to be rewritten 'cause of mem leaks and other bugs in Backbone's original implementation of listenTo.

[jamesplease]

If anyone wants to try their hand @ this, you can find the new listenTo implementation in Backbone's 1.2 branch, and semi-complete unit tests in #157. I doubt I'll have time to do it anytime soon.

[jamesplease]

I don't use Radio much these days, so I'm still open to PRs!

[kilbot]

Hi @jmeas, this is not related to the issue .. but I was curious why you are not using Radio much these days? ie: is it because the projects you are working on don't require messaging or are you using a different library? .. just curious :smile: