fire sourcedata when source metadata and tiles are loaded

src/source/canvas_source.js

@@ -75,7 +75,6 @@ class CanvasSource extends ImageSource {
         this.load();
         if (this.canvas) {
             if (this.animate) this.play();
-            this.setCoordinates(this.coordinates);
         }
     }
 

src/source/geojson_source.js

@@ -108,8 +108,11 @@ class GeoJSONSource extends Evented {
                 this.fire('error', {error: err});
                 return;
             }
-            this.fire('data', {dataType: 'source'});
-            this.fire('source.load');
+            // `update` is included here to prevent a race condition where `Style#_updateSources` is called
+            // before the TileJSON arrives. this makes sure the tiles needed are loaded once TileJSON arrives
+            // ref: https://github.com/mapbox/mapbox-gl-js/pull/4347#discussion_r104418088
+            this.fire('data', {dataType: 'source', sourceDataType: 'metadata'});
+            this.fire('data', {dataType: 'source', sourceDataType: 'update'});
         });
     }
 
@@ -126,13 +129,12 @@ class GeoJSONSource extends Evented {
      */
     setData(data) {
         this._data = data;
-
         this.fire('dataloading', {dataType: 'source'});
         this._updateWorkerData((err) => {
             if (err) {
                 return this.fire('error', { error: err });
             }
-            this.fire('data', {dataType: 'source'});
+            this.fire('data', {dataType: 'source', sourceDataType: 'update'});
         });
 
         return this;

src/source/image_source.js

@@ -75,10 +75,9 @@ class ImageSource extends Evented {
     }
 
     _finishLoading() {
-        this.fire('source.load');
-
         if (this.map) {
             this.setCoordinates(this.coordinates);
+            this.fire('data', {dataType: 'source', sourceDataType: 'metadata'});
         }
     }
 
@@ -124,7 +123,7 @@ class ImageSource extends Evented {
                 Math.round((zoomedCoord.row - centerCoord.row) * EXTENT));
         });
 
-        this.fire('data', {dataType: 'source'});
+        this.fire('data', {dataType:'source', sourceDataType: 'update'});
         return this;
     }
 

src/source/raster_tile_source.js

@@ -32,8 +32,13 @@ class RasterTileSource extends Evented {
                 return this.fire('error', err);
             }
             util.extend(this, tileJSON);
-            this.fire('data', {dataType: 'source'});
-            this.fire('source.load');
+
+            // `update` is included here to prevent a race condition where `Style#_updateSources` is called
+            // before the TileJSON arrives. this makes sure the tiles needed are loaded once TileJSON arrives
+            // ref: https://github.com/mapbox/mapbox-gl-js/pull/4347#discussion_r104418088
+            this.fire('data', {dataType: 'source', sourceDataType: 'metadata'});
+            this.fire('data', {dataType: 'source', sourceDataType: 'update'});
+
         });
     }
 

src/source/source.js

@@ -50,7 +50,7 @@ exports.setType = function (name, type) {
  * @param {string} options.type The source type, matching the value of `name` used in {@link Style#addSourceType}.
  * @param {Dispatcher} dispatcher A {@link Dispatcher} instance, which can be used to send messages to the workers.
  *
- * @fires load to indicate source data has been loaded, so that it's okay to call `loadTile`
+ * @fires data to indicate source data has been loaded, so that it's okay to call `loadTile`
  * @fires change to indicate source data has changed, so that any current caches should be flushed
  * @property {string} id The id for the source.  Must match the id passed to the constructor.
  * @property {number} minzoom

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('data', function(event) {
-            if (this._sourceLoaded && event.dataType === 'source') {
+        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.sourceDataType==='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.sourceDataType==='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 = {};
@@ -110,6 +113,7 @@ class SourceCache extends Evented {
         return this._source.serialize();
     }
 
+
     prepare() {
         if (this._sourceLoaded && this._source.prepare)
             return this._source.prepare();
@@ -169,7 +173,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'});
+        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;
@@ -296,6 +300,7 @@ class SourceCache extends Evented {
      * @private
      */
     update(transform) {
+        this.transform = transform;
         if (!this._sourceLoaded) { return; }
         let i;
         let coord;
@@ -394,8 +399,6 @@ class SourceCache extends Evented {
         for (i = 0; i < remove.length; i++) {
             this.removeTile(+remove[i]);
         }
-
-        this.transform = transform;
     }
 
     /**
@@ -423,8 +426,8 @@ class SourceCache extends Evented {
                 }
             }
         }
-
-        if (!tile) {
+        const cached = Boolean(tile);
+        if (!cached) {
             const zoom = coord.z;
             const overscaling = zoom > this._source.maxzoom ? Math.pow(2, zoom - this._source.maxzoom) : 1;
             tile = new Tile(wrapped, this._source.tileSize * overscaling, this._source.maxzoom);
@@ -433,7 +436,7 @@ class SourceCache extends Evented {
 
         tile.uses++;
         this._tiles[coord.id] = tile;
-        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,7 +478,6 @@ class SourceCache extends Evented {
             clearTimeout(this._timers[id]);
             this._timers[id] = undefined;
         }
-        this._source.fire('data', { tile: tile, coord: tile.coord, dataType: 'tile' });
 
         if (tile.uses > 0)
             return;

src/source/tile.js

@@ -155,7 +155,6 @@ class Tile {
             showCollisionBoxes: source.map.showCollisionBoxes
         }, (_, data) => {
             this.reloadSymbolData(data, source.map.style);
-            source.fire('data', {tile: this, coord: this.coord, dataType: 'tile'});
 
             // HACK this is nescessary to fix https://github.com/mapbox/mapbox-gl-js/issues/2986
             if (source.map) source.map.painter.tileExtentVAO.vao = null;

src/source/vector_tile_source.js

@@ -39,8 +39,7 @@ class VectorTileSource extends Evented {
                 return;
             }
             util.extend(this, tileJSON);
-            this.fire('data', {dataType: 'source'});
-            this.fire('source.load');
+            this.fire('data', {dataType: 'source', sourceDataType: 'metadata'});
         });
     }
 

src/style/style.js

@@ -118,13 +118,15 @@ class Style extends Evented {
             browser.frame(stylesheetLoaded.bind(this, null, stylesheet));
         }
 
-        this.on('source.load', (event) => {
-            const source = this.sourceCaches[event.sourceId].getSource();
-            if (source && source.vectorLayerIds) {
-                for (const layerId in this._layers) {
-                    const layer = this._layers[layerId];
-                    if (layer.source === source.id) {
-                        this._validateLayer(layer);
+        this.on('data', (event) => {
+            if (event.dataType === 'source' && event.sourceDataType === 'metadata') {
+                const source = this.sourceCaches[event.sourceId].getSource();
+                if (source && source.vectorLayerIds) {
+                    for (const layerId in this._layers) {
+                        const layer = this._layers[layerId];
+                        if (layer.source === source.id) {
+                            this._validateLayer(layer);
+                        }
                     }
                 }
             }
@@ -137,7 +139,6 @@ class Style extends Evented {
         if (!layer.sourceLayer) return;
         if (!sourceCache) return;
         const source = sourceCache.getSource();
-
         if (source.type === 'geojson' || (source.vectorLayerIds &&
             source.vectorLayerIds.indexOf(layer.sourceLayer) === -1)) {
             this.fire('error', {

src/ui/control/attribution_control.js

@@ -44,7 +44,7 @@ class AttributionControl {
         this._updateAttributions();
         this._updateEditLink();
 
-        this._map.on('data', 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('data', this._updateData);
+        this._map.off('sourcedata', this._updateData);
         this._map.off('moveend', this._updateEditLink);
         this._map.off('resize', this._updateCompact);
 
@@ -74,16 +74,14 @@ class AttributionControl {
         }
     }
 
-    _updateData(event) {
-        if (event.dataType === 'source') {
-            this._updateAttributions();
-            this._updateEditLink();
-        }
+    _updateData(e) {
+        this._updateAttributions(e);
+        this._updateEditLink();
     }
 
-    _updateAttributions() {
+    _updateAttributions(e) {
         if (!this._map.style) return;
-
+        if (e && e.sourceDataType!=='metadata') return;
         let attributions = [];
 
         const sourceCaches = this._map.style.sourceCaches;

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('data', this._updateLogo);
+        this._map.on('sourcedata', this._updateLogo);
         this._updateLogo();
         return this._container;
     }
 
     onRemove() {
         this._container.parentNode.removeChild(this._container);
-        this._map.off('data', this._updateLogo);
+        this._map.off('sourcedata', this._updateLogo);
     }
 
     getDefaultPosition() {
         return 'bottom-left';
     }
 
-    _updateLogo() {
-        if (this._logoRequired()) {
+    _updateLogo(e) {
+        if (e && e.sourceDataType==='metadata' && this._logoRequired()) {
             const anchor = DOM.create('a', 'mapboxgl-ctrl-logo');
             anchor.target = "_blank";
             anchor.href = "https://www.mapbox.com/";

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();
+        t.equal(source.minzoom, 0);
+        t.equal(source.maxzoom, 22);
+        t.equal(source.tileSize, 512);
+        t.equal(source.animate, true);
+        source.on('data', (e) => {
+            if (e.dataType === 'source' && e.sourceDataType === 'metadata') {
+                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.sourceDataType === 'metadata' && e.dataType === 'source') {
+                t.ok(true, 'fires load event without rerendering');
+                t.end();
+            }
         });
 
         source.onAdd(map);

test/unit/source/geojson_source.test.js

@@ -63,18 +63,15 @@ test('GeoJSONSource#setData', (t) => {
     t.test('fires "data" event', (t) => {
         const source = createSource();
         source.once('data', () => {
-            source.on('data', t.end);
+            source.once('data', t.end);
             source.setData({});
         });
         source.load();
     });
 
     t.test('fires "dataloading" event', (t) => {
         const source = createSource();
-        source.once('data', () => {
-            source.on('dataloading', t.end);
-            source.setData({});
-        });
+        source.on('dataloading', t.end);
         source.load();
     });
 
@@ -142,7 +139,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 +148,8 @@ test('GeoJSONSource#update', (t) => {
 
         const source = new GeoJSONSource('id', {data: {}}, mockDispatcher);
 
-        source.on('source.load', () => {
-            t.end();
+        source.on('data', (e) => {
+            if (e.sourceDataType === 'metadata') t.end();
         });
 
         source.load();
@@ -191,9 +188,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.sourceDataType === 'metadata') {
+                source.setData({});
+                source.loadTile(new Tile(new TileCoord(0, 0, 0), 512), () => {});
+            }
         });
 
         source.load();