set respects input order in absence of comparator

vaclavbohac · Apr 4, 2013 · d12e4cf · d12e4cf
1 parent 9c9463c
commit d12e4cf
Show file tree

Hide file tree

Showing 2 changed files with 30 additions and 7 deletions.
diff --git a/backbone.js b/backbone.js
@@ -666,6 +666,8 @@
       var sortable = this.comparator && (at == null) && options.sort !== false;
       var sortAttr = _.isString(this.comparator) ? this.comparator : null;
       var toAdd = [], toRemove = [], modelMap = {};
+      var add = options.add, merge = options.merge, remove = options.remove;
+      var order = !sortable && add && remove ? [] : false;
 
       // Turn bare objects into model references, and prevent invalid models
       // from being added.
@@ -675,14 +677,14 @@
         // If a duplicate is found, prevent it from being added and
         // optionally merge it into the existing model.
         if (existing = this.get(model)) {
-          if (options.remove) modelMap[existing.cid] = true;
-          if (options.merge) {
+          if (remove) modelMap[existing.cid] = true;
+          if (merge) {
             existing.set(model.attributes, options);
             if (sortable && !sort && existing.hasChanged(sortAttr)) sort = true;
           }
 
         // This is a new model, push it to the `toAdd` list.
-        } else if (options.add) {
+        } else if (add) {
           toAdd.push(model);
 
           // Listen to added models' events, and index models for lookup by
@@ -691,24 +693,26 @@
           this._byId[model.cid] = model;
           if (model.id != null) this._byId[model.id] = model;
         }
+        if (order) order.push(existing || model);
       }
 
       // Remove nonexistent models if appropriate.
-      if (options.remove) {
+      if (remove) {
         for (i = 0, l = this.length; i < l; ++i) {
           if (!modelMap[(model = this.models[i]).cid]) toRemove.push(model);
         }
         if (toRemove.length) this.remove(toRemove, options);
       }
 
       // See if sorting is needed, update `length` and splice in new models.
-      if (toAdd.length) {
+      if (toAdd.length || (order && order.length)) {
         if (sortable) sort = true;
         this.length += toAdd.length;
         if (at != null) {
           splice.apply(this.models, [at, 0].concat(toAdd));
         } else {
-          push.apply(this.models, toAdd);
+          if (order) this.models.length = 0;
+          push.apply(this.models, order || toAdd);
         }
       }
 
@@ -723,7 +727,7 @@
       }
 
       // Trigger `sort` if the collection was sorted.
-      if (sort) this.trigger('sort', this, options);
+      if (sort || (order && order.length)) this.trigger('sort', this, options);
       return this;
     },
 

diff --git a/test/collection.js b/test/collection.js
@@ -996,6 +996,25 @@ $(document).ready(function() {
     collection.set({}, {parse: true});
   });
 
+  test('`set` matches input order in the absence of a comparator', function () {
+    var one = new Backbone.Model({id: 1});
+    var two = new Backbone.Model({id: 2});
+    var three = new Backbone.Model({id: 3});
+    var collection = new Backbone.Collection([one, two, three]);
+    collection.set([{id: 3}, {id: 2}, {id: 1}]);
+    deepEqual(collection.models, [three, two, one]);
+    collection.set([{id: 1}, {id: 2}]);
+    deepEqual(collection.models, [one, two]);
+    collection.set([two, three, one]);
+    deepEqual(collection.models, [two, three, one]);
+    collection.set([{id: 1}, {id: 2}], {remove: false});
+    deepEqual(collection.models, [two, three, one]);
+    collection.set([{id: 1}, {id: 2}, {id: 3}], {merge: false});
+    deepEqual(collection.models, [one, two, three]);
+    collection.set([three, two, one, {id: 4}], {add: false});
+    deepEqual(collection.models, [one, two, three]);
+  });
+
   test("#1894 - Push should not trigger a sort", 0, function() {
     var Collection = Backbone.Collection.extend({
       comparator: 'id',