Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 7 additions & 2 deletions backbone-nested.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
36 changes: 36 additions & 0 deletions test/nested-model.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});

});