Fix Layer.addTo

CHANGELOG.md

@@ -13,6 +13,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
 - Add `enable`/`disable` methods and `isEnabled` property to `Interactivity`
 - Enable user-defined buckets in `viewportHistogram`
 - Add and improve examples (legends, widgets, labeling and basemaps)
+- Add helpers to validate browser support (`isBrowserSupported` & `unsupportedBrowserReasons`)
 
 ### Fixed
 - Fix `linear` error when evaluating a date property in a feature
@@ -26,6 +27,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
 - Fix unwanted `Interactivity` events while using dragPan (and performance improvement)
 - Fix `linear` applied to cluster aggregations without explicit limits
 - Fix `MVT` category property returning a numeric value
+- Fix `Layer.addTo` for MGL v0.52
 
 ### Changed
 - Change examples to use Mapbox GL version 0.52

src/Layer.js

@@ -157,21 +157,29 @@ export default class Layer {
      * @api
      */
     addTo (map, beforeLayerID) {
-        const STYLE_ERROR_REGEX = /Style is not done loading/;
-
-        const addTheLayer = () => { map.addLayer(this, beforeLayerID); };
+        // Manage errors, whether they are an Evented Error or a common Error
         try {
-            addTheLayer();
-            // Note: map.isStyleLoaded() has been tested here without success
+            map.once('error', (data) => {
+                console.warn(data.error.message);
+                this._waitForMapToLoad(map, beforeLayerID);
+            });
+            map.addLayer(this, beforeLayerID);
         } catch (error) {
-            if (!STYLE_ERROR_REGEX.test(error)) {
+            const STYLE_ERROR_REGEX = /Style is not done loading/;
+            const NO_STYLE_AT_ALL = /Cannot read property 'addLayer' of undefined/;
+            if (!(STYLE_ERROR_REGEX.test(error) || NO_STYLE_AT_ALL.test(error))) {
                 throw new CartoRuntimeError(`Error adding layer to map: ${error}`);
             }
-
-            map.on('load', addTheLayer);
+            this._waitForMapToLoad(map, beforeLayerID);
         }
     }
 
+    _waitForMapToLoad (map, beforeLayerID) {
+        map.on('load', () => {
+            map.addLayer(this, beforeLayerID);
+        });
+    }
+
     /**
      * Remove this layer from the map. It should be called after the layer is loaded. Otherwise, it will not delete the layer.
      *

test/CartoMap.js

@@ -42,6 +42,10 @@ class CartoMap { // eslint-disable-line no-unused-vars
         }
     }
 
+    once () {
+
+    }
+
     addLayer (layer, beforeLayerID) {
         if (!this._layers.has(layer)) {
             this._layers.add(layer);

test/integration/render/scenarios/regression/layer-addTo/scenario.js

@@ -0,0 +1,30 @@
+const map = new mapboxgl.Map({
+    container: 'map',
+    center: [0, 0],
+    style: { version: 8, sources: {}, layers: [] },
+    zoom: 0
+});
+
+const source = new carto.source.GeoJSON(sources['points']);
+const viz = new carto.Viz();
+const layer = new carto.Layer('layer', source, viz);
+
+layer.addTo(map, 'background');
+
+// Setting map's style after trying to add the layer...
+const style = {
+    version: 8,
+    sources: {},
+    layers: [{
+        id: 'background',
+        type: 'background',
+        paint: {
+            'background-color': 'black'
+        }
+    }]
+};
+map.setStyle(style);
+
+layer.on('loaded', () => {
+    window.loaded = true;
+});

test/integration/user/test/layer-waits-map.test.js

@@ -0,0 +1,41 @@
+import carto from '../../../../src/';
+import * as util from '../../util';
+import mapboxgl from 'mapbox-gl';
+
+const featureData = {
+    type: 'Feature',
+    geometry: {
+        type: 'Point',
+        coordinates: [0, 0]
+    },
+    properties: {}
+};
+
+describe('when map is not ready', () => {
+    let div, map;
+
+    it('Layer should wait for it load', (done) => {
+        const name = 'map';
+        div = util.createMapDivHolder(name);
+        map = new mapboxgl.Map({
+            container: name,
+            center: [0, 0],
+            zoom: 0
+        });
+
+        const source = new carto.source.GeoJSON(featureData);
+        const layer = new carto.Layer('layer', source, new carto.Viz());
+
+        const waitForMapToLoad = spyOn(layer, '_waitForMapToLoad').and.callThrough();
+        layer.addTo(map, 'watername_ocean'); // << adding before setting basemap
+        map.setStyle(carto.basemaps.voyager);
+
+        expect(waitForMapToLoad).toHaveBeenCalled();
+        done();
+    });
+
+    afterEach(() => {
+        map.remove();
+        document.body.removeChild(div);
+    });
+});

test/integration/util.js

@@ -4,6 +4,19 @@ import { projectToWebMercator, WM_2R } from '../../src/utils/util';
 const mapSize = 600;
 
 export function createMap (name, size) {
+    const div = createMapDivHolder(name, size);
+
+    const map = new mapboxgl.Map({
+        container: name,
+        style: { version: 8, sources: {}, layers: [] },
+        center: [0, 0],
+        zoom: 0
+    });
+
+    return { div, map };
+}
+
+export function createMapDivHolder (name, size) {
     size = size || mapSize;
 
     const div = document.createElement('div');
@@ -16,14 +29,7 @@ export function createMap (name, size) {
     document.body.style.padding = '0';
     document.body.appendChild(div);
 
-    const map = new mapboxgl.Map({
-        container: name,
-        style: { version: 8, sources: {}, layers: [] },
-        center: [0, 0],
-        zoom: 0
-    });
-
-    return { div, map };
+    return div;
 }
 
 function _mouseParamsFromCoords (coordinates) {

test/unit/layer.test.js

@@ -146,7 +146,8 @@ describe('api/layer', () => {
         it('should call onMapLoaded when the map is loaded', () => {
             const layer = new Layer('layer0', source, viz);
             const mapMock = {
-                addLayer: jasmine.createSpy('addLayer')
+                addLayer: jasmine.createSpy('addLayer'),
+                once: () => { }
             };
             layer.addTo(mapMock, 'beforeLayer');
             expect(mapMock.addLayer).toHaveBeenCalledWith(layer, 'beforeLayer');
@@ -159,7 +160,8 @@ describe('api/layer', () => {
             layer.onAdd = (map) => { layer.map = map; };
             const mapMock = {
                 addLayer: (layer) => { layer.onAdd(mapMock); },
-                removeLayer: jasmine.createSpy('removeLayer')
+                removeLayer: jasmine.createSpy('removeLayer'),
+                once: () => { }
             };
             layer.addTo(mapMock);
             layer.remove();