diff --git a/changelog/upgrade-notes.md b/changelog/upgrade-notes.md index cf6fec5f25..de2f36035d 100644 --- a/changelog/upgrade-notes.md +++ b/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. diff --git a/src/ol/Collection.js b/src/ol/Collection.js index 5b562c05a5..79ee531a3d 100644 --- a/src/ol/Collection.js +++ b/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,24 +277,24 @@ class Collection extends BaseObject { */ setAt(index, elem) { const n = this.getLength(); - if (index < n) { - if (this.unique_) { - this.assertUnique_(elem, index); - } - const prev = this.array_[index]; - this.array_[index] = elem; - this.dispatchEvent( - new CollectionEvent(CollectionEventType.REMOVE, prev, index) - ); - this.dispatchEvent( - new CollectionEvent(CollectionEventType.ADD, elem, index) - ); - } else { - for (let j = n; j < index; ++j) { - this.insertAt(j, undefined); - } + 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); + } + const prev = this.array_[index]; + this.array_[index] = elem; + this.dispatchEvent( + new CollectionEvent(CollectionEventType.REMOVE, prev, index) + ); + this.dispatchEvent( + new CollectionEvent(CollectionEventType.ADD, elem, index) + ); } /** diff --git a/test/node/ol/Collection.test.js b/test/node/ol/Collection.test.js index bbef014f6e..8f5551447e 100644 --- a/test/node/ol/Collection.test.js +++ b/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]); }); });