afeld
change event options parameter not detailed enough
The change event handler is passed the model and an options object that contains the attributes that have changed. Because you're calling the super's set(), its just specifying the top level attribute that changed. Ideally, this would specify the actual attributes that changed, similar to Backbone.DeepModel. Basically, there's a loss of information with what actually changed, and binding to the all event to get the change:* events has the problem that its not possible to dedupe changes.
Using model.changedAttributes() doesn't handle this use case because it accumulates all changes to the model
The use case I'm trying to support to syncing a model to a plain old javascript object for use with JsViews, and I would like to do this as efficiently as possible.
I'm guessing this isn't something you'd be able to support, but it would be nice.
I'm pretty sure this is a separate issue. From what I understand:
(1) changedAttributes() will tell you all the attributes that have been changed since the last sync
(2) in the change event handler, function (model, options) {}, options.changes tells you the attributes that have changed in this change
This is at least the functionality I've seen in Backbone (it may change to support the features you're providing in NestedModel). #35 handles (1), but (2) is still an issue. options.changes only contains the top level attribute.
A basic test:
var m = new Backbone.NestedModel({foo: { bar: 0 });
var changes;
m.on('change', function (model, options) {
changes = options.changes;
});
m.set('foo.bar', 1);
deepEqual(changes, { foo: { bar: true } }); // or deepEqual(changes, { 'foo.bar': true });
// right now, changes == { foo : true }
Sorry for the bump, @afeld, but just wondering what your take on this issue was, and if there was any chance of there being an option to provide this type type of information in the callback.
Thanks!
Hey, sorry for not responding sooner. I actually wasn't aware of options.changes - it's not in the general documentation. Wouldn't be a difficult addition, but I probably can't get to it til this weekend. Pull requests are welcome, though :-)
Can you please retest using the backbone 1.x supported backbone-nested that's currently in master? It's possible that with how things are working now, we're getting this now. Thanks.
