Prevent inserting undefined into Collection

changelog/upgrade-notes.md

@@ -14,6 +14,10 @@ The `toStringHDMS` function from the `ol/coordinate.js` module now formats longi
 
 The default intervals now align with integer minutes and seconds better suited to the default label formatter.  If formatting in decimal degrees you may wish to specify custom intervals suited to that format.
 
+#### ol/Collection
+
+Inserting with `setAt` or `insertAt` beyond the current length used to create a sparse Collection with `undefined` inserted for any missing indexes.  This will now throw an error instead.
+
 ### 6.15.0
 
 #### Deprecated `tilePixelRatio` option for data tile sources.

src/ol/Collection.js

@@ -22,10 +22,10 @@ const Property = {
 export class CollectionEvent extends Event {
   /**
    * @param {import("./CollectionEventType.js").default} type Type.
-   * @param {*} [opt_element] Element.
-   * @param {number} [opt_index] The index of the added or removed element.
+   * @param {*} element Element.
+   * @param {number} index The index of the added or removed element.
    */
-  constructor(type, opt_element, opt_index) {
+  constructor(type, element, index) {
     super(type);
 
     /**
@@ -33,14 +33,14 @@ export class CollectionEvent extends Event {
      * @type {*}
      * @api
      */
-    this.element = opt_element;
+    this.element = element;
 
     /**
      * The index of the added or removed element.
      * @type {number}
      * @api
      */
-    this.index = opt_index;
+    this.index = index;
   }
 }
 
@@ -195,6 +195,9 @@ class Collection extends BaseObject {
    * @api
    */
   insertAt(index, elem) {
+    if (index < 0 || index > this.getLength()) {
+      throw new Error('Index out of bounds: ' + index);
+    }
     if (this.unique_) {
       this.assertUnique_(elem);
     }
@@ -274,7 +277,13 @@ class Collection extends BaseObject {
    */
   setAt(index, elem) {
     const n = this.getLength();
-    if (index < n) {
+    if (index >= n) {
+      this.insertAt(index, elem);
+      return;
+    }
+    if (index < 0) {
+      throw new Error('Index out of bounds: ' + index);
+    }
     if (this.unique_) {
       this.assertUnique_(elem, index);
     }
@@ -286,12 +295,6 @@ class Collection extends BaseObject {
     this.dispatchEvent(
       new CollectionEvent(CollectionEventType.ADD, elem, index)
     );
-    } else {
-      for (let j = n; j < index; ++j) {
-        this.insertAt(j, undefined);
-      }
-      this.insertAt(index, elem);
-    }
   }
 
   /**

test/node/ol/Collection.test.js

@@ -73,10 +73,15 @@ describe('ol/Collection.js', function () {
 
   describe('setAt', function () {
     it('sets at the correct location', function () {
-      collection.setAt(1, 1);
-      expect(collection.getLength()).to.eql(2);
-      expect(collection.item(0)).to.be(undefined);
-      expect(collection.item(1)).to.eql(1);
+      collection.setAt(0, 1);
+      collection.setAt(1, 2);
+      expect(collection.getLength()).to.be(2);
+      expect(collection.item(0)).to.be(1);
+      expect(collection.item(1)).to.be(2);
+
+      collection.setAt(0, 3);
+      expect(collection.getLength()).to.be(2);
+      expect(collection.item(0)).to.be(3);
     });
   });
 
@@ -195,23 +200,32 @@ describe('ol/Collection.js', function () {
   });
 
   describe('setAt beyond end', function () {
+    it('does not allow setting invalid index', function () {
+      try {
+        collection.setAt(1, 1);
+      } catch (e) {
+        return;
+      }
+      throw new Error('Collection should throw');
+    });
     it('triggers events properly', function () {
-      const added = [],
-        addedIndexes = [];
+      const added = [];
+      const addedIndexes = [];
       listen(collection, CollectionEventType.ADD, function (e) {
         added.push(e.element);
         addedIndexes.push(e.index);
       });
-      collection.setAt(2, 0);
-      expect(collection.getLength()).to.eql(3);
-      expect(collection.item(0)).to.be(undefined);
-      expect(collection.item(1)).to.be(undefined);
-      expect(collection.item(2)).to.eql(0);
-      expect(added.length).to.eql(3);
-      expect(added[0]).to.eql(undefined);
-      expect(added[1]).to.eql(undefined);
-      expect(added[2]).to.eql(0);
-      expect(addedIndexes).to.eql([0, 1, 2]);
+      collection.setAt(0, 0);
+      collection.setAt(1, 1);
+      collection.setAt(0, 2);
+      expect(collection.getLength()).to.be(2);
+      expect(collection.item(0)).to.be(2);
+      expect(collection.item(1)).to.be(1);
+      expect(added.length).to.be(3);
+      expect(added[0]).to.be(0);
+      expect(added[1]).to.be(1);
+      expect(added[2]).to.be(2);
+      expect(addedIndexes).to.eql([0, 1, 0]);
     });
   });