From a51de9e3984860b8a7aebcfabf452bad186910dc Mon Sep 17 00:00:00 2001 From: Tim van der Lippe Date: Thu, 18 Jan 2018 15:11:34 +0100 Subject: [PATCH] Fix issue where el.splice could not clear full array --- lib/mixins/property-effects.html | 28 ++++++++++++++++++++++++---- test/unit/path-effects.html | 7 +++++++ 2 files changed, 31 insertions(+), 4 deletions(-) diff --git a/lib/mixins/property-effects.html b/lib/mixins/property-effects.html index 3e7cb521c1..a238379414 100644 --- a/lib/mixins/property-effects.html +++ b/lib/mixins/property-effects.html @@ -1933,13 +1933,33 @@ // Normalize fancy native splice handling of crazy start values if (start < 0) { start = array.length - Math.floor(-start); - } else { + } else if (start) { start = Math.floor(start); } - if (!start) { - start = 0; + // array.splice does different things based on the number of arguments + // you pass in. Therefore, array.splice(0) and array.splice(0, undefined) + // do different things. In the former, the whole array is cleared. In the + // latter, no items are removed. + // This means that we need to detect whether 1. one of the arguments + // is actually passed in and then 2. determine how many arguments + // we should pass on to the native array.splice + // + let ret; + // Omit any additional arguments if they were not passed in + if (start !== undefined && !deleteCount && !items.length) { + ret = array.splice(start); + // Either start was undefined and the others were defined, but in this + // case we can safely pass on all arguments + // + // Note: this includes the case where none of the arguments were passed in, + // e.g. this.splice('array'). However, if both start and deleteCount + // are undefined, array.splice will not modify the array (as expected) + } else { + ret = array.splice(start, deleteCount, ...items); } - let ret = array.splice(start, deleteCount, ...items); + // At the end, check whether any items were passed in (e.g. insertions) + // or if the return array contains items (e.g. deletions). + // Only notify if items were added or deleted. if (items.length || ret.length) { notifySplice(this, array, info.path, start, items.length, ret); } diff --git a/test/unit/path-effects.html b/test/unit/path-effects.html index 4ba3b96384..8659c87890 100644 --- a/test/unit/path-effects.html +++ b/test/unit/path-effects.html @@ -498,6 +498,13 @@ assert.equal(el.$.boundChild.arrayLength, el.data.length); }); + test('splice correctly deletes length', function() { + el.data = [1,2,3]; + assert.equal(el.data.length, 3); + el.splice('data', 0); + assert.equal(el.data.length, 0); + }); + }); suite('path API', function() {