Ext.Object.merge doesn't merges objects in arrays

[Alexey.Solonets]

This will work as expected:

extjs = [{
    id: 1,
    name: 'Ext JS'
}, {
    id: 2,
    name: 'Ext GWT'
}, {
    id: 3,
    name: 'Ext Designer'
}]

extjs_descr = [{
    description: 'Javascript Framework'
}, {
    description: 'Google Web Toolkit Framework'
}, {
    description: 'Visual Designer'
}]

Ext.Object.merge(extjs, extjs_descr);

// got:

// {
//    id: 1,
//    name: "Ext JS",
//    description: "Javascript Framework"
// }, {
//    id: 2,
//    name: "Ext GWT",
//    description: "Google Web Toolkit Framework"
// }, {
//    id: 3,
//    name: "Ext Designer",
//    description: "Visual Designer"
// }

Excellent. But in this case it will work different:

extjs = {
    products: [{
        id: 1,
        name: 'Ext JS'
    }, {
        id: 2,
        name: 'Ext GWT'
    }, {
        id: 3,
        name: 'Ext Designer'
    }]
}

extjs_descr = {
    products: [{
        description: 'Javascript Framework'
    }, {
        description: 'Google Web Toolkit Framework'
    }, {
        description: 'Visual Designer'
    }]
}

Ext.Object.merge(extjs, extjs_descr);

// expected: 

// {
//     products: [{
//         id: 1,
//         name: 'Ext JS',
//         description: 'Javascript Framework'
//     }, {
//         id: 2,
//         name: 'Ext GWT',
//         description: 'Google Web Toolkit Framework'
//     }, {
//         id: 3,
//         name: 'Ext Designer',
//         description: 'Visual Designer'
//     }]
// }

// got:

// {
//     products: [{
//         description: 'Javascript Framework'
//     }, {
//         description: 'Google Web Toolkit Framework'
//     }, {
//         description: 'Visual Designer'
//     }]
// }

Why?
According to tests in [src/core/test/unit/spec/lang/Object.js]

describe("merge", ...
...
it("should overwrite simple values" ...

but Array is not a simple value.
If you need I can provide nice example when second expected behavior is useful.


Possible fix. [src/core/src/lang/Object.js]

merge: function (destination) {
    var i = 1,
        ln = arguments.length,
        mergeFn = Ext.Object.merge,
        cloneFn = Ext.clone,
        object, key, value, sourceKey;


    for (; i < ln; i++) {
        object = arguments[i];


        for (key in object) {
            value = object[key];
            if (value && (value.constructor === Object || value.constructor == Array)) {
                sourceKey = destination[key];
                if (sourceKey && (sourceKey.constructor === Object || sourceKey.constructor == Array)) {
                    mergeFn(sourceKey, value);
                } else {
                    destination[key] = cloneFn(value);
                }
            } else {
                destination[key] = value;
            }
        }
    }


    return destination;
}

// ... same for mergeIf

[kevin.chen]

Thanks for the report! I have opened a bug in our bug tracker.

[Alexey.Solonets]

Fixed so fast? Wow!..
I'm happy

[dongryphon]

The fix is in review.. sorry for the status hiccup there. We want to make sure behaviors at this level are coordinated with Sencha Touch is all.

[ATGardner]

The workaround posted earlier does not really work well. Calling the mergeFn on an two arrays will not merge them properly. It should use the Ext.Array.merge function instead.

I made these changes, but it doesn't handle all cases (If the source had an array field, and the object field is just an object, or the other way around).

merge: function(source) {        var i = 1,
            ln = arguments.length,
            mergeFn = ExtObject.merge,
            cloneFn = Ext.clone,
            arrayMergeFn = Ext.Array.merge,
            object, key, value, sourceKey;


        for (; i < ln; i++) {
            object = arguments[i];


            for (key in object) {
                value = object[key];
                if (value) {
                    if (value.constructor === Object) {
                        sourceKey = source[key];
                        if (sourceKey && sourceKey.constructor === Object) {
                            mergeFn(sourceKey, value);
                        }
                        else {
                            source[key] = cloneFn(value);
                        }
                    }
                    else if (value.constructor === Array) {
                        sourceKey = source[key];
                        if (sourceKey && sourceKey.constructor === Array) {
                            arrayMergeFn(sourceKey, value);
                        }
                        else {
                            source[key] = cloneFn(value);
                        }
                    } else {
                        source[key] = cloneFn(value);
                    }
                }
                else {
                    source[key] = value;
                }
            }
        }


        return source;
    },

Is there some other, better temporary fix, until the bug is fixed in release?

---
EDIT:
I just fixed an error in the code, but it still won't work properly in my code, so I guess there are more bugs in it.
I am still looking for an official (even in beta stages) fix for the problem.

[Alexey.Solonets]

The workaround posted earlier does not really work well. Calling the mergeFn on an two arrays will not merge them properly.

Do you have a test case? This workaround works well with first example (with arrays).

[ATGardner]

I am having the same problem in Sencha Touch 2.1
Are there any plans on merging the fix to the Touch release as well?

[Alexey.Solonets]

I am having the same problem in Sencha Touch 2.1
Are there any plans on merging the fix to the Touch release as well?

Not sure there is a reason to wait. You can simply override the merge method in Ext.Object in a separate file. When new version come you'll be able to easily delete it.

[ATGardner]

But the original workaround doesn't work, and my proposed solution also causes my app to throw an exception on startup (I tried debugging it and couldn't understand the reason. It wasn't thrown from the new "merge" code directly).
So I was hoping for a more robust solution from the sencha team.

[Alexey.Solonets]

Show us an example where proposed solution doesn't work please