{"action":"closed","issue":{"url":"https://api.github.com/repos/jashkenas/backbone/issues/3693","labels_url":"https://api.github.com/repos/jashkenas/backbone/issues/3693/labels{/name}","comments_url":"https://api.github.com/repos/jashkenas/backbone/issues/3693/comments","events_url":"https://api.github.com/repos/jashkenas/backbone/issues/3693/events","html_url":"https://github.com/jashkenas/backbone/issues/3693","id":90728162,"number":3693,"title":"_removeModels regression bug","user":{"login":"kmalakoff","id":756520,"avatar_url":"https://avatars.githubusercontent.com/u/756520?v=3","gravatar_id":"","url":"https://api.github.com/users/kmalakoff","html_url":"https://github.com/kmalakoff","followers_url":"https://api.github.com/users/kmalakoff/followers","following_url":"https://api.github.com/users/kmalakoff/following{/other_user}","gists_url":"https://api.github.com/users/kmalakoff/gists{/gist_id}","starred_url":"https://api.github.com/users/kmalakoff/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/kmalakoff/subscriptions","organizations_url":"https://api.github.com/users/kmalakoff/orgs","repos_url":"https://api.github.com/users/kmalakoff/repos","events_url":"https://api.github.com/users/kmalakoff/events{/privacy}","received_events_url":"https://api.github.com/users/kmalakoff/received_events","type":"User","site_admin":false},"labels":[{"url":"https://api.github.com/repos/jashkenas/backbone/labels/bug","name":"bug","color":"ff0000"}],"state":"closed","locked":false,"assignee":null,"milestone":null,"comments":8,"created_at":"2015-06-24T16:37:33Z","updated_at":"2015-09-25T14:48:48Z","closed_at":"2015-09-25T14:48:48Z","body":"It looks like in 1.2.1, _removeModels was simplified, but introduced a regression bug.\r\n\r\n```\r\n    _removeModels: function(models, options) {\r\n      var removed = [];\r\n      for (var i = 0; i < models.length; i++) {\r\n        var model = this.get(models[i]);\r\n        if (!model) continue;\r\n        // CODE REMOVED HERE\r\n\r\n        var index = this.indexOf(model);\r\n        this.models.splice(index, 1);\r\n        this.length--;\r\n\r\n        if (!options.silent) {\r\n          options.index = index;\r\n          model.trigger('remove', model, this, options);\r\n        }\r\n\r\n        removed.push(model);\r\n        this._removeReference(model, options);\r\n      }\r\n      return removed.length ? removed : false;\r\n    },\r\n```\r\n\r\nRemoved the following lines after `if (!model) continue;`\r\n\r\n```\r\n        var id = this.modelId(model.attributes);\r\n        if (id != null) delete this._byId[id];\r\n        delete this._byId[model.cid];\r\n```\r\n\r\nWhat this means is that if you are listening to a remove event and you check for removal, you will get into an infinite loop because the event is now called before the id is removed from `this._byId`:\r\n\r\n```\r\n    if (current_related_model = collection.get(id)) collection.remove(current_related_model)\r\n```\r\n\r\nI think the _byId code should be put back so when events are fired, they represent the most up-to-date state.\r\n"}}