Only pass idAttribute to collection.modelId()

jashkenas · Feb 25, 2016 · b3a2813 · b3a2813
1 parent c713366
commit b3a2813
Show file tree

Hide file tree

Showing 2 changed files with 11 additions and 11 deletions.
diff --git a/backbone.js b/backbone.js
@@ -984,7 +984,7 @@
     get: function(obj) {
       if (obj == null) return void 0;
       return this._byId[obj] ||
-        this._byId[this.modelId(obj.attributes || obj, obj)] ||
+        this._byId[this.modelId(obj.attributes || obj, obj.idAttribute)] ||
         obj.cid && this._byId[obj.cid];
     },
 
@@ -1088,8 +1088,8 @@
     },
 
     // Define how to uniquely identify models in the collection.
-    modelId: function(attrs, model) {
-      return attrs[(model && model.idAttribute) || this.model.prototype.idAttribute || 'id'];
+    modelId: function(attrs, idAttribute) {
+      return attrs[idAttribute || this.model.prototype.idAttribute || 'id'];
     },
 
     // Private method to reset all internal state. Called when the collection
@@ -1129,7 +1129,7 @@
         // Remove references before triggering 'remove' event to prevent an
         // infinite loop. #3693
         delete this._byId[model.cid];
-        var id = this.modelId(model.attributes, model);
+        var id = this.modelId(model.attributes, model.idAttribute);
         if (id != null) delete this._byId[id];
 
         if (!options.silent) {
@@ -1152,15 +1152,15 @@
     // Internal method to create a model's ties to a collection.
     _addReference: function(model, options) {
       this._byId[model.cid] = model;
-      var id = this.modelId(model.attributes, model);
+      var id = this.modelId(model.attributes, model.idAttribute);
       if (id != null) this._byId[id] = model;
       model.on('all', this._onModelEvent, this);
     },
 
     // Internal method to sever a model's ties to a collection.
     _removeReference: function(model, options) {
       delete this._byId[model.cid];
-      var id = this.modelId(model.attributes, model);
+      var id = this.modelId(model.attributes, model.idAttribute);
       if (id != null) delete this._byId[id];
       if (this === model.collection) delete model.collection;
       model.off('all', this._onModelEvent, this);
@@ -1175,8 +1175,8 @@
         if ((event === 'add' || event === 'remove') && collection !== this) return;
         if (event === 'destroy') this.remove(model, options);
         if (event === 'change') {
-          var prevId = this.modelId(model.previousAttributes(), model);
-          var id = this.modelId(model.attributes, model);
+          var prevId = this.modelId(model.previousAttributes(), model.idAttribute);
+          var id = this.modelId(model.attributes, model.idAttribute);
           if (prevId !== id) {
             if (prevId != null) delete this._byId[prevId];
             if (id != null) this._byId[id] = model;

diff --git a/test/collection.js b/test/collection.js
@@ -1650,7 +1650,7 @@
     // Default to using `model::idAttribute` if present.
     Stooge.prototype.idAttribute = '_id';
     var model = new Stooge({_id: 1});
-    assert.equal(StoogeCollection.prototype.modelId(model.attributes, model), 1);
+    assert.equal(StoogeCollection.prototype.modelId(model.attributes, model.idAttribute), 1);
 
     // Default to using `Collection::model::idAttribute` if model::idAttribute not present.
     StoogeCollection.prototype.model = Stooge;
@@ -1735,11 +1735,11 @@
     });
     var c2 = new C2({'_id': 1});
     assert.equal(c2.get(1), c2.at(0));
-    assert.equal(c2.modelId(c2.at(0).attributes, c2.at(0)), 1);
+    assert.equal(c2.modelId(c2.at(0).attributes, c2.at(0).idAttribute), 1);
     var m = new M({'_id': 2});
     c2.add(m);
     assert.equal(c2.get(2), m);
-    assert.equal(c2.modelId(m.attributes, m), 2);
+    assert.equal(c2.modelId(m.attributes, m.idAttribute), 2);
   });
 
   QUnit.test('#3039 #3951: adding at index fires with correct at', function(assert) {