Refactor internal events API (#1266)

* Refactor events API

src/events.js

@@ -212,10 +212,9 @@ export default function(ctx) {
     currentModeRender(geojson, push) {
       return currentMode.render(geojson, push);
     },
-    fire(name, event) {
-      if (events[name]) {
-        events[name](event);
-      }
+    fire(eventName, eventData) {
+      if (!ctx.map) return;
+      ctx.map.fire(eventName, eventData);
     },
     addEventListeners() {
       ctx.map.on('mousemove', events.mousemove);

src/modes/direct_select.js

@@ -13,7 +13,7 @@ const DirectSelect = {};
 // INTERNAL FUCNTIONS
 
 DirectSelect.fireUpdate = function() {
-  this.map.fire(Constants.events.UPDATE, {
+  this.fire(Constants.events.UPDATE, {
     action: Constants.updateActions.CHANGE_COORDINATES,
     features: this.getSelected().map(f => f.toGeoJSON())
   });

src/modes/draw_line_string.js

@@ -118,7 +118,7 @@ DrawLineString.onStop = function(state) {
   //remove last added coordinate
   state.line.removeCoordinate(`${state.currentVertexPosition}`);
   if (state.line.isValid()) {
-    this.map.fire(Constants.events.CREATE, {
+    this.fire(Constants.events.CREATE, {
       features: [state.line.toGeoJSON()]
     });
   } else {

src/modes/draw_point.js

@@ -34,7 +34,7 @@ DrawPoint.stopDrawingAndRemove = function(state) {
 DrawPoint.onTap = DrawPoint.onClick = function(state, e) {
   this.updateUIClasses({ mouse: Constants.cursors.MOVE });
   state.point.updateCoordinate('', e.lngLat.lng, e.lngLat.lat);
-  this.map.fire(Constants.events.CREATE, {
+  this.fire(Constants.events.CREATE, {
     features: [state.point.toGeoJSON()]
   });
   this.changeMode(Constants.modes.SIMPLE_SELECT, { featureIds: [state.point.id] });

src/modes/draw_polygon.js

@@ -78,7 +78,7 @@ DrawPolygon.onStop = function(state) {
   //remove last added coordinate
   state.polygon.removeCoordinate(`0.${state.currentVertexPosition}`);
   if (state.polygon.isValid()) {
-    this.map.fire(Constants.events.CREATE, {
+    this.fire(Constants.events.CREATE, {
       features: [state.polygon.toGeoJSON()]
     });
   } else {

src/modes/mode_interface_accessors.js

@@ -151,6 +151,16 @@ ModeInterface.prototype.changeMode = function(mode, opts = {}, eventOpts = {}) {
   return this._ctx.events.changeMode(mode, opts, eventOpts);
 };
 
+/**
+ * Fire a map event
+ * @name this.fire
+ * @param {String} eventName - the event name.
+ * @param {Object} eventData - the event data object.
+ */
+ModeInterface.prototype.fire = function(eventName, eventData) {
+  return this._ctx.events.fire(eventName, eventData);
+};
+
 /**
  * Update the state of draw map classes
  * @name this.updateUIClasses

src/modes/simple_select.js

@@ -35,7 +35,7 @@ SimpleSelect.onSetup = function(opts) {
 };
 
 SimpleSelect.fireUpdate = function() {
-  this.map.fire(Constants.events.UPDATE, {
+  this.fire(Constants.events.UPDATE, {
     action: Constants.updateActions.MOVE,
     features: this.getSelected().map(f => f.toGeoJSON())
   });
@@ -347,7 +347,7 @@ SimpleSelect.onCombineFeatures = function() {
     this.deleteFeature(this.getSelectedIds(), { silent: true });
     this.setSelected([multiFeature.id]);
 
-    this.map.fire(Constants.events.COMBINE_FEATURES, {
+    this.fire(Constants.events.COMBINE_FEATURES, {
       createdFeatures: [multiFeature.toGeoJSON()],
       deletedFeatures: featuresCombined
     });
@@ -378,7 +378,7 @@ SimpleSelect.onUncombineFeatures = function() {
   }
 
   if (createdFeatures.length > 1) {
-    this.map.fire(Constants.events.UNCOMBINE_FEATURES, {
+    this.fire(Constants.events.UNCOMBINE_FEATURES, {
       createdFeatures,
       deletedFeatures: featuresUncombined
     });

src/render.js

@@ -52,33 +52,7 @@ export default function render() {
     features: store.sources.hot
   });
 
-  if (store._emitSelectionChange) {
-    store.ctx.map.fire(Constants.events.SELECTION_CHANGE, {
-      features: store.getSelected().map(feature => feature.toGeoJSON()),
-      points: store.getSelectedCoordinates().map(coordinate => ({
-        type: Constants.geojsonTypes.FEATURE,
-        properties: {},
-        geometry: {
-          type: Constants.geojsonTypes.POINT,
-          coordinates: coordinate.coordinates
-        }
-      }))
-    });
-    store._emitSelectionChange = false;
-  }
-
-  if (store._deletedFeaturesToEmit.length) {
-    const geojsonToEmit = store._deletedFeaturesToEmit.map(feature => feature.toGeoJSON());
-
-    store._deletedFeaturesToEmit = [];
-
-    store.ctx.map.fire(Constants.events.DELETE, {
-      features: geojsonToEmit
-    });
-  }
-
   cleanup();
-  store.ctx.map.fire(Constants.events.RENDER, {});
 
   function cleanup() {
     store.isDirty = false;

src/store.js

@@ -1,15 +1,14 @@
 import toDenseArray from './lib/to_dense_array.js';
 import StringSet from './lib/string_set.js';
 import render from './render.js';
-import {interactions} from './constants.js';
+import * as Constants from './constants.js';
 
 export default function Store(ctx) {
   this._features = {};
   this._featureIds = new StringSet();
   this._selectedFeatureIds = new StringSet();
   this._selectedCoordinates = [];
   this._changedFeatureIds = new StringSet();
-  this._deletedFeaturesToEmit = [];
   this._emitSelectionChange = false;
   this._mapInitialConfig = {};
   this.ctx = ctx;
@@ -25,13 +24,33 @@ export default function Store(ctx) {
       renderRequest = requestAnimationFrame(() => {
         renderRequest = null;
         render.call(this);
+
+        // Fire deduplicated selection change event
+        if (this._emitSelectionChange) {
+          this.ctx.events.fire(Constants.events.SELECTION_CHANGE, {
+            features: this.getSelected().map(feature => feature.toGeoJSON()),
+            points: this.getSelectedCoordinates().map(coordinate => ({
+              type: Constants.geojsonTypes.FEATURE,
+              properties: {},
+              geometry: {
+                type: Constants.geojsonTypes.POINT,
+                coordinates: coordinate.coordinates
+              }
+            }))
+          });
+
+          this._emitSelectionChange = false;
+        }
+
+        // Fire render event
+        this.ctx.events.fire(Constants.events.RENDER, {});
       });
     }
   };
+
   this.isDirty = false;
 }
 
-
 /**
  * Delays all rendering until the returned function is invoked
  * @return {Function} renderBatch
@@ -119,18 +138,24 @@ Store.prototype.add = function(feature) {
  * @return {Store} this
  */
 Store.prototype.delete = function(featureIds, options = {}) {
+  const deletedFeaturesToEmit = [];
   toDenseArray(featureIds).forEach((id) => {
     if (!this._featureIds.has(id)) return;
     this._featureIds.delete(id);
     this._selectedFeatureIds.delete(id);
     if (!options.silent) {
-      if (this._deletedFeaturesToEmit.indexOf(this._features[id]) === -1) {
-        this._deletedFeaturesToEmit.push(this._features[id]);
+      if (deletedFeaturesToEmit.indexOf(this._features[id]) === -1) {
+        deletedFeaturesToEmit.push(this._features[id].toGeoJSON());
       }
     }
     delete this._features[id];
     this.isDirty = true;
   });
+
+  if (deletedFeaturesToEmit.length) {
+    this.ctx.events.fire(Constants.events.DELETE, {features: deletedFeaturesToEmit});
+  }
+
   refreshSelectedCoordinates(this, options);
   return this;
 };
@@ -256,7 +281,7 @@ Store.prototype.getSelectedIds = function() {
  * @return {Array<Object>} Selected features.
  */
 Store.prototype.getSelected = function() {
-  return this._selectedFeatureIds.values().map(id => this.get(id));
+  return this.getSelectedIds().map(id => this.get(id));
 };
 
 /**
@@ -305,7 +330,7 @@ function refreshSelectedCoordinates(store, options) {
  * Stores the initial config for a map, so that we can set it again after we're done.
 */
 Store.prototype.storeMapConfig = function() {
-  interactions.forEach((interaction) => {
+  Constants.interactions.forEach((interaction) => {
     const interactionSet = this.ctx.map[interaction];
     if (interactionSet) {
       this._mapInitialConfig[interaction] = this.ctx.map[interaction].isEnabled();

test/store.test.js

@@ -1,13 +1,20 @@
 import test from 'node:test';
 import assert from 'node:assert/strict';
+import {spy} from 'sinon';
+
 import Store from '../src/store.js';
 import createFeature from './utils/create_feature.js';
 import getPublicMemberKeys from './utils/get_public_member_keys.js';
 import createMap from './utils/create_map.js';
 
 function createStore() {
-  const map = createMap();
-  const ctx = { map };
+  const ctx = {
+    map: createMap(),
+    events: {
+      fire: spy()
+    }
+  };
+
   return new Store(ctx);
 }
 
@@ -211,16 +218,16 @@ test('Store#setSelected', () => {
   const line = createFeature('line');
   const polygon = createFeature('polygon');
 
-  store.setSelected(point.id);
+  store.setSelected(point.id, {silent: true});
   assert.deepEqual(store.getSelectedIds(), [point.id]);
 
-  store.setSelected([line.id, polygon.id]);
+  store.setSelected([line.id, polygon.id], {silent: true});
   assert.deepEqual(store.getSelectedIds(), [line.id, polygon.id]);
 
-  store.setSelected(line.id);
+  store.setSelected(line.id, {silent: true});
   assert.deepEqual(store.getSelectedIds(), [line.id]);
 
-  store.setSelected();
+  store.setSelected(undefined, {silent: true});
   assert.deepEqual(store.getSelectedIds(), []);
 });
 

test/utils/create_mock_draw_mode_context.js

@@ -21,6 +21,7 @@ export default function createMockDrawModeContext() {
       }
     },
     events: {
+      fire: spy(),
       changeMode: spy(),
       actionable: spy()
     },