Skip to content

Commit

Permalink
Merge pull request #3803 from jridgewell/removeModels-regression
Browse files Browse the repository at this point in the history
Fix _removeModels regression
  • Loading branch information
jashkenas committed Sep 25, 2015
2 parents bd0fbde + 4b64eeb commit 979adf5
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 1 deletion.
6 changes: 6 additions & 0 deletions backbone.js
Original file line number Diff line number Diff line change
Expand Up @@ -1103,6 +1103,12 @@
this.models.splice(index, 1);
this.length--;

// Remove references before triggering 'remove' event to prevent an
// infinite loop. #3693
delete this._byId[model.cid];
var id = this.modelId(model.attributes);
if (id != null) delete this._byId[id];

if (!options.silent) {
options.index = index;
model.trigger('remove', model, this, options);
Expand Down
3 changes: 2 additions & 1 deletion test/collection.js
Original file line number Diff line number Diff line change
Expand Up @@ -305,12 +305,13 @@
deepEqual(col.pluck('id'), [1, 2, 3]);
});

test("remove", 10, function() {
test("remove", 11, function() {
var removed = null;
var result = null;
col.on('remove', function(model, col, options) {
removed = model.get('label');
equal(options.index, 3);
equal(col.get(model), undefined, '#3693: model cannot be fetched from collection');
});
result = col.remove(d);
equal(removed, 'd');
Expand Down

0 comments on commit 979adf5

Please sign in to comment.