Merge pull request #2215 from knockout/2215-array-sorted-remove-updates

Array Remove and Observable Cascading
knockout · Apr 8, 2017 · 6897d17 · 6897d17
2 parents 0ee72ac + 05c7aa6
commit 6897d17
Show file tree

Hide file tree

Showing 2 changed files with 46 additions and 1 deletion.
diff --git a/spec/observableArrayBehaviors.js b/spec/observableArrayBehaviors.js
@@ -193,6 +193,20 @@ describe('Observable Array', function() {
         expect(notifiedValues).toEqual([[x]]);
     });
 
+    it ('Should throw an exception if matching array item moved or removed during "remove"', function () {
+        testObservableArray(["Alpha", "Beta", "Gamma"]);
+        notifiedValues = [];
+        expect(function () {
+            testObservableArray.remove(function (value) {
+                if (value == "Beta") {
+                    testObservableArray.splice(0, 1);
+                    return true;
+                }
+            });
+        }).toThrow();
+        expect(testObservableArray()).toEqual(["Beta", "Gamma"]);
+    });
+
     it('Should notify subscribers on replace', function () {
         testObservableArray(["Alpha", "Beta", "Gamma"]);
         notifiedValues = [];
@@ -285,6 +299,26 @@ describe('Observable Array', function() {
         expect(notifiedValues).toEqual([ [3, 2, 1], [1, 2, 3] ]);
     });
 
+    it('Should return a new sorted array from "sorted"', function() {
+        // set some unsorted values so we can see that the new array is sorted
+        testObservableArray([ 5, 7, 3, 1 ]);
+        notifiedValues = [];
+
+        var newArray = testObservableArray.sorted();
+        expect(newArray).toEqual([ 1, 3, 5, 7 ]);
+        expect(newArray).not.toBe(testObservableArray());
+
+        expect(notifiedValues).toEqual([]);
+    });
+
+    it('Should return a new reversed array from "reversed"', function() {
+        var newArray = testObservableArray.reversed();
+        expect(newArray).toEqual([ 3, 2, 1 ]);
+        expect(newArray).not.toBe(testObservableArray());
+
+        expect(notifiedValues).toEqual([]);
+    });
+
     it('Should inherit any properties defined on ko.subscribable.fn, ko.observable.fn, or ko.observableArray.fn', function() {
         this.after(function() {
             delete ko.subscribable.fn.subscribableProp; // Will be able to reach this

diff --git a/src/subscribables/observableArray.js b/src/subscribables/observableArray.js
@@ -20,6 +20,9 @@ ko.observableArray['fn'] = {
                 if (removedValues.length === 0) {
                     this.valueWillMutate();
                 }
+                if (underlyingArray[i] !== value) {
+                    throw Error("Array modified during remove; cannot remove item");
+                }
                 removedValues.push(value);
                 underlyingArray.splice(i, 1);
                 i--;
@@ -56,7 +59,7 @@ ko.observableArray['fn'] = {
         for (var i = underlyingArray.length - 1; i >= 0; i--) {
             var value = underlyingArray[i];
             if (predicate(value))
-                underlyingArray[i]["_destroy"] = true;
+                value["_destroy"] = true;
         }
         this.valueHasMutated();
     },
@@ -86,6 +89,14 @@ ko.observableArray['fn'] = {
             this.peek()[index] = newItem;
             this.valueHasMutated();
         }
+    },
+
+    'sorted': function (compareFunction) {
+        return this().slice(0).sort(compareFunction);
+    },
+
+    'reversed': function () {
+        return this().slice(0).reverse();
     }
 };