Allow array API's accept string & negative args. Fixes #2062.

Brings the API more in line with native splice, etc.
Polymer · Jul 24, 2015 · 7e2ceeb · 7e2ceeb
1 parent cb32751
commit 7e2ceeb
Show file tree

Hide file tree

Showing 2 changed files with 286 additions and 6 deletions.
diff --git a/src/standard/notify-path.html b/src/standard/notify-path.html
@@ -380,7 +380,9 @@
         var args = Array.prototype.slice.call(arguments, 1);
         var len = array.length;
         var ret = array.push.apply(array, args);
-        this._notifySplice(array, path, len, args.length, []);
+        if (args.length) {
+          this._notifySplice(array, path, len, args.length, []);
+        }
         return ret;
       },
 
@@ -399,10 +401,12 @@
        */
       pop: function(path) {
         var array = this.get(path);
+        var hadLength = Boolean(array.length);
         var args = Array.prototype.slice.call(arguments, 1);
-        var rem = array.slice(-1);
         var ret = array.pop.apply(array, args);
-        this._notifySplice(array, path, array.length, 0, rem);
+        if (hadLength) {
+          this._notifySplice(array, path, array.length, 0, [ret]);
+        }
         return ret;
       },
 
@@ -425,9 +429,21 @@
        */
       splice: function(path, start, deleteCount) {
         var array = this.get(path);
+        // Normalize fancy native splice handling of crazy start values
+        if (start < 0) {
+          start = array.length - Math.floor(Math.abs(start));
+        } else {
+          start = Math.floor(start);
+        }
+        if (!start || !isFinite(start)) {
+          start = 0;
+        }
         var args = Array.prototype.slice.call(arguments, 1);
         var ret = array.splice.apply(array, args);
-        this._notifySplice(array, path, start, args.length - 2, ret);
+        var addedCount = Math.max(args.length - 2, 0);
+        if (addedCount || ret.length) {
+          this._notifySplice(array, path, start, addedCount, ret);
+        }
         return ret;
       },
 
@@ -446,9 +462,12 @@
        */
       shift: function(path) {
         var array = this.get(path);
+        var hadLength = Boolean(array.length);
         var args = Array.prototype.slice.call(arguments, 1);
         var ret = array.shift.apply(array, args);
-        this._notifySplice(array, path, 0, 0, [ret]);
+        if (hadLength) {
+          this._notifySplice(array, path, 0, 0, [ret]);
+        }
         return ret;
       },
 
@@ -470,7 +489,9 @@
         var array = this.get(path);
         var args = Array.prototype.slice.call(arguments, 1);
         var ret = array.unshift.apply(array, args);
-        this._notifySplice(array, path, 0, args.length, []);
+        if (args.length) {
+          this._notifySplice(array, path, 0, args.length, []);
+        }
         return ret;
       }
 

diff --git a/test/unit/notify-path.html b/test/unit/notify-path.html
@@ -901,6 +901,265 @@
     assert.equal(el.get('array.prop'), 'foo');
   });
 
+  var nop = function() {};
+
+  test('push array', function() {
+    el.arrayChanged = nop;
+    el.array = ['orig1', 'orig2', 'orig3'];
+    Polymer.Collection.get(el.array);
+    var key = el.array.length;
+    el.arrayChanged = function(change) {
+      assert.strictEqual(change.indexSplices.length, 1);
+      assert.strictEqual(change.indexSplices[0].index, 3);
+      assert.strictEqual(change.indexSplices[0].addedCount, 2);
+      assert.strictEqual(change.indexSplices[0].removed.length, 0);
+      assert.strictEqual(change.keySplices.length, 1);
+      assert.strictEqual(change.keySplices[0].index, 3);
+      assert.strictEqual(change.keySplices[0].added.length, 2);
+      assert.strictEqual(change.keySplices[0].added[0], key);
+      assert.strictEqual(change.keySplices[0].added[1], key+1);
+      assert.strictEqual(change.keySplices[0].removed.length, 0);
+    };
+    var ret = el.push('array', 'new1', 'new2');
+    assert.strictEqual(ret, 5);
+    assert.strictEqual(el.array.length, 5);
+    assert.strictEqual(el.array[0], 'orig1');
+    assert.strictEqual(el.array[1], 'orig2');
+    assert.strictEqual(el.array[2], 'orig3');
+    assert.strictEqual(el.array[3], 'new1');
+    assert.strictEqual(el.array[4], 'new2');
+  });
+
+  test('pop array', function() {
+    el.arrayChanged = nop;
+    el.array = ['orig1', 'orig2', 'orig3'];
+    Polymer.Collection.get(el.array);
+    var key = el.array.length-1;
+    el.arrayChanged = function(change) {
+      assert.strictEqual(change.indexSplices.length, 1);
+      assert.strictEqual(change.indexSplices[0].index, 2);
+      assert.strictEqual(change.indexSplices[0].addedCount, 0);
+      assert.strictEqual(change.indexSplices[0].removed.length, 1);
+      assert.strictEqual(change.indexSplices[0].removed[0], 'orig3');
+      assert.strictEqual(change.keySplices.length, 1);
+      assert.strictEqual(change.keySplices[0].index, 2);
+      assert.strictEqual(change.keySplices[0].added.length, 0);
+      assert.strictEqual(change.keySplices[0].removed.length, 1);
+      assert.strictEqual(change.keySplices[0].removed[0], key);
+    };
+    var ret = el.pop('array');
+    assert.strictEqual(ret, 'orig3');
+    assert.strictEqual(el.array.length, 2);
+    assert.strictEqual(el.array[0], 'orig1');
+    assert.strictEqual(el.array[1], 'orig2');
+  });
+
+  test('unshift array', function() {
+    el.arrayChanged = nop;
+    el.array = ['orig1', 'orig2', 'orig3'];
+    Polymer.Collection.get(el.array);
+    var key = el.array.length;
+    el.arrayChanged = function(change) {
+      assert.strictEqual(change.indexSplices.length, 1);
+      assert.strictEqual(change.indexSplices[0].index, 0);
+      assert.strictEqual(change.indexSplices[0].addedCount, 2);
+      assert.strictEqual(change.indexSplices[0].removed.length, 0);
+      assert.strictEqual(change.keySplices.length, 1);
+      assert.strictEqual(change.keySplices[0].index, 0);
+      assert.strictEqual(change.keySplices[0].added.length, 2);
+      assert.strictEqual(change.keySplices[0].added[0], key);
+      assert.strictEqual(change.keySplices[0].added[1], key+1);
+      assert.strictEqual(change.keySplices[0].removed.length, 0);
+    };
+    var ret = el.unshift('array', 'new1', 'new2');
+    assert.strictEqual(ret, 5);
+    assert.strictEqual(el.array.length, 5);
+    assert.strictEqual(el.array[0], 'new1');
+    assert.strictEqual(el.array[1], 'new2');
+    assert.strictEqual(el.array[2], 'orig1');
+    assert.strictEqual(el.array[3], 'orig2');
+    assert.strictEqual(el.array[4], 'orig3');
+  });
+
+  test('shift array', function() {
+    el.arrayChanged = nop;
+    el.array = ['orig1', 'orig2', 'orig3'];
+    Polymer.Collection.get(el.array);
+    el.arrayChanged = function(change) {
+      assert.strictEqual(change.indexSplices.length, 1);
+      assert.strictEqual(change.indexSplices[0].index, 0);
+      assert.strictEqual(change.indexSplices[0].addedCount, 0);
+      assert.strictEqual(change.indexSplices[0].removed.length, 1);
+      assert.strictEqual(change.indexSplices[0].removed[0], 'orig1');
+      assert.strictEqual(change.keySplices[0].index, 0);
+      assert.strictEqual(change.keySplices.length, 1);
+      assert.strictEqual(change.keySplices[0].added.length, 0);
+      assert.strictEqual(change.keySplices[0].removed.length, 1);
+      assert.strictEqual(change.keySplices[0].removed[0], 0);
+    };
+    var ret = el.shift('array');
+    assert.strictEqual(ret, 'orig1');
+    assert.strictEqual(el.array.length, 2);
+    assert.strictEqual(el.array[0], 'orig2');
+    assert.strictEqual(el.array[1], 'orig3');
+  });
+
+  test('splice array', function() {
+    el.arrayChanged = nop;
+    el.array = ['orig1', 'orig2', 'orig3'];
+    var key = el.array.length;
+    Polymer.Collection.get(el.array);
+    el.arrayChanged = function(change) {
+      assert.strictEqual(change.indexSplices.length, 1);
+      assert.strictEqual(change.indexSplices[0].index, 1);
+      assert.strictEqual(change.indexSplices[0].addedCount, 2);
+      assert.strictEqual(change.indexSplices[0].removed.length, 1);
+      assert.strictEqual(change.indexSplices[0].removed[0], 'orig2');
+      assert.strictEqual(change.keySplices.length, 1);
+      assert.strictEqual(change.keySplices[0].index, 1);
+      assert.strictEqual(change.keySplices[0].added.length, 2);
+      assert.strictEqual(change.keySplices[0].added[0], key);
+      assert.strictEqual(change.keySplices[0].added[1], key+1);
+      assert.strictEqual(change.keySplices[0].removed.length, 1);
+      assert.strictEqual(change.keySplices[0].removed[0], 1);
+    };
+    var ret = el.splice('array', 1, 1, 'new1', 'new2');
+    assert.deepEqual(ret, ['orig2']);
+    assert.strictEqual(el.array.length, 4);
+    assert.strictEqual(el.array[0], 'orig1');
+    assert.strictEqual(el.array[1], 'new1');
+    assert.strictEqual(el.array[2], 'new2');
+    assert.strictEqual(el.array[3], 'orig3');
+  });
+
+  test('corner: no-op push array', function() {
+    el.arrayChanged = nop;
+    el.array = ['orig1', 'orig2', 'orig3'];
+    Polymer.Collection.get(el.array);
+    el.arrayChanged = function(change) {
+      throw new Error("should not notify");
+    };
+    var ret = el.push('array');
+    assert.deepEqual(ret, 3);
+    assert.strictEqual(el.array.length, 3);
+    assert.strictEqual(el.array[0], 'orig1');
+    assert.strictEqual(el.array[1], 'orig2');
+    assert.strictEqual(el.array[2], 'orig3');
+  });
+
+  test('corner: no-op pop array', function() {
+    el.arrayChanged = nop;
+    el.array = [];
+    Polymer.Collection.get(el.array);
+    el.arrayChanged = function(change) {
+      throw new Error("should not notify");
+    };
+    var ret = el.pop('array');
+    assert.strictEqual(ret, undefined);
+    assert.strictEqual(el.array.length, 0);
+  });
+
+  test('corner: no-op unshift array', function() {
+    el.arrayChanged = nop;
+    el.array = ['orig1', 'orig2', 'orig3'];
+    Polymer.Collection.get(el.array);
+    var key = el.array.length;
+    el.arrayChanged = function(change) {
+      throw new Error("should not notify");
+    };
+    var ret = el.unshift('array');
+    assert.deepEqual(ret, 3);
+    assert.strictEqual(el.array.length, 3);
+    assert.strictEqual(el.array[0], 'orig1');
+    assert.strictEqual(el.array[1], 'orig2');
+    assert.strictEqual(el.array[2], 'orig3');
+  });
+
+  test('corner: no-op shift array', function() {
+    el.arrayChanged = nop;
+    el.array = [];
+    Polymer.Collection.get(el.array);
+    el.arrayChanged = function(change) {
+      throw new Error("should not notify");
+    };
+    var ret = el.shift('array');
+    assert.strictEqual(ret, undefined);
+    assert.strictEqual(ret, undefined);
+    assert.strictEqual(el.array.length, 0);
+  });
+
+  test('corner: no-op splice array', function() {
+    el.arrayChanged = nop;
+    el.array = ['orig1', 'orig2', 'orig3'];
+    var key = el.array.length;
+    Polymer.Collection.get(el.array);
+    el.arrayChanged = function(change) {
+      throw new Error("should not notify");
+    };
+    var ret = el.splice('array');
+    assert.deepEqual(ret, []);
+    assert.strictEqual(el.array.length, 3);
+    assert.strictEqual(el.array[0], 'orig1');
+    assert.strictEqual(el.array[1], 'orig2');
+    assert.strictEqual(el.array[2], 'orig3');
+  });
+
+  test('corner: splice array: string args', function() {
+    el.arrayChanged = nop;
+    el.array = ['orig1', 'orig2', 'orig3'];
+    var key = el.array.length;
+    Polymer.Collection.get(el.array);
+    el.arrayChanged = function(change) {
+      assert.strictEqual(change.indexSplices.length, 1);
+      assert.strictEqual(change.indexSplices[0].index, 1);
+      assert.strictEqual(change.indexSplices[0].addedCount, 2);
+      assert.strictEqual(change.indexSplices[0].removed.length, 1);
+      assert.strictEqual(change.indexSplices[0].removed[0], 'orig2');
+      assert.strictEqual(change.keySplices.length, 1);
+      assert.strictEqual(change.keySplices[0].index, 1);
+      assert.strictEqual(change.keySplices[0].added.length, 2);
+      assert.strictEqual(change.keySplices[0].added[0], key);
+      assert.strictEqual(change.keySplices[0].added[1], key+1);
+      assert.strictEqual(change.keySplices[0].removed.length, 1);
+      assert.strictEqual(change.keySplices[0].removed[0], 1);
+    };
+    var ret = el.splice('array', '1', '1', 'new1', 'new2');
+    assert.deepEqual(ret, ['orig2']);
+    assert.strictEqual(el.array.length, 4);
+    assert.strictEqual(el.array[0], 'orig1');
+    assert.strictEqual(el.array[1], 'new1');
+    assert.strictEqual(el.array[2], 'new2');
+    assert.strictEqual(el.array[3], 'orig3');
+  });
+
+  test('corner: splice array: negative start', function() {
+    el.arrayChanged = nop;
+    el.array = ['orig1', 'orig2', 'orig3'];
+    var key = el.array.length;
+    Polymer.Collection.get(el.array);
+    el.arrayChanged = function(change) {
+      assert.strictEqual(change.indexSplices.length, 1);
+      assert.strictEqual(change.indexSplices[0].index, 1);
+      assert.strictEqual(change.indexSplices[0].addedCount, 2);
+      assert.strictEqual(change.indexSplices[0].removed.length, 1);
+      assert.strictEqual(change.indexSplices[0].removed[0], 'orig2');
+      assert.strictEqual(change.keySplices.length, 1);
+      assert.strictEqual(change.keySplices[0].index, 1);
+      assert.strictEqual(change.keySplices[0].added.length, 2);
+      assert.strictEqual(change.keySplices[0].added[0], key);
+      assert.strictEqual(change.keySplices[0].added[1], key+1);
+      assert.strictEqual(change.keySplices[0].removed.length, 1);
+      assert.strictEqual(change.keySplices[0].removed[0], 1);
+    };
+    var ret = el.splice('array', '-2', '1', 'new1', 'new2');
+    assert.deepEqual(ret, ['orig2']);
+    assert.strictEqual(el.array.length, 4);
+    assert.strictEqual(el.array[0], 'orig1');
+    assert.strictEqual(el.array[1], 'new1');
+    assert.strictEqual(el.array[2], 'new2');
+    assert.strictEqual(el.array[3], 'orig3');
+  });
+
 });
 
 </script>