Revert change of issue #2846 (regarding options of nested change events)
We recently stumbled about a change that was introduced in version 1.1.1, caused by issue #2846:
- Before that version:
- You could have nested changes with own options - without regarding other listeners to the "change" event.
- You could use these own options in concrete change:attribute callbacks.
- You didn't get these own options in callbacks to a "change" event (which causes the issue)
- Now, since version 1.1.1:
- The options hash has side effects:
- The options hash of the final "change" event depends on the last call to the set method - which can be done by any listener/callback by any other model or view.
- You can never be sure, that the options hash of a "change" callback is replaced by one of a inner change
Notice the problem? Sure, both cases have their problem. But I really think that the second case causes much more new problems than it solves. Because really, what was implemented there is a side effect. You can now accidentally "destroy" the options hash of another listener, which may be set by a very different module in your application. This hurts the principle of encapsulation and modularization...
Here's an example for what happens currently:
// File A
this.listenTo(anyModel, "change", function(model, options) {
options = options || {}; // Hash is always empty
if(options.fooFlag) { // Flag will never exist
doSomething();
}
});
// File B
this.listenTo(anyModel, "change:anAttribute", function(model, newVal) {
model.set("anotherAttribute", 42); // Passing no options -> Accidentally overwrites hash for change event
});
// File C:
anyModel.set("anAttribute", 23, {fooFlag: true});
So what you want to do in file B, is just setting another attribute. But if you do not regard the listener in file A (as in this example), you break it. Therefor you must check all other listeners: Are they using options? Or you always have to pass around the options hash by inner changes.
But even then you can only add (or overwrite) flags to the final options hash of the "change" event. You cannot use options for a change:attribute event only. As an example you may think of a kind of "disabling flag":
// File A
this.listenTo(anyModel, "change", function(model, options) {
options = options || {}; // Always contains disableSomething flag
if(!options.disableSomething) { // Never true, but expected to be true for default options
doSomething();
}
});
// File B
this.listenTo(anyModel, "change:anAttribute", function(model, newVal) {
model.set("anotherAttribute", 42, {disableSomething: true}); // We don't want to call doSomething() a second time (and cannot use silent flag)
});
// File C:
anyModel.set("anAttribute", 23);
A kind of proposal
In my opinion the current behavior is not obvious - the default case should be the first/old one of version 1.1.0. If you intentionally (for whatever reason) want to have the options hash of an inner change to the final "change" event, then there are two possibilities:
- A kind of new flag for the set method to force using inner hash options
- Using the reference nature of the options hash: If you set additional flags to the options hash from inside a change callback, they are simultaneously set to the original hash (because it's passed in by reference):
// File A
this.listenTo(anyModel, "change", function(model, options) {
options = options || {}; // Hash contains fooFlag and myOwnOption
if(options.fooFlag) {
doSomething();
}
});
// File B
this.listenTo(anyModel, "change:anAttribute", function(model, newVal, options) {
options = options || {}; // Hash contains fooFlag
options.myOwnOption = 123;
model.set("anotherAttribute", 42, options);
});
// File C:
anyModel.set("anAttribute", 23, {fooFlag: true});
Side note
Finally I really want to stress out to explicitly mention such changes in the changelog ;-) If you look into the changelog of version 1.1.1: There is only the point "Performance fine-tuning for Backbone Events." This can be anything, but surely not a change causing different behavior on backbone models! In our team we did a search over hours to find the problem caused by this change - till we found out that backbone is doing something different here.