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 15c51f866b..276d4180f8 100644 --- a/src/ol/Collection.js +++ b/src/ol/Collection.js @@ -18,37 +18,39 @@ const Property = { * @classdesc * Events emitted by {@link module:ol/Collection~Collection} instances are instances of this * type. + * @template T */ 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 {T} 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); /** * The element that is added to or removed from the collection. - * @type {*} + * @type {T} * @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; } } /*** + * @template T * @template Return * @typedef {import("./Observable").OnSignature & * import("./Observable").OnSignature & - * import("./Observable").OnSignature<'add'|'remove', CollectionEvent, Return> & + * import("./Observable").OnSignature<'add'|'remove', CollectionEvent, Return> & * import("./Observable").CombinedOnSignature} CollectionOnSignature */ @@ -81,17 +83,17 @@ class Collection extends BaseObject { super(); /*** - * @type {CollectionOnSignature} + * @type {CollectionOnSignature} */ this.on; /*** - * @type {CollectionOnSignature} + * @type {CollectionOnSignature} */ this.once; /*** - * @type {CollectionOnSignature} + * @type {CollectionOnSignature} */ this.un; @@ -195,6 +197,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); } @@ -254,11 +259,16 @@ class Collection extends BaseObject { * @api */ removeAt(index) { + if (index < 0 || index >= this.getLength()) { + return undefined; + } const prev = this.array_[index]; this.array_.splice(index, 1); this.updateLength_(); this.dispatchEvent( - new CollectionEvent(CollectionEventType.REMOVE, prev, index) + /** @type {CollectionEvent} */ ( + new CollectionEvent(CollectionEventType.REMOVE, prev, index) + ) ); return prev; } @@ -271,24 +281,28 @@ 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( + /** @type {CollectionEvent} */ ( + new CollectionEvent(CollectionEventType.REMOVE, prev, index) + ) + ); + this.dispatchEvent( + /** @type {CollectionEvent} */ ( + new CollectionEvent(CollectionEventType.ADD, elem, index) + ) + ); } /** diff --git a/src/ol/PluggableMap.js b/src/ol/PluggableMap.js index 3fa410d5a1..a159feb9ed 100644 --- a/src/ol/PluggableMap.js +++ b/src/ol/PluggableMap.js @@ -439,7 +439,7 @@ class PluggableMap extends BaseObject { this.controls.addEventListener( CollectionEventType.ADD, /** - * @param {import("./Collection.js").CollectionEvent} event CollectionEvent. + * @param {import("./Collection.js").CollectionEvent} event CollectionEvent */ function (event) { event.element.setMap(this); @@ -449,7 +449,7 @@ class PluggableMap extends BaseObject { this.controls.addEventListener( CollectionEventType.REMOVE, /** - * @param {import("./Collection.js").CollectionEvent} event CollectionEvent. + * @param {import("./Collection.js").CollectionEvent} event CollectionEvent. */ function (event) { event.element.setMap(null); @@ -459,7 +459,7 @@ class PluggableMap extends BaseObject { this.interactions.addEventListener( CollectionEventType.ADD, /** - * @param {import("./Collection.js").CollectionEvent} event CollectionEvent. + * @param {import("./Collection.js").CollectionEvent} event CollectionEvent. */ function (event) { event.element.setMap(this); @@ -469,7 +469,7 @@ class PluggableMap extends BaseObject { this.interactions.addEventListener( CollectionEventType.REMOVE, /** - * @param {import("./Collection.js").CollectionEvent} event CollectionEvent. + * @param {import("./Collection.js").CollectionEvent} event CollectionEvent. */ function (event) { event.element.setMap(null); @@ -479,25 +479,20 @@ class PluggableMap extends BaseObject { this.overlays_.addEventListener( CollectionEventType.ADD, /** - * @param {import("./Collection.js").CollectionEvent} event CollectionEvent. + * @param {import("./Collection.js").CollectionEvent} event CollectionEvent. */ function (event) { - this.addOverlayInternal_( - /** @type {import("./Overlay.js").default} */ (event.element) - ); + this.addOverlayInternal_(event.element); }.bind(this) ); this.overlays_.addEventListener( CollectionEventType.REMOVE, /** - * @param {import("./Collection.js").CollectionEvent} event CollectionEvent. + * @param {import("./Collection.js").CollectionEvent} event CollectionEvent. */ function (event) { - const overlay = /** @type {import("./Overlay.js").default} */ ( - event.element - ); - const id = overlay.getId(); + const id = event.element.getId(); if (id !== undefined) { delete this.overlayIdIndex_[id.toString()]; } @@ -1709,7 +1704,12 @@ function createOptionsInternal(options) { options.layers && typeof (/** @type {?} */ (options.layers).getLayers) === 'function' ? /** @type {LayerGroup} */ (options.layers) - : new LayerGroup({layers: /** @type {Collection} */ (options.layers)}); + : new LayerGroup({ + layers: + /** @type {Collection|Array} */ ( + options.layers + ), + }); values[MapProperty.LAYERGROUP] = layerGroup; values[MapProperty.TARGET] = options.target; @@ -1717,6 +1717,7 @@ function createOptionsInternal(options) { values[MapProperty.VIEW] = options.view instanceof View ? options.view : new View(); + /** @type {Collection} */ let controls; if (options.controls !== undefined) { if (Array.isArray(options.controls)) { @@ -1726,10 +1727,11 @@ function createOptionsInternal(options) { typeof (/** @type {?} */ (options.controls).getArray) === 'function', 47 ); // Expected `controls` to be an array or an `import("./Collection.js").Collection` - controls = /** @type {Collection} */ (options.controls); + controls = options.controls; } } + /** @type {Collection} */ let interactions; if (options.interactions !== undefined) { if (Array.isArray(options.interactions)) { @@ -1740,10 +1742,11 @@ function createOptionsInternal(options) { 'function', 48 ); // Expected `interactions` to be an array or an `import("./Collection.js").Collection` - interactions = /** @type {Collection} */ (options.interactions); + interactions = options.interactions; } } + /** @type {Collection} */ let overlays; if (options.overlays !== undefined) { if (Array.isArray(options.overlays)) { diff --git a/src/ol/control.js b/src/ol/control.js index 358083e6e4..847109d3e0 100644 --- a/src/ol/control.js +++ b/src/ol/control.js @@ -50,6 +50,7 @@ export {default as ZoomToExtent} from './control/ZoomToExtent.js'; export function defaults(opt_options) { const options = opt_options ? opt_options : {}; + /** @type {Collection} */ const controls = new Collection(); const zoomControl = options.zoom !== undefined ? options.zoom : true; diff --git a/src/ol/interaction.js b/src/ol/interaction.js index 8308ae795d..ffd8a94b3d 100644 --- a/src/ol/interaction.js +++ b/src/ol/interaction.js @@ -85,6 +85,7 @@ export {default as Translate} from './interaction/Translate.js'; export function defaults(opt_options) { const options = opt_options ? opt_options : {}; + /** @type {Collection} */ const interactions = new Collection(); const kinetic = new Kinetic(-0.005, 0.05, 100); diff --git a/src/ol/interaction/Modify.js b/src/ol/interaction/Modify.js index fe184f355d..4fccb73ab3 100644 --- a/src/ol/interaction/Modify.js +++ b/src/ol/interaction/Modify.js @@ -361,6 +361,7 @@ class Modify extends PointerInteraction { */ this.hitDetection_ = null; + /** @type {Collection} */ let features; if (options.features) { features = options.features; @@ -574,11 +575,11 @@ class Modify extends PointerInteraction { } /** - * @param {import("../Collection.js").CollectionEvent} evt Event. + * @param {import("../Collection.js").CollectionEvent} evt Event. * @private */ handleFeatureAdd_(evt) { - this.addFeature_(/** @type {Feature} */ (evt.element)); + this.addFeature_(evt.element); } /** @@ -594,12 +595,11 @@ class Modify extends PointerInteraction { } /** - * @param {import("../Collection.js").CollectionEvent} evt Event. + * @param {import("../Collection.js").CollectionEvent} evt Event. * @private */ handleFeatureRemove_(evt) { - const feature = /** @type {Feature} */ (evt.element); - this.removeFeature_(feature); + this.removeFeature_(evt.element); } /** diff --git a/src/ol/interaction/Select.js b/src/ol/interaction/Select.js index 556fcbea4e..6efee1180f 100644 --- a/src/ol/interaction/Select.js +++ b/src/ol/interaction/Select.js @@ -75,7 +75,7 @@ const SelectEventType = { * @property {boolean} [multi=false] A boolean that determines if the default * behaviour should select only single features or all (overlapping) features at * the clicked map position. The default of `false` means single select. - * @property {import("../Collection.js").default} [features] + * @property {Collection} [features] * Collection where the interaction will place selected features. Optional. If * not set the interaction will create a collection. In any case the collection * used by the interaction is returned by @@ -245,7 +245,7 @@ class Select extends Interaction { /** * @private - * @type {import("../Collection.js").default} + * @type {Collection} */ this.features_ = options.features || new Collection(); @@ -290,7 +290,7 @@ class Select extends Interaction { /** * Get the selected features. - * @return {import("../Collection.js").default} Features collection. + * @return {Collection} Features collection. * @api */ getFeatures() { @@ -367,7 +367,7 @@ class Select extends Interaction { } /** - * @param {import("../Collection.js").CollectionEvent} evt Event. + * @param {import("../Collection.js").CollectionEvent} evt Event. * @private */ addFeature_(evt) { @@ -396,13 +396,12 @@ class Select extends Interaction { } /** - * @param {import("../Collection.js").CollectionEvent} evt Event. + * @param {import("../Collection.js").CollectionEvent} evt Event. * @private */ removeFeature_(evt) { - const feature = evt.element; if (this.style_) { - this.restorePreviousStyle_(feature); + this.restorePreviousStyle_(evt.element); } } @@ -414,7 +413,7 @@ class Select extends Interaction { } /** - * @param {import("../Feature.js").default} feature Feature + * @param {Feature} feature Feature * @private */ applySelectedStyle_(feature) { @@ -426,7 +425,7 @@ class Select extends Interaction { } /** - * @param {import("../Feature.js").default} feature Feature + * @param {Feature} feature Feature * @private */ restorePreviousStyle_(feature) { @@ -450,7 +449,7 @@ class Select extends Interaction { } /** - * @param {import("../Feature.js").default} feature Feature. + * @param {Feature} feature Feature. * @private */ removeFeatureLayerAssociation_(feature) { @@ -476,12 +475,12 @@ class Select extends Interaction { const features = this.getFeatures(); /** - * @type {Array} + * @type {Array} */ const deselected = []; /** - * @type {Array} + * @type {Array} */ const selected = []; diff --git a/src/ol/interaction/Snap.js b/src/ol/interaction/Snap.js index 4395fb8e11..794a9ce6fa 100644 --- a/src/ol/interaction/Snap.js +++ b/src/ol/interaction/Snap.js @@ -45,7 +45,7 @@ import {listen, unlistenByKey} from '../events.js'; */ /** - * @param {import("../source/Vector.js").VectorSourceEvent|import("../Collection.js").CollectionEvent} evt Event. + * @param {import("../source/Vector.js").VectorSourceEvent|import("../Collection.js").CollectionEvent} evt Event. * @return {import("../Feature.js").default} Feature. */ function getFeatureFromEvent(evt) { @@ -55,11 +55,13 @@ function getFeatureFromEvent(evt) { return /** @type {import("../source/Vector.js").VectorSourceEvent} */ (evt) .feature; } else if ( - /** @type {import("../Collection.js").CollectionEvent} */ (evt).element + /** @type {import("../Collection.js").CollectionEvent} */ ( + evt + ).element ) { - return /** @type {import("../Feature.js").default} */ ( - /** @type {import("../Collection.js").CollectionEvent} */ (evt).element - ); + return /** @type {import("../Collection.js").CollectionEvent} */ ( + evt + ).element; } } @@ -261,6 +263,7 @@ class Snap extends PointerInteraction { * @private */ getFeatures_() { + /** @type {import("../Collection.js").default|Array} */ let features; if (this.features_) { features = this.features_; @@ -284,7 +287,7 @@ class Snap extends PointerInteraction { } /** - * @param {import("../source/Vector.js").VectorSourceEvent|import("../Collection.js").CollectionEvent} evt Event. + * @param {import("../source/Vector.js").VectorSourceEvent|import("../Collection.js").CollectionEvent} evt Event. * @private */ handleFeatureAdd_(evt) { @@ -293,7 +296,7 @@ class Snap extends PointerInteraction { } /** - * @param {import("../source/Vector.js").VectorSourceEvent|import("../Collection.js").CollectionEvent} evt Event. + * @param {import("../source/Vector.js").VectorSourceEvent|import("../Collection.js").CollectionEvent} evt Event. * @private */ handleFeatureRemove_(evt) { diff --git a/src/ol/layer/Group.js b/src/ol/layer/Group.js index 72cbf0c855..4361c99387 100644 --- a/src/ol/layer/Group.js +++ b/src/ol/layer/Group.js @@ -66,7 +66,7 @@ export class GroupEvent extends Event { * visible. * @property {number} [maxZoom] The maximum view zoom level (inclusive) at which this layer will * be visible. - * @property {Array|import("../Collection.js").default} [layers] Child layers. + * @property {Array|Collection} [layers] Child layers. * @property {Object} [properties] Arbitrary observable properties. Can be accessed with `#get()` and `#set()`. */ @@ -214,26 +214,22 @@ class LayerGroup extends BaseLayer { } /** - * @param {import("../Collection.js").CollectionEvent} collectionEvent CollectionEvent. + * @param {import("../Collection.js").CollectionEvent} collectionEvent CollectionEvent. * @private */ handleLayersAdd_(collectionEvent) { - const layer = /** @type {import("./Base.js").default} */ ( - collectionEvent.element - ); + const layer = collectionEvent.element; this.registerLayerListeners_(layer); this.dispatchEvent(new GroupEvent('addlayer', layer)); this.changed(); } /** - * @param {import("../Collection.js").CollectionEvent} collectionEvent CollectionEvent. + * @param {import("../Collection.js").CollectionEvent} collectionEvent CollectionEvent. * @private */ handleLayersRemove_(collectionEvent) { - const layer = /** @type {import("./Base.js").default} */ ( - collectionEvent.element - ); + const layer = collectionEvent.element; const key = getUid(layer); this.listenerKeys_[key].forEach(unlistenByKey); delete this.listenerKeys_[key]; @@ -244,13 +240,13 @@ class LayerGroup extends BaseLayer { /** * Returns the {@link module:ol/Collection~Collection collection} of {@link module:ol/layer/Layer~Layer layers} * in this group. - * @return {!import("../Collection.js").default} Collection of + * @return {!Collection} Collection of * {@link module:ol/layer/Base~BaseLayer layers} that are part of this group. * @observable * @api */ getLayers() { - return /** @type {!import("../Collection.js").default} */ ( + return /** @type {!Collection} */ ( this.get(Property.LAYERS) ); } @@ -258,7 +254,7 @@ class LayerGroup extends BaseLayer { /** * Set the {@link module:ol/Collection~Collection collection} of {@link module:ol/layer/Layer~Layer layers} * in this group. - * @param {!import("../Collection.js").default} layers Collection of + * @param {!Collection} layers Collection of * {@link module:ol/layer/Base~BaseLayer layers} that are part of this group. * @observable * @api diff --git a/src/ol/source/Vector.js b/src/ol/source/Vector.js index 12b283c4a7..1f518d0ef4 100644 --- a/src/ol/source/Vector.js +++ b/src/ol/source/Vector.js @@ -72,7 +72,7 @@ export class VectorSourceEvent extends Event { /** * @typedef {Object} Options * @property {import("./Source.js").AttributionLike} [attributions] Attributions. - * @property {Array|Collection} [features] + * @property {Array>|Collection>} [features] * Features. If provided as {@link module:ol/Collection~Collection}, the features in the source * and the collection will stay in sync. * @property {import("../format/Feature.js").default} [format] The feature format used by the XHR @@ -159,6 +159,7 @@ export class VectorSourceEvent extends Event { * @property {boolean} [wrapX=true] Wrap the world horizontally. For vector editing across the * -180° and 180° meridians to work properly, this should be set to `false`. The * resulting geometry coordinates will then exceed the world bounds. + * @template {import("../geom/Geometry.js").default} [Geometry=import("../geom/Geometry.js").default] */ /** @@ -173,7 +174,7 @@ export class VectorSourceEvent extends Event { */ class VectorSource extends Source { /** - * @param {Options} [opt_options] Vector source options. + * @param {Options} [opt_options] Vector source options. */ constructor(opt_options) { const options = opt_options || {}; @@ -296,17 +297,14 @@ class VectorSource extends Source { */ this.featuresCollection_ = null; - let collection, features; + /** @type {Collection>} */ + let collection; + /** @type {Array>} */ + let features; if (Array.isArray(options.features)) { - features = - /** @type {Array>} */ ( - options.features - ); + features = options.features; } else if (options.features) { - collection = - /** @type {Collection>} */ ( - options.features - ); + collection = options.features; features = collection.getArray(); } if (!useSpatialIndex && collection === undefined) { @@ -500,16 +498,12 @@ class VectorSource extends Source { collection.addEventListener( CollectionEventType.ADD, /** - * @param {import("../Collection.js").CollectionEvent} evt The collection event + * @param {import("../Collection.js").CollectionEvent>} evt The collection event */ function (evt) { if (!modifyingCollection) { modifyingCollection = true; - this.addFeature( - /** @type {import("../Feature.js").default} */ ( - evt.element - ) - ); + this.addFeature(evt.element); modifyingCollection = false; } }.bind(this) @@ -517,16 +511,12 @@ class VectorSource extends Source { collection.addEventListener( CollectionEventType.REMOVE, /** - * @param {import("../Collection.js").CollectionEvent} evt The collection event + * @param {import("../Collection.js").CollectionEvent>} evt The collection event */ function (evt) { if (!modifyingCollection) { modifyingCollection = true; - this.removeFeature( - /** @type {import("../Feature.js").default} */ ( - evt.element - ) - ); + this.removeFeature(evt.element); modifyingCollection = false; } }.bind(this) diff --git a/test/node/ol/Collection.test.js b/test/node/ol/Collection.test.js index 6723172706..8f5551447e 100644 --- a/test/node/ol/Collection.test.js +++ b/test/node/ol/Collection.test.js @@ -5,6 +5,7 @@ import sinon from 'sinon'; import {listen} from '../../../src/ol/events.js'; describe('ol/Collection.js', function () { + /** @type {Collection} */ let collection; beforeEach(function () { @@ -72,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); }); }); @@ -86,6 +92,13 @@ describe('ol/Collection.js', function () { expect(collection.item(0)).to.eql(0); expect(collection.item(1)).to.eql(2); }); + it('does not fire event for invalid index', function () { + const collection = new Collection([0, 1, 2]); + collection.on('remove', function () { + throw new Error('Should not fire event for invalid index'); + }); + expect(collection.removeAt(3)).to.be(undefined); + }); }); describe('forEach', function () { @@ -187,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]); }); });