Array merging is failing for simple types

[IPWright83]

Not 100% sure why yet but I've just discovered that the array merging isn't working properly for simple objects (e.g. an array of numbers).

Essentially given the following, the resultant viewModel.values()[0] will still equal 0.

var viewModel = { values: ko.observableArray([ 0 ]); }
var data = { values: [50] };
ko.merge.fromJS(viewModel, data);

I'm looking into this at the moment, if I find a resolution I'll submit a pull request.

[grofit]

is it appending? as I am sure there is a boolean you can pass in that tells it to replace or append.

[grofit]

Turns out I was thinking of the merge constructor:

ko.observableArray.fn.withMergeConstructor = function(mergeConstructor, replaceOnMerge) {
        this.mergeConstructor = mergeConstructor;

        if(replaceOnMerge) { 
            this.replaceOnMerge = replaceOnMerge; 
        }

        return this;
    }

Which has the boolean to replace or append on merge, so wont be applicable here.

[grofit]

On a side note given a few people use this lib now it may be worth adding some unit tests to it so at least there are some scenarios to test for all these things, if I get a moment in the week I will try to add some.

[IPWright83]

@grofit some automated tests would be good. I get a little lost as soon as people start getting complex build processes, but happy to try and add some simple Jasmine tests etc at some point. I'm basically writing a couple of small tests in a JSFiddle while I'm trying to fix.

[grofit]

Yeah I didnt want to require nodejs to test it all, and I just never bothered adding jasmine. Have done so now so will add more tests as I go, feel free to fill in the gaps.

[IPWright83]

@grofit almost got a fix for this. I'll add some tests when I create a fork. Quick question - you ever had any issues with observable arrays not updating subscribers?

I'm currently using the following:

// If we have an observable array and a data element which is an array
// then we need to merge the values item by item
for (var i = 0; i < dataElement.length; i++) {

    // We don't yet have an item in the array so we need to 
    // create one. Use a placeholder and the standard merge
    // logic to do this
    if (i >= knockoutElement().length) {
        var placeholder = {};
        exports.fromJS(placeholder, dataElement[i]);
        knockoutElement.push(placeholder);
    } else if (isPrimitive(knockoutElement()[i]) && isPrimitive(dataElement[i])) {
        knockoutElement().splice(i, 1, dataElement[i]);
    } else {
        exports.fromJS(knockoutElement()[i], dataElement[i]);
    }
}

What I'm finding is that I jump to the knockoutElement().splice(i, 1, dataElement[i]); and the array gets updated correctly, but anything that's listening to the array doesn't seem to update properly. I've had to put a knockoutElement.notifySubscribers(); to get this working properly in our codebase but I can't quite figure out why I need it.

[grofit]

Not that I can remember, what version of KO are you using?

[IPWright83]

@grofit 3.3 debug version from cdnjs.com

[IPWright83]

@grofit nevermind, had an extra () that I didn't need in there. Got a fix, will write some tests quick then issue a pull request. Thanks :)

[IPWright83]

@grofit love the speed at which I submit a pull request and it gets merged in :) Thanks!

[grofit]

❤️ Github