Add event propagation system to Evented (mapbox#3244)

* Minor refactoring of Evented * Refactor Evented tests * Add Evented#pipe * Fix flaky map test * Use Evented#pipe in SourceCache * Rename "Evented#pipe" to "Evented#forwardEvents" * Add "passes original 'target' to forwarded listeners" test * Use event forwarding for source -> style events * Use event forwarding for layer -> style events * Use event forwarding for sprite -> style events * Use event forwarding for all style -> map events * Get rid of typed error events * Replace "forwardEvents" with "setEventedParent" * Minor refactoring * Make Evented#setEventedParent private * Fix benchmarks * Eliminate remaining ImageSprite#load events * Fix removed bound arguments per PR comment mapbox#3244 (comment)
developmentseed · Sep 23, 2016 · 24f2135 · 24f2135
1 parent f3ad12b
commit 24f2135
Show file tree

Hide file tree

Showing 18 changed files with 409 additions and 439 deletions.
diff --git a/bench/benchmarks/buffer.js b/bench/benchmarks/buffer.js
@@ -90,7 +90,7 @@ function preloadAssets(stylesheet, callback) {
 
     var style = new Style(stylesheet);
 
-    style.on('load', function() {
+    style.on('style.load', function() {
         function getGlyphs(params, callback) {
             style['get glyphs'](0, params, function(err, glyphs) {
                 assets.glyphs[JSON.stringify(params)] = glyphs;

diff --git a/bench/benchmarks/style_load.js b/bench/benchmarks/style_load.js
@@ -28,7 +28,7 @@ module.exports = function() {
                 .on('error', function(err) {
                     evented.fire('error', { error: err });
                 })
-                .on('load', function() {
+                .on('style.load', function() {
                     var time = performance.now() - timeStart;
                     timeSum += time;
                     timeCount++;

diff --git a/js/source/geojson_source.js b/js/source/geojson_source.js
@@ -88,7 +88,7 @@ function GeoJSONSource(id, options, dispatcher) {
             this.fire('error', {error: err});
             return;
         }
-        this.fire('load');
+        this.fire('source.load');
     }.bind(this));
 }
 
@@ -119,7 +119,7 @@ GeoJSONSource.prototype = util.inherit(Evented, /** @lends GeoJSONSource.prototy
             if (err) {
                 return this.fire('error', { error: err });
             }
-            this.fire('change');
+            this.fire('source.change');
         }.bind(this));
 
         return this;

diff --git a/js/source/image_source.js b/js/source/image_source.js
@@ -53,7 +53,7 @@ function ImageSource(id, options, dispatcher) {
 
         this.image = image;
         this._loaded = true;
-        this.fire('load');
+        this.fire('source.load');
 
         if (this.map) {
             this.setCoordinates(options.coordinates);
@@ -106,7 +106,7 @@ ImageSource.prototype = util.inherit(Evented, /** @lends ImageSource.prototype *
                 Math.round((zoomedCoord.row - centerCoord.row) * EXTENT));
         });
 
-        this.fire('change');
+        this.fire('source.change');
         return this;
     },
 

diff --git a/js/source/raster_tile_source.js b/js/source/raster_tile_source.js
@@ -17,7 +17,7 @@ function RasterTileSource(id, options, dispatcher) {
             return this.fire('error', err);
         }
         util.extend(this, tileJSON);
-        this.fire('load');
+        this.fire('source.load');
     }.bind(this));
 }
 

diff --git a/js/source/source_cache.js b/js/source/source_cache.js
@@ -27,8 +27,10 @@ function SourceCache(id, options, dispatcher) {
     this.id = id;
     this.dispatcher = dispatcher;
 
-    var source = this._source = Source.create(id, options, dispatcher)
-    .on('load', function () {
+    var source = this._source = Source.create(id, options, dispatcher);
+    source.setEventedParent(this);
+
+    this.on('source.load', function() {
         if (this.map && this._source.onAdd) { this._source.onAdd(this.map); }
 
         this._sourceLoaded = true;
@@ -42,20 +44,18 @@ function SourceCache(id, options, dispatcher) {
         this.attribution = source.attribution;
 
         this.vectorLayerIds = source.vectorLayerIds;
+    });
 
-        this.fire('load');
-    }.bind(this))
-    .on('error', function (e) {
+    this.on('error', function() {
         this._sourceErrored = true;
-        this.fire('error', e);
-    }.bind(this))
-    .on('change', function () {
+    });
+
+    this.on('source.change', function() {
         this.reload();
         if (this.transform) {
             this.update(this.transform, this.map && this.map.style.rasterFadeDuration);
         }
-        this.fire('change');
-    }.bind(this));
+    });
 
     this._tiles = {};
     this._cache = new Cache(0, this.unloadTile.bind(this));
@@ -160,14 +160,12 @@ SourceCache.prototype = util.inherit(Evented, {
     _tileLoaded: function (tile, err) {
         if (err) {
             tile.state = 'errored';
-            this.fire('tile.error', {tile: tile, error: err});
-            this._source.fire('tile.error', {tile: tile, error: err});
+            this._source.fire('error', {tile: tile, error: err});
             return;
         }
 
         tile.source = this;
         tile.timeAdded = new Date().getTime();
-        this.fire('tile.load', {tile: tile});
         this._source.fire('tile.load', {tile: tile});
 
         // HACK this is nescessary to fix https://github.com/mapbox/mapbox-gl-js/issues/2986
@@ -409,7 +407,6 @@ SourceCache.prototype = util.inherit(Evented, {
 
         tile.uses++;
         this._tiles[coord.id] = tile;
-        this.fire('tile.add', {tile: tile});
         this._source.fire('tile.add', {tile: tile});
 
         return tile;
@@ -428,7 +425,6 @@ SourceCache.prototype = util.inherit(Evented, {
 
         tile.uses--;
         delete this._tiles[id];
-        this.fire('tile.remove', {tile: tile});
         this._source.fire('tile.remove', {tile: tile});
 
         if (tile.uses > 0)

diff --git a/js/source/vector_tile_source.js b/js/source/vector_tile_source.js
@@ -23,7 +23,7 @@ function VectorTileSource(id, options, dispatcher) {
             return;
         }
         util.extend(this, tileJSON);
-        this.fire('load');
+        this.fire('source.load');
     }.bind(this));
 }
 

diff --git a/js/source/video_source.js b/js/source/video_source.js
@@ -73,7 +73,7 @@ function VideoSource(id, options) {
             this.setCoordinates(options.coordinates);
         }
 
-        this.fire('load');
+        this.fire('source.load');
     }.bind(this));
 }
 
@@ -135,7 +135,7 @@ VideoSource.prototype = util.inherit(Evented, /** @lends VideoSource.prototype *
                 Math.round((zoomedCoord.row - centerCoord.row) * EXTENT));
         });
 
-        this.fire('change');
+        this.fire('source.change');
         return this;
     },
 

diff --git a/js/style/image_sprite.js b/js/style/image_sprite.js
@@ -20,7 +20,7 @@ function ImageSprite(base) {
         }
 
         this.data = data;
-        if (this.img) this.fire('load');
+        if (this.img) this.fire('style.change');
     }.bind(this));
 
     ajax.getImage(normalizeURL(base, format, '.png'), function(err, img) {
@@ -41,7 +41,7 @@ function ImageSprite(base) {
         }
 
         this.img = img;
-        if (this.data) this.fire('load');
+        if (this.data) this.fire('style.change');
     }.bind(this));
 }
 
@@ -58,7 +58,7 @@ ImageSprite.prototype.loaded = function() {
 ImageSprite.prototype.resize = function(/*gl*/) {
     if (browser.devicePixelRatio > 1 !== this.retina) {
         var newSprite = new ImageSprite(this.base);
-        newSprite.on('load', function() {
+        newSprite.on('style.change', function() {
             this.img = newSprite.img;
             this.data = newSprite.data;
             this.retina = newSprite.retina;

diff --git a/js/style/style.js b/js/style/style.js
@@ -34,12 +34,7 @@ function Style(stylesheet, animationLoop, options) {
     this.sources = {};
     this.zoomHistory = {};
 
-    util.bindAll([
-        '_forwardSourceEvent',
-        '_forwardTileEvent',
-        '_forwardLayerEvent',
-        '_redoPlacement'
-    ], this);
+    util.bindAll(['_redoPlacement'], this);
 
     this._resetUpdates();
 
@@ -67,12 +62,12 @@ function Style(stylesheet, animationLoop, options) {
 
         if (stylesheet.sprite) {
             this.sprite = new ImageSprite(stylesheet.sprite);
-            this.sprite.on('load', this.fire.bind(this, 'change'));
+            this.sprite.setEventedParent(this);
         }
 
         this.glyphSource = new GlyphSource(stylesheet.glyphs);
         this._resolve();
-        this.fire('load');
+        this.fire('style.load');
     }.bind(this);
 
     if (typeof stylesheet === 'string') {
@@ -146,7 +141,7 @@ Style.prototype = util.inherit(Evented, {
             if (layerJSON.ref) continue;
             layer = StyleLayer.create(layerJSON);
             this._layers[layer.id] = layer;
-            layer.on('error', this._forwardLayerEvent);
+            layer.setEventedParent(this, {layer: {id: layer.id}});
         }
 
         // resolve all layers WITH a ref
@@ -156,7 +151,7 @@ Style.prototype = util.inherit(Evented, {
             var refLayer = this.getLayer(layerJSON.ref);
             layer = StyleLayer.create(layerJSON, refLayer);
             this._layers[layer.id] = layer;
-            layer.on('error', this._forwardLayerEvent);
+            layer.setEventedParent(this, {layer: {id: layer.id}});
         }
 
         this._groupLayers();
@@ -305,7 +300,7 @@ Style.prototype = util.inherit(Evented, {
         this._applyClasses(classes, options);
 
         if (this._updates.changed) {
-            this.fire('change');
+            this.fire('style.change');
         }
 
         this._resetUpdates();
@@ -340,15 +335,7 @@ Style.prototype = util.inherit(Evented, {
         source = new SourceCache(id, source, this.dispatcher);
         this.sources[id] = source;
         source.style = this;
-        source
-            .on('load', this._forwardSourceEvent)
-            .on('error', this._forwardSourceEvent)
-            .on('change', this._forwardSourceEvent)
-            .on('tile.add', this._forwardTileEvent)
-            .on('tile.load', this._forwardTileEvent)
-            .on('tile.error', this._forwardTileEvent)
-            .on('tile.remove', this._forwardTileEvent)
-            .on('tile.stats', this._forwardTileEvent);
+        source.setEventedParent(this, {source: source});
 
         this._updates.events.push(['source.add', {source: source}]);
         this._updates.changed = true;
@@ -372,15 +359,7 @@ Style.prototype = util.inherit(Evented, {
         var source = this.sources[id];
         delete this.sources[id];
         delete this._updates.sources[id];
-        source
-            .off('load', this._forwardSourceEvent)
-            .off('error', this._forwardSourceEvent)
-            .off('change', this._forwardSourceEvent)
-            .off('tile.add', this._forwardTileEvent)
-            .off('tile.load', this._forwardTileEvent)
-            .off('tile.error', this._forwardTileEvent)
-            .off('tile.remove', this._forwardTileEvent)
-            .off('tile.stats', this._forwardTileEvent);
+        source.setEventedParent(null);
 
         this._updates.events.push(['source.remove', {source: source}]);
         this._updates.changed = true;
@@ -420,7 +399,7 @@ Style.prototype = util.inherit(Evented, {
         }
         this._validateLayer(layer);
 
-        layer.on('error', this._forwardLayerEvent);
+        layer.setEventedParent(this, {layer: {id: layer.id}});
 
         this._layers[layer.id] = layer;
         this._order.splice(before ? this._order.indexOf(before) : Infinity, 0, layer.id);
@@ -454,7 +433,7 @@ Style.prototype = util.inherit(Evented, {
             }
         }
 
-        layer.off('error', this._forwardLayerEvent);
+        layer.setEventedParent(null);
 
         delete this._layers[id];
         delete this._updates.layers[id];
@@ -730,18 +709,6 @@ Style.prototype = util.inherit(Evented, {
         }
     },
 
-    _forwardSourceEvent: function(e) {
-        this.fire('source.' + e.type, util.extend({source: e.target.getSource()}, e));
-    },
-
-    _forwardTileEvent: function(e) {
-        this.fire(e.type, util.extend({source: e.target}, e));
-    },
-
-    _forwardLayerEvent: function(e) {
-        this.fire('layer.' + e.type, util.extend({layer: {id: e.target.id}}, e));
-    },
-
     // Callbacks from web workers
 
     'get icons': function(mapId, params, callback) {
@@ -751,7 +718,7 @@ Style.prototype = util.inherit(Evented, {
             spriteAtlas.setSprite(sprite);
             spriteAtlas.addIcons(params.icons, callback);
         } else {
-            sprite.on('load', function() {
+            sprite.on('style.change', function() {
                 spriteAtlas.setSprite(sprite);
                 spriteAtlas.addIcons(params.icons, callback);
             });