Ext.apply. Is this a bug, would it break existing code to change it?

[Animal]

On another thread I recommended using Ext.apply in a constructor to provide mandatory config values which must override those passed in, and supply defaults when values are not included in the passed config. So I suggested this:



Ext.ux.LoginDialog = function(config) {
config = Ext.apply(config || {}, {
overrideConfig: "MyValue"
}, {
defaultConfig: "DefaultValue"
});
};


That should apply your own overrides where they must be set to the values, and provide defaults where the use does not pass a value in.

But I think that exposes a flaw in Ext.apply.



Ext.apply = function(o, c, defaults){
if(defaults){
// no "this" reference for friendly out of scope calls
Ext.apply(o, defaults);
}
if(o && c && typeof c == 'object'){
for(var p in c){
o[p] = c[p];
}
}
return o;
};


The "defaults" are applied even if the value already exists in "o". That's not really defaulting.

It should surely be



Ext.apply = function(o, c, defaults){
if(o && c && typeof c == 'object'){
for(var p in c){
o[p] = c[p];
}
}
if(defaults){
// no "this" reference for friendly out of scope calls
Ext.applyIf(o, defaults);
}
return o;
};


Jack, is that right? I don't think it would break any implementations to change it to working like that, and it would be more useful in exactly the situation described at the top.

[jack.slocum]

Defaults are for values not passed in config, not for values that already exist on the object. So setting them in a first pass is safe.

If you have a value already on the prototype, then that value IS the default and I wouldn't recommend using defaults. That will save the overhead of copying them every time as well.