fixes and tests for add, remove, move and update

src/observable/list/SortedMapList.js

@@ -62,19 +62,21 @@ export default class SortedMapList extends BaseObservableList {
     constructor(sourceMap, comparator) {
         super();
         this._sourceMap = sourceMap;
-        this._comparator = comparator;
+        this._comparator = (a, b) => comparator(a.value, b.value);
         this._sortedPairs = null;
         this._mapSubscription = null;
     }
     
     onAdd(key, value) {
-        const idx = sortedIndex(this._sortedPairs, value, this._comparator);
-        this._sortedPairs.splice(idx, 0, {key, value});
+        const pair = {key, value};
+        const idx = sortedIndex(this._sortedPairs, pair, this._comparator);
+        this._sortedPairs.splice(idx, 0, pair);
         this.emitAdd(idx, value);
     }
 
     onRemove(key, value) {
-        const idx = sortedIndex(this._sortedPairs, value, this._comparator);
+        const pair = {key, value};
+        const idx = sortedIndex(this._sortedPairs, pair, this._comparator);
         // assert key === this._sortedPairs[idx].key;
         this._sortedPairs.splice(idx, 1);
         this.emitRemove(idx, value);
@@ -83,11 +85,12 @@ export default class SortedMapList extends BaseObservableList {
     onUpdate(key, value, params) {
         // TODO: suboptimal for performance, see above for idea with BST to speed this up if we need to
         const oldIdx = this._sortedPairs.findIndex(p => p.key === key);
-        // neccesary to remove item from array before
+        // neccesary to remove pair from array before
         // doing sortedIndex as it relies on being sorted
         this._sortedPairs.splice(oldIdx, 1);
-        const newIdx = sortedIndex(this._sortedPairs, value, this._comparator);
-        this._sortedPairs.splice(newIdx, 0, {key, value});
+        const pair = {key, value};
+        const newIdx = sortedIndex(this._sortedPairs, pair, this._comparator);
+        this._sortedPairs.splice(newIdx, 0, pair);
         if (oldIdx !== newIdx) {
             this.emitMove(oldIdx, newIdx, value);
         }
@@ -103,8 +106,8 @@ export default class SortedMapList extends BaseObservableList {
         this._mapSubscription = this._sourceMap.subscribe(this);
         this._sortedPairs = new Array(this._sourceMap.size);
         let i = 0;
-        for (let [, value] of this._sourceMap) {
-            this._sortedPairs[i] = value;
+        for (let [key, value] of this._sourceMap) {
+            this._sortedPairs[i] = {key, value};
             ++i;
         }
         this._sortedPairs.sort(this._comparator);
@@ -118,7 +121,7 @@ export default class SortedMapList extends BaseObservableList {
     }
 
     get(index) {
-        return this._sortedPairs[index];
+        return this._sortedPairs[index].value;
     }
 
     get length() {
@@ -126,7 +129,16 @@ export default class SortedMapList extends BaseObservableList {
     }
 
     [Symbol.iterator]() {
-        return this._sortedPairs.values();
+        const it = this._sortedPairs.values();
+        return {
+            next() {
+                const v = it.next();
+                if (v.value) {
+                    v.value = v.value.value;
+                }
+                return v;
+            }
+        }
     }
 }
 
@@ -136,11 +148,15 @@ import ObservableMap from "../map/ObservableMap.js";
 export function tests() {
     return {
         test_sortIndex(assert) {
-            let idx = sortedIndex([1, 5, 6, 8], 0, (a, b) => a - b);
+            const a = [1, 5, 6, 8];
+            const cmp = (a, b) => a - b;
+            let idx = sortedIndex(a, 0, cmp);
             assert.equal(idx, 0);
-            idx = sortedIndex([1, 5, 6, 8], 3, (a, b) => a - b);
+            idx = sortedIndex(a, 3, cmp);
             assert.equal(idx, 1);
-            idx = sortedIndex([1, 5, 6, 8], 8, (a, b) => a - b);
+            idx = sortedIndex(a, 5, cmp);
+            assert.equal(idx, 1);
+            idx = sortedIndex(a, 8, cmp);
             assert.equal(idx, 3);
         },
 
@@ -167,7 +183,100 @@ export function tests() {
             const list = new SortedMapList(map, (a, b) => a - b);
             list.subscribe({}); //needed to populate iterator
             assert.deepEqual(Array.from(list), [-5, 6, 50]);
+            assert.equal(list.length, 3);
+        },
+
+        test_add(assert) {
+            const map = new ObservableMap([["a", 50], ["b", 6], ["c", -5]]);
+            const list = new SortedMapList(map, (a, b) => a - b);
+            let fired = 0;
+            list.subscribe({
+                onAdd(idx, value) {
+                    fired++;
+                    assert.equal(idx, 2);
+                    assert.equal(value, 20);
                 }
+            });
+            map.add("d", 20);
+            assert.equal(fired, 1);
+            assert.equal(list.length, 4);
+        },
+
+        test_remove(assert) {
+            const map = new ObservableMap([["a", 50], ["b", 6], ["c", -5]]);
+            const list = new SortedMapList(map, (a, b) => a - b);
+            let fired = 0;
+            list.subscribe({
+                onRemove(idx, value) {
+                    fired++;
+                    assert.equal(idx, 2);
+                    assert.equal(value, 50);
+                }
+            });
+            map.remove("a");
+            assert.equal(fired, 1);
+            assert.equal(list.length, 2);
+        },
+
+        test_move_reference(assert) {
+            const a = {number: 3};
+            const map = new ObservableMap([
+                ["a", a],
+                ["b", {number: 11}],
+                ["c", {number: 1}],
+            ]);
+            const list = new SortedMapList(map, (a, b) => a.number - b.number);
+            let updateFired = 0, moveFired = 0;
+            list.subscribe({
+                onUpdate(idx, value, param) {
+                    updateFired++;
+                    assert.equal(idx, 2);
+                    assert.equal(value, a);
+                    assert.equal(param, "number");
+                },
+
+                onMove(oldIdx, newIdx, value) {
+                    moveFired++;
+                    assert.equal(oldIdx, 1);
+                    assert.equal(newIdx, 2);
+                    assert.equal(value, a);
+                }
+            });
+            a.number = 111;
+            map.update("a", "number");
+            assert.equal(moveFired, 1);
+            assert.equal(updateFired, 1);
+        },
+
+        test_update_without_move(assert) {
+            const a = {number: 3};
+            const map = new ObservableMap([
+                ["a", a],
+                ["b", {number: 11}],
+                ["c", {number: 1}],
+            ]);
+            const list = new SortedMapList(map, (a, b) => a.number - b.number);
+            let updateFired = 0, moveFired = 0;
+            list.subscribe({
+                onUpdate(idx, value, param) {
+                    updateFired++;
+                    assert.equal(idx, 1);
+                    assert.equal(value, a);
+                    assert.equal(param, "number");
+                },
+
+                onMove() {
+                    moveFired++;
+                }
+            });
+            assert.deepEqual(Array.from(list).map(v => v.number), [1, 3, 11]);
+            // asume some part of a that doesn't affect
+            // sorting order has changed here
+            map.update("a", "number");
+            assert.equal(moveFired, 0);
+            assert.equal(updateFired, 1);
+            assert.deepEqual(Array.from(list).map(v => v.number), [1, 3, 11]);
+        },
     }
 }
 //#endif