Looks like we can't reproduce the issue or there's a problem in the test case provided.
  1. #1
    Sencha Premium Member
    Join Date
    Nov 2010
    Posts
    31
    Vote Rating
    4
    Alexey.Solonets is on a distinguished road

      0  

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

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


    This will work as expected:
    Code:
    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:
    Code:
    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]
    Code:
    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

  2. #2
    Sencha User kevin.chen's Avatar
    Join Date
    Sep 2012
    Location
    Redwood City, CA
    Posts
    242
    Vote Rating
    6
    kevin.chen is on a distinguished road

      0  

    Default


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

  3. #3
    Sencha Premium Member
    Join Date
    Nov 2010
    Posts
    31
    Vote Rating
    4
    Alexey.Solonets is on a distinguished road

      0  

    Default


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

  4. #4
    Sencha - Ext JS Dev Team dongryphon's Avatar
    Join Date
    Jul 2009
    Posts
    1,337
    Vote Rating
    130
    dongryphon is a name known to all dongryphon is a name known to all dongryphon is a name known to all dongryphon is a name known to all dongryphon is a name known to all dongryphon is a name known to all

      0  

    Default


    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.
    Don Griffin
    Ext JS Development Team Lead

    Check the docs. Learn how to (properly) report a framework issue and a Sencha Cmd issue

    "Use the source, Luke!"

  5. #5
    Sencha User
    Join Date
    Aug 2011
    Posts
    5
    Vote Rating
    0
    ATGardner is on a distinguished road

      0  

    Default Is there a working workaround?

    Is there a working workaround?


    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).

    Code:
    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.
    Last edited by ATGardner; 27 Nov 2012 at 1:48 AM. Reason: fixed error in code

  6. #6
    Sencha Premium Member
    Join Date
    Nov 2010
    Posts
    31
    Vote Rating
    4
    Alexey.Solonets is on a distinguished road

      0  

    Default


    Quote Originally Posted by ATGardner View Post
    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).

  7. #7
    Sencha User
    Join Date
    Aug 2011
    Posts
    5
    Vote Rating
    0
    ATGardner is on a distinguished road

      0  

    Default Sorry, I just noticed it was fixed in Ext JS

    Sorry, I just noticed it was fixed in Ext JS


    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?

  8. #8
    Sencha Premium Member
    Join Date
    Nov 2010
    Posts
    31
    Vote Rating
    4
    Alexey.Solonets is on a distinguished road

      0  

    Default


    Quote Originally Posted by ATGardner View Post
    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.

  9. #9
    Sencha User
    Join Date
    Aug 2011
    Posts
    5
    Vote Rating
    0
    ATGardner is on a distinguished road

      0  

    Default I tried over-riding the method in my code

    I tried over-riding the method in my code


    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.

  10. #10
    Sencha Premium Member
    Join Date
    Nov 2010
    Posts
    31
    Vote Rating
    4
    Alexey.Solonets is on a distinguished road

      0  

    Default


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

Turkiyenin en sevilen filmlerinin yer aldigi xnxx internet sitemiz olan ve porn sex tarzi bir site olan mobil porno izle sitemiz gercekten dillere destan bir durumda herkesin sevdigi bir site olarak tarihe gececege benziyor. Sitenin en belirgin ozelliklerinden birisi de Turkiyede gercekten kaliteli ve muntazam, duzenli porno izle siteleri olmamasidir. Bu yuzden iste. Ayrica en net goruntu kalitesine sahip adresinde yayinlanmaktadir. Mesela diğer sitelerimizden bahsedecek olursak, en iyi hd porno video arşivine sahip bir siteyiz. "The Best anal porn videos and slut anus, big asses movies set..." hd porno faketaxi