From 62995221745cd9fc78804b3d20496f10bca450a6 Mon Sep 17 00:00:00 2001 From: unknown Date: Mon, 13 Jul 2015 13:15:29 +0100 Subject: [PATCH] Improved merging of arrays to handle primitive values and added some backing unit tests to verify this. --- src/knockout.merge.js | 41 ++++++++++- tests/array-merge-spec.js | 139 ++++++++++++++++++++++++++++++++++++++ tests/test-runner.html | 1 + 3 files changed, 178 insertions(+), 3 deletions(-) create mode 100644 tests/array-merge-spec.js diff --git a/src/knockout.merge.js b/src/knockout.merge.js index d359a2a..c3b6b4b 100644 --- a/src/knockout.merge.js +++ b/src/knockout.merge.js @@ -26,13 +26,25 @@ mergeMethod(knockoutElement, dataElement); } } else if(isObservableArray(knockoutElement) && isArray(dataElement)) { + // 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); + if(isPrimitive(dataElement[i])) { + knockoutElement.push(dataElement[i]); + } else { + var placeholder = {}; + exports.fromJS(placeholder, dataElement[i]); + knockoutElement.push(placeholder); + } + } else if (isPrimitive(knockoutElement()[i]) && isPrimitive(dataElement[i])) { + // Handle primitive array merging by simply splicing the value into the array + knockoutElement.splice(i, 1, dataElement[i]); } else { exports.fromJS(knockoutElement()[i], dataElement[i]); } @@ -65,6 +77,29 @@ return Object.prototype.toString.call(dataElement) === '[object Array]'; }; + // Determine if the given item is a primitive type or not + var isPrimitive = function (element) { + + // Technically these are primitives and we may want to overwrite with these values + if (element === null) return true; + if (element === undefined) return true; + + // Date is a bit special, in that typeof reports an object + if (element instanceof Date) return true; + + // Handle the regular primitives + switch (typeof element) { + case "string": + case "number": + case "boolean": + case "symbol": + return true; + + default: + return false; + } + }; + var getMethodForMergeRule = function(mergeRule) { for(var property in exports.rules) { diff --git a/tests/array-merge-spec.js b/tests/array-merge-spec.js new file mode 100644 index 0000000..844440f --- /dev/null +++ b/tests/array-merge-spec.js @@ -0,0 +1,139 @@ +describe("Array Merge", function() { + + it("should correctly merge complex objects", function() { + var vm = { values: ko.observableArray() }; + vm.values.push({ id: 1, value: 1 }); + vm.values.push({ id: 2, value: 2 }); + vm.values.push({ id: 3, value: 3 }); + + var data = { + values: [{ id: 1, value: 4 }, + { id: 2, value: 5 }, + { id: 3, value: 6 }] + }; + ko.merge.fromJS(vm, data); + + expect(vm.values().length).toBe(3); + expect(vm.values()[0].value).toBe(4); + expect(vm.values()[1].value).toBe(5); + expect(vm.values()[2].value).toBe(6); + }); + + it("should correctly merge complex observable objects", function() { + var vm = { values: ko.observableArray() }; + vm.values.push({ id: ko.observable(1), value: ko.observable(1) }); + vm.values.push({ id: ko.observable(2), value: ko.observable(2) }); + vm.values.push({ id: ko.observable(3), value: ko.observable(3) }); + + var data = { + values: [{ id: 1, value: 4 }, + { id: 2, value: 5 }, + { id: 3, value: 6 }] + }; + ko.merge.fromJS(vm, data); + + // Ensure objects are still observable + expect(ko.isObservable(vm.values()[0].id)).toBe(true); + expect(ko.isObservable(vm.values()[0].value)).toBe(true); + expect(ko.isObservable(vm.values()[1].id)).toBe(true); + expect(ko.isObservable(vm.values()[1].value)).toBe(true); + expect(ko.isObservable(vm.values()[2].id)).toBe(true); + expect(ko.isObservable(vm.values()[2].value)).toBe(true); + + expect(vm.values().length).toBe(3); + expect(vm.values()[0].value()).toBe(4); + expect(vm.values()[1].value()).toBe(5); + expect(vm.values()[2].value()).toBe(6); + }); + + it("should correctly merge numbers", function() { + var vm = { values: ko.observableArray([1, 2, 3]) }; + var data = { values: [4, 5, 6] }; + ko.merge.fromJS(vm, data); + + expect(vm.values().length).toBe(3); + expect(vm.values()[0]).toBe(4); + expect(vm.values()[1]).toBe(5); + expect(vm.values()[2]).toBe(6); + }); + + it("should correctly merge strings", function() { + var vm = { values: ko.observableArray(["a", "b", "c"]) }; + var data = { values: ["A", "B", "C"] }; + ko.merge.fromJS(vm, data); + + expect(vm.values().length).toBe(3); + expect(vm.values()[0]).toBe("A"); + expect(vm.values()[1]).toBe("B"); + expect(vm.values()[2]).toBe("C"); + }); + + it("should correctly merge booleans", function() { + var vm = { values: ko.observableArray([true, false, true]) }; + var data = { values: [false, true, false] }; + ko.merge.fromJS(vm, data); + + expect(vm.values().length).toBe(3); + expect(vm.values()[0]).toBe(false); + expect(vm.values()[1]).toBe(true); + expect(vm.values()[2]).toBe(false); + }); + + it("should correctly merge dates", function() { + var vm = { values: ko.observableArray([new Date(2013,11, 25), new Date(2014,11, 25), new Date(2015,11, 25)]) }; + var data = { values: [new Date(2013,11, 24), new Date(2014,11, 24), new Date(2015,11, 24)]}; + ko.merge.fromJS(vm, data); + + expect(vm.values().length).toBe(3); + expect(vm.values()[0].getTime()).toBe(new Date(2013,11, 24).getTime()); + expect(vm.values()[1].getTime()).toBe(new Date(2014,11, 24).getTime()); + expect(vm.values()[2].getTime()).toBe(new Date(2015,11, 24).getTime()); + }); + + it("should correctly merge nulls", function() { + var vm = { values: ko.observableArray(["a", false, new Date()]) }; + var data = { values: [null, true, null] }; + ko.merge.fromJS(vm, data); + + expect(vm.values().length).toBe(3); + expect(vm.values()[0]).toBe(null); + expect(vm.values()[1]).toBe(true); + expect(vm.values()[2]).toBe(null); + }); + + it("should correctly merge undefined", function() { + var vm = { values: ko.observableArray(["a", false, new Date()]) }; + var data = { values: [undefined, true, undefined] }; + ko.merge.fromJS(vm, data); + + expect(vm.values().length).toBe(3); + expect(vm.values()[0]).toBe(undefined); + expect(vm.values()[1]).toBe(true); + expect(vm.values()[2]).toBe(undefined); + }); + + it("should correctly leave any extra items", function() { + var vm = { values: ko.observableArray(["a", "b", "c", "d"]) }; + var data = { values: ["A", "B", "C"] }; + ko.merge.fromJS(vm, data); + + expect(vm.values().length).toBe(4); + expect(vm.values()[0]).toBe("A"); + expect(vm.values()[1]).toBe("B"); + expect(vm.values()[2]).toBe("C"); + expect(vm.values()[3]).toBe("d"); + }); + + it("should correctly add new items", function() { + var vm = { values: ko.observableArray(["a", "b", "c"]) }; + var data = { values: ["A", "B", "C", "D"] }; + ko.merge.fromJS(vm, data); + + expect(vm.values().length).toBe(4); + expect(vm.values()[0]).toBe("A"); + expect(vm.values()[1]).toBe("B"); + expect(vm.values()[2]).toBe("C"); + expect(vm.values()[3]).toBe("D"); + }); + +}); \ No newline at end of file diff --git a/tests/test-runner.html b/tests/test-runner.html index 5621124..6fb812e 100644 --- a/tests/test-runner.html +++ b/tests/test-runner.html @@ -14,5 +14,6 @@ + \ No newline at end of file