refactor to make everything dependent on 'data' event with new proper…

…ties
mapbox · Mar 6, 2017 · 799779c · 799779c
1 parent 17cda5c
commit 799779c
Show file tree

Hide file tree

Showing 15 changed files with 957 additions and 902 deletions.
diff --git a/src/source/geojson_source.js b/src/source/geojson_source.js
@@ -108,8 +108,7 @@ class GeoJSONSource extends Evented {
                 this.fire('error', {error: err});
                 return;
             }
-            this.fire('source.load');
-            this.fire('data', {dataType: 'source'});
+            this.fire('data', {dataType: 'source', metadata: true});
         });
     }
 
@@ -131,8 +130,7 @@ class GeoJSONSource extends Evented {
             if (err) {
                 return this.fire('error', { error: err });
             }
-            this.fire('source.update');
-            this.fire('data', {dataType: 'source'});
+            this.fire('data', {dataType: 'source', update: true});
         });
 
         return this;

diff --git a/src/source/image_source.js b/src/source/image_source.js
@@ -75,11 +75,9 @@ class ImageSource extends Evented {
     }
 
     _finishLoading() {
-        this.fire('source.load');
-
         if (this.map) {
             this.setCoordinates(this.coordinates);
-            this.fire('data', {dataType: 'source'});
+            this.fire('data', {dataType: 'source', metadata: true});
         }
     }
 
@@ -116,7 +114,8 @@ class ImageSource extends Evented {
         centerCoord.column = Math.round(centerCoord.column);
         centerCoord.row = Math.round(centerCoord.row);
 
-        this.minzoom = this.maxzoom = centerCoord.zoom;
+        // TODO this is making tests fail... what is it for?
+        // this.minzoom = this.maxzoom = centerCoord.zoom;
         this.coord = new TileCoord(centerCoord.zoom, centerCoord.column, centerCoord.row);
         this._tileCoords = cornerZ0Coords.map((coord) => {
             const zoomedCoord = coord.zoomTo(centerCoord.zoom);
@@ -125,7 +124,7 @@ class ImageSource extends Evented {
                 Math.round((zoomedCoord.row - centerCoord.row) * EXTENT));
         });
 
-        this.fire('source.update');
+        this.fire('data', {dataType:'source', update: true});
         return this;
     }
 

diff --git a/src/source/raster_tile_source.js b/src/source/raster_tile_source.js
@@ -32,8 +32,7 @@ class RasterTileSource extends Evented {
                 return this.fire('error', err);
             }
             util.extend(this, tileJSON);
-            this.fire('source.load');
-            this.fire('data', {dataType: 'source'});
+            this.fire('data', {dataType: 'source', metadata: true});
         });
     }
 

diff --git a/src/source/source_cache.js b/src/source/source_cache.js
@@ -27,23 +27,26 @@ class SourceCache extends Evented {
         this.id = id;
         this.dispatcher = dispatcher;
 
-        this.on('source.load', function() {
-            this._sourceLoaded = true;
-        });
-
-        this.on('error', function() {
-            this._sourceErrored = true;
-        });
-
-        this.on('source.update', function(event) {
-            if (this._sourceLoaded) {
+        this.on('data', function(e) {
+            // this._sourceLoaded signifies that the TileJSON is loaded if applicable
+            // if the source type does not come with a TileJSON, the flag signifies the
+            // source data has loaded (i.e geojson has been tiled on the worker and is ready)
+            if (e.dataType === 'source' && e.metadata) this._sourceLoaded = true;
+
+            // for sources with mutable data, this event fires when the underlying data
+            // to a source is changed. (i.e. GeoJSONSource#setData and ImageSource#serCoordinates)
+            if (this._sourceLoaded && e.dataType==="source" && e.update) {
                 this.reload();
                 if (this.transform) {
                     this.update(this.transform);
                 }
             }
         });
 
+        this.on('error', function() {
+            this._sourceErrored = true;
+        });
+
         this._source = Source.create(id, options, dispatcher, this);
 
         this._tiles = {};
@@ -169,8 +172,7 @@ class SourceCache extends Evented {
         tile.timeAdded = new Date().getTime();
         if (previousState === 'expired') tile.refreshedUponExpiration = true;
         this._setTileReloadTimer(id, tile);
-        this._source.fire('data', {tile: tile, coord: tile.coord, dataType: 'tile'});
-        if (this.loaded()) this.fire('data', {dataType: 'source'});
+        this._source.fire('data', {dataType: 'source', tile: tile, coord: tile.coord});
 
         // HACK this is necessary to fix https://github.com/mapbox/mapbox-gl-js/issues/2986
         if (this.map) this.map.painter.tileExtentVAO.vao = null;
@@ -433,7 +435,7 @@ class SourceCache extends Evented {
 
         tile.uses++;
         this._tiles[coord.id] = tile;
-        if (!cached) this._source.fire('dataloading', {tile: tile, coord: tile.coord, dataType: 'tile'});
+        if (!cached) this._source.fire('dataloading', {tile: tile, coord: tile.coord, dataType: 'source'});
 
         return tile;
     }
@@ -475,6 +477,7 @@ class SourceCache extends Evented {
             clearTimeout(this._timers[id]);
             this._timers[id] = undefined;
         }
+        // TODO should we keep this??
         this._source.fire('data', { tile: tile, coord: tile.coord, dataType: 'tile' });
 
         if (tile.uses > 0)

diff --git a/src/source/vector_tile_source.js b/src/source/vector_tile_source.js
@@ -39,8 +39,7 @@ class VectorTileSource extends Evented {
                 return;
             }
             util.extend(this, tileJSON);
-            this.fire('source.load');
-            this.fire('source.update');
+            this.fire('data', {dataType: 'source', metadata: true});
         });
     }
 

diff --git a/src/style/style.js b/src/style/style.js
@@ -119,7 +119,7 @@ class Style extends Evented {
         }
 
         this.on('data', (event) => {
-            if (event.dataType === 'source') {
+            if (event.dataType === 'source' && event.metadata) {
                 const source = this.sourceCaches[event.sourceId].getSource();
                 if (source && source.vectorLayerIds) {
                     for (const layerId in this._layers) {

diff --git a/src/ui/control/attribution_control.js b/src/ui/control/attribution_control.js
@@ -44,7 +44,7 @@ class AttributionControl {
         this._updateAttributions();
         this._updateEditLink();
 
-        this._map.on('source.load', this._updateData);
+        this._map.on('sourcedata', this._updateData);
         this._map.on('moveend', this._updateEditLink);
 
         if (compact === undefined) {
@@ -58,7 +58,7 @@ class AttributionControl {
     onRemove() {
         this._container.parentNode.removeChild(this._container);
 
-        this._map.off('source.load', this._updateData);
+        this._map.off('sourcedata', this._updateData);
         this._map.off('moveend', this._updateEditLink);
         this._map.off('resize', this._updateCompact);
 
@@ -74,14 +74,14 @@ class AttributionControl {
         }
     }
 
-    _updateData(event) {
-        this._updateAttributions();
+    _updateData(e) {
+        this._updateAttributions(e);
         this._updateEditLink();
     }
 
-    _updateAttributions() {
+    _updateAttributions(e) {
         if (!this._map.style) return;
-
+        if (e && !e.metadata) return;
         let attributions = [];
 
         const sourceCaches = this._map.style.sourceCaches;

diff --git a/src/ui/control/logo_control.js b/src/ui/control/logo_control.js
@@ -22,22 +22,22 @@ class LogoControl {
         this._map = map;
         this._container = DOM.create('div', 'mapboxgl-ctrl');
 
-        this._map.on('source.load', this._updateLogo);
+        this._map.on('sourcedata', this._updateLogo);
         this._updateLogo();
         return this._container;
     }
 
     onRemove() {
         this._container.parentNode.removeChild(this._container);
-        this._map.off('source.load', this._updateLogo);
+        this._map.off('sourcedata', this._updateLogo);
     }
 
     getDefaultPosition() {
         return 'bottom-left';
     }
 
-    _updateLogo() {
-        if (this._logoRequired()) {
+    _updateLogo(e) {
+        if (e && e.metadata && this._logoRequired()) {
             const anchor = DOM.create('a', 'mapboxgl-ctrl-logo');
             anchor.target = "_blank";
             anchor.href = "https://www.mapbox.com/";

diff --git a/test/unit/source/canvas_source.test.js b/test/unit/source/canvas_source.test.js
@@ -20,7 +20,6 @@ function createSource(options) {
     }, options);
 
     const source = new CanvasSource('id', options, { send: function() {} }, options.eventedParent);
-
     source.canvas = c;
 
     return source;
@@ -47,13 +46,15 @@ test('CanvasSource', (t) => {
     t.test('constructor', (t) => {
         const source = createSource();
 
-        source.on('source.load', () => {
-            t.equal(source.minzoom, 0);
-            t.equal(source.maxzoom, 22);
-            t.equal(source.tileSize, 512);
-            t.equal(source.animate, true);
-            t.equal(typeof source.play, 'function');
-            t.end();
+        source.on('data', (e) => {
+            if (e.metadata && e.dataType === 'source') {
+                t.equal(source.minzoom, 0);
+                t.equal(source.maxzoom, 22);
+                t.equal(source.tileSize, 512);
+                t.equal(source.animate, true);
+                t.equal(typeof source.play, 'function');
+                t.end();
+            }
         });
 
         source.onAdd(new StubMap());
@@ -82,9 +83,11 @@ test('CanvasSource', (t) => {
             t.end();
         });
 
-        source.on('source.load', () => {
-            t.ok(true, 'fires load event without rerendering');
-            t.end();
+        source.on('data', (e) => {
+            if (e.metadata && e.dataType === 'source') {
+                t.ok(true, 'fires load event without rerendering');
+                t.end();
+            }
         });
 
         source.onAdd(map);

diff --git a/test/unit/source/geojson_source.test.js b/test/unit/source/geojson_source.test.js
@@ -142,7 +142,7 @@ test('GeoJSONSource#update', (t) => {
         }, mockDispatcher).load();
     });
 
-    t.test('fires "source.load"', (t) => {
+    t.test('fires event when metadata loads', (t) => {
         const mockDispatcher = {
             send: function(message, args, callback) {
                 setTimeout(callback, 0);
@@ -151,8 +151,8 @@ test('GeoJSONSource#update', (t) => {
 
         const source = new GeoJSONSource('id', {data: {}}, mockDispatcher);
 
-        source.on('source.load', () => {
-            t.end();
+        source.on('data', (e) => {
+            if (e.metadata) t.end();
         });
 
         source.load();
@@ -191,9 +191,11 @@ test('GeoJSONSource#update', (t) => {
             transform: {}
         };
 
-        source.on('source.load', () => {
-            source.setData({});
-            source.loadTile(new Tile(new TileCoord(0, 0, 0), 512), () => {});
+        source.on('data', (e) => {
+            if (e.metadata) {
+                source.setData({});
+                source.loadTile(new Tile(new TileCoord(0, 0, 0), 512), () => {});
+            }
         });
 
         source.load();