Sorting a Backbone Collection in an Add event handler re-fires the Add event

Question

I have a collection which sorts itself based on a model's attribute, sometimes I want to add a model and just calculate the order within the add event handler. The problem is that when I call collection.sort() within the add event handler, for some reason it fires the add event again.

Since my add event does some DOM inserts, I end up getting duplicate items in my DOM.

So far the only solution I have found is to calculate the next order before adding the model or calling _.defer and sorting the collection then running any further code

See full fiddle: http://jsfiddle.net/DD23n/9/

Ah event storms! The gentle clapping of your software dying. Backbone's design has a high degree of this occurring because of how its designed. Mainly the over use of event bindings to attach all forms of logic. — chubbsondubs, Jun 07 '12 at 19:33
@chubbard: This isn't an event storm, this is an "I'm changing the array that I'm iterating over" storm. — mu is too short, Jun 07 '12 at 20:02
You're splitting hairs (maybe as a joke, but still its a trivial objection). Changing the array dispatches an event, which triggers more logic that changes the array, dispatching more events, so and so on. All caused from automated events dispatching and binding locked in a death grip choke hold that backbone is all to happy to offer up as "best practice." — chubbsondubs, Jun 08 '12 at 03:12
@chubbard: No, this isn't a problem of events triggering events at all, not even close. The problem is, specifically, that Clay was modifying some that was currently being looped over, the fact that an event dispatcher was doing the looping is wholly immaterial. Just because you don't like event based systems doesn't mean they're always at fault. My correction was just that: a correction for your mistaken comment. — mu is too short, Jun 08 '12 at 16:49
I don't like the way Backbone advocates, or lack of giving any direction really, using events because I think it creates messes like this. It's not event based systems in general. Don't assume I'm biased. — chubbsondubs, Jun 08 '12 at 18:00

score 4 · Accepted Answer · edited May 23 '17 at 12:12

sntran is right about where your problem is coming from: you're modifying an array while you're iterating over it.

You start off with your new model with nOrder: null at the beginning of the model list:

splice.apply(this.models, [index, 0].concat(models));

then add loops through the models triggering 'add' events as it goes:

for (i = 0, length = this.models.length; i < length; i++) {
  if (!cids[(model = this.models[i]).cid]) continue;
  options.index = i;
  model.trigger('add', model, this, options);
}

But inside your 'add' callback, you modify the model:

if(model.get('nOrder') == null)
    model.set('nOrder', _.max(collection.pluck('nOrder')) + 1);

and then sort the collection:

collection.sort({silent: true});

These two actions move this.models[0] to this.models[3] in the event triggering loop; but the i for that loop will just keep ticking along and the new this.models[3] (which used to be at 0) will get past the if (!cids[(model = this.models[i]).cid]) test again and there's your second 'add' event.

You can watch this version of your fiddle work to see how the collection's array is changing behind your back:

http://jsfiddle.net/ambiguous/p8Fp4/

I think the simplest solution is to add an append method to your collection that sets the appropriate nOrder value on the model and then adds it to the collection:

append: function(m) {
    var nOrder = _.max(this.pluck('nOrder')) + 1;
    if(m instanceof Backbone.Model)
        m.set({ nOrder: nOrder });
    else
        m.nOrder = nOrder;
    this.add(m);
}

Then your 'add' callback can leave nOrder alone and stop sorting the collection.

Demo: http://jsfiddle.net/ambiguous/JqWVP/

You could also override the collection's add method but that is a lot more complicated if you want to do it right.

Thanks, that's a great solution - ya I did some further testing and this is surely an ugly race condition of sorts. Originally I thought that when I called `sort` from within the add handler, it fired add events to manipulate the collection - not realizing it was the original add loop. Fun stuff — Clarence Liu, Jun 08 '12 at 16:52

Sơn Trần-Nguyễn · Answer 2 · 2012-06-07T21:31:18.367

2

If you read through the source code of backbone, the add function concatenates the inserting model into collection.models. In this case, you have four.

Because you did not tell the add function to silent, it loops through collection.models and trigger 'add' event for the inserting element.

But you call sort in an add handler, collection.models become different. When the collection does its loop in add function, the inserting model will be at a different position, and it will trigger 'add' again.

I'm not sure it makes sense, but I think it's better that you don't rearrange/modify an array during a loop.

edited Jun 07 '12 at 21:31

answered Jun 07 '12 at 19:25

Sơn Trần-Nguyễn

2,188
1
26
30

But if the `add` is silent then the handler won't be called at all. I think the real solution would be to set the `nOrder` before adding the model so that the `set` and `order` wouldn't be needed in the `'add'` event handler. – mu is too short Jun 07 '12 at 20:06
I think so too. Setting it before adding would let the collection sort by itself without touching the collection in between. – Sơn Trần-Nguyễn Jun 07 '12 at 21:44

Sorting a Backbone Collection in an Add event handler re-fires the Add event

2 Answers2