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 1/3] 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() { From eee609d4b296451871a4fdbd424dfb049057d5f1 Mon Sep 17 00:00:00 2001 From: Tim van der Lippe Date: Thu, 18 Jan 2018 15:29:13 +0100 Subject: [PATCH 2/3] [ci skip] Fix test case name --- test/unit/path-effects.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/unit/path-effects.html b/test/unit/path-effects.html index 8659c87890..ba516d170a 100644 --- a/test/unit/path-effects.html +++ b/test/unit/path-effects.html @@ -498,7 +498,7 @@ assert.equal(el.$.boundChild.arrayLength, el.data.length); }); - test('splice correctly deletes length', function() { + test('splice correctly deletes full array if only 1 argument is passed in', function() { el.data = [1,2,3]; assert.equal(el.data.length, 3); el.splice('data', 0); From 2775010978a873d52edba7c823b47b082a61928e Mon Sep 17 00:00:00 2001 From: Tim van der Lippe Date: Tue, 23 Jan 2018 19:44:39 +0100 Subject: [PATCH 3/3] Change if-condition to check for arguments.length --- lib/mixins/property-effects.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/mixins/property-effects.html b/lib/mixins/property-effects.html index a238379414..2afd52319e 100644 --- a/lib/mixins/property-effects.html +++ b/lib/mixins/property-effects.html @@ -1946,7 +1946,7 @@ // let ret; // Omit any additional arguments if they were not passed in - if (start !== undefined && !deleteCount && !items.length) { + if (arguments.length === 2) { ret = array.splice(start); // Either start was undefined and the others were defined, but in this // case we can safely pass on all arguments