diff --git a/backbone-nested.js b/backbone-nested.js index 1d4d657..7b8d84a 100644 --- a/backbone-nested.js +++ b/backbone-nested.js @@ -267,8 +267,13 @@ if (!opts.silent && _.isObject(newValue) && isNewValue){ var visited = []; var checkChanges = function(obj, prefix) { - // Don't choke on circular references - if(_.indexOf(visited, obj) > -1) { + // Not checking changes on sub-Backbone.Models/Collection props + // in order to avoid to put observers on potential heavy objects + // (typically, Backbone.Models have _events properties which may have lots of things inside it, and + // we should not put useless observers on it) + if(obj instanceof Backbone.Model || obj instanceof Backbone.Collection) { + return; + } else if(_.indexOf(visited, obj) > -1) { // Don't choke on circular references return; } else { visited.push(obj); diff --git a/test/nested-model.js b/test/nested-model.js index baed677..c7b32ba 100644 --- a/test/nested-model.js +++ b/test/nested-model.js @@ -1117,4 +1117,40 @@ $(document).ready(function() { equal(model, doc); }); + test("#walkPath() should not observe Backbone.Models properties, you should observe this by yourself directly by binding watchers on submodels", function(){ + var nonAttributeChange = sinon.spy(); + var changeFromRootModel = sinon.spy(); + var changeFromSubModel = sinon.spy(); + + var model = new Backbone.NestedModel({ + submodel: new Backbone.Model({ foo: "bar" }) + }); + + // submodel.cid is amongst the Backbone.Model fields which should not be observable + model.bind('change:submodel.cid', nonAttributeChange); + + // By defining a new Backbone.Model instance, we're implicitely changing submodel.cid field + model.set("submodel", new Backbone.Model(model.get("submodel").toJSON())); + sinon.assert.notCalled(nonAttributeChange); + + // We should avoid setting sub backbone models watchers directly through model props + // because it may lead to lots of observers set up on Backbone.Model props + // particularly _events props which may contain huge objects + model.bind("change:submodel.attributes", changeFromRootModel); + + // Trying to update submodel attributes as well... but this should not be allowed either + // because here, we won't trigger any submodel's event (only model's event) + // We'll see a better way to handle this below... + model.set("submodel.attributes.foo", "bar"); + sinon.assert.calledOnce(changeFromRootModel); + + + // We should rather consider binding the change event directly on the submodel + model.get("submodel").bind("change:foo", changeFromSubModel); + + // This will be far better and will trigger an update this time + model.get("submodel").set("foo", "quix"); + sinon.assert.calledOnce(changeFromSubModel); + }); + });