Merge pull request #1795 from Polymer/1744-kschaaf

Do not apply/notify keySplices if array has not been Collectionified. Fixes #1744
Polymer · Jun 11, 2015 · f76ff64 · f76ff64
2 parents 6b43314 + 55524b9
commit f76ff64
Show file tree

Hide file tree

Showing 5 changed files with 48 additions and 9 deletions.
diff --git a/src/lib/collection.html b/src/lib/collection.html
@@ -96,7 +96,7 @@
       return items;
     },
 
-    applySplices: function(splices) {
+    _applySplices: function(splices) {
       var keySplices = [];
       for (var i=0; i<splices.length; i++) {
         var j, o, key, s = splices[i];
@@ -128,8 +128,17 @@
   };
 
   Polymer.Collection.get = function(userArray) {
-    return Polymer._collections.get(userArray)
-      || new Polymer.Collection(userArray);
+    return Polymer._collections.get(userArray) ||
+      new Polymer.Collection(userArray);
+  };
+
+  Polymer.Collection.applySplices = function(userArray, splices) {
+    // Only apply splices & generate keySplices if the array already has a
+    // backing Collection, meaning there is an element monitoring its keys;
+    // Splices that happen before the collection has been created must be
+    // discarded to avoid double-entries
+    var coll = Polymer._collections.get(userArray);
+    return coll ? coll._applySplices(splices) : null;
   };
 
 </script>
diff --git a/src/lib/experimental/observe-js-behavior.html b/src/lib/experimental/observe-js-behavior.html
@@ -76,7 +76,7 @@
       observer = new ArrayObserver(object);
       observer.open(function(splices) {
         this._notifyObserveJsPath(els, 'splices', {
-          keySplices: Polymer.Collection.get(object).applySplices(splices),
+          keySplices: Polymer.Collection.applySplices(object, splices),
           indexSplices: splices
         });
       }, this);

diff --git a/src/standard/notify-path.html b/src/standard/notify-path.html
@@ -332,7 +332,7 @@
           type: 'splice'
         }];
         var change = {
-          keySplices: Polymer.Collection.get(array).applySplices(splices),
+          keySplices: Polymer.Collection.applySplices(array, splices),
           indexSplices: splices
         };
         this.set(path + '.splices', change);

diff --git a/test/unit/notify-path-elements.html b/test/unit/notify-path-elements.html
@@ -119,6 +119,7 @@
       'multipleChanged(a, b, nested.obj.*)',
       'multiplePathsChanged(a, nested.b, nested.obj.c)',
       'arrayChanged(array.splices)',
+      'arrayNoCollChanged(arrayNoColl.splices)',
       'arrayOrPropChanged(prop, array.splices)'
     ],
     created: function() {
@@ -130,6 +131,7 @@
         multipleChanged: 0,
         multiplePathsChanged: 0,
         arrayChanged: 0,
+        arrayNoCollChanged: 0,
         arrayOrPropChanged: 0
       };
     },
@@ -191,13 +193,22 @@
       this.observerCounts.arrayChanged++;
       if (this.array.length) {
         assert.equal(splices.indexSplices.length, 1,
-          'arrayOrPropChanged indexSplices incorrect');
+          'arrayChanged indexSplices incorrect');
         assert.equal(splices.indexSplices[0].addedCount, 3,
-          'arrayOrPropChanged indexSplices incorrect');
+          'arrayChanged indexSplices incorrect');
         assert.equal(splices.keySplices.length, 1,
-          'arrayOrPropChanged keySplices incorrect');
+          'arrayChanged keySplices incorrect');
         assert.equal(splices.keySplices[0].added.length, 3,
-          'arrayOrPropChanged indexSplices incorrect');
+          'arrayChanged keySplices incorrect');
+      }
+    },
+    arrayNoCollChanged: function(splices) {
+      this.observerCounts.arrayNoCollChanged++;
+      if (this.arrayNoColl.length) {
+        assert.equal(splices.indexSplices.length, 1,
+          'arrayNoCollChanged indexSplices incorrect');
+        assert.equal(splices.indexSplices[0].addedCount, 3,
+          'arrayNoCollChanged indexSplices incorrect');
       }
     },
     arrayOrPropChanged: function(prop, splices) {

diff --git a/test/unit/notify-path.html b/test/unit/notify-path.html
@@ -722,17 +722,34 @@
     assert.equal(el.$.boundChild.computedFromPaths, 100);
   });
 
+  test('array.splices notified (no Collection)', function() {
+    el.arrayNoColl = [];
+    assert.equal(el.observerCounts.arrayNoCollChanged, 1);
+    el.push('arrayNoColl', 1, 2, 3);
+    assert.equal(el.observerCounts.arrayNoCollChanged, 2);
+    assert.equal(Polymer.Collection.get(el.arrayNoColl).getKeys().length, 3);
+    el.push('arrayNoColl', 1, 2, 3);
+    assert.equal(el.observerCounts.arrayNoCollChanged, 3);
+    assert.equal(Polymer.Collection.get(el.arrayNoColl).getKeys().length, 6);
+  });
+
   test('array.splices notified', function() {
     el.array = [];
+    // Ensure keySplices are generated for test purposes
+    Polymer.Collection.get(el.array);
     assert.equal(el.observerCounts.arrayChanged, 1);
     el.push('array', 1, 2, 3);
     assert.equal(el.observerCounts.arrayChanged, 2);
+    assert.equal(Polymer.Collection.get(el.array).getKeys().length, 3);
     el.push('array', 1, 2, 3);
     assert.equal(el.observerCounts.arrayChanged, 3);
+    assert.equal(Polymer.Collection.get(el.array).getKeys().length, 6);
   });
 
   test('array.splices notified, multiple args, prop first', function() {
     el.array = [];
+    // Ensure keySplices are generated for test purposes
+    Polymer.Collection.get(el.array);
     el.prop = 'first';
     el.push('array', 1, 2, 3);
     assert.equal(el.observerCounts.arrayOrPropChanged, 1);
@@ -742,6 +759,8 @@
 
   test('array.splices notified, multiple args, prop last', function() {
     el.array = [];
+    // Ensure keySplices are generated for test purposes
+    Polymer.Collection.get(el.array);
     el.push('array', 1, 2, 3);
     el.prop = 'last';
     assert.equal(el.observerCounts.arrayOrPropChanged, 1);