fix: rjxs subscription leaks resulting in attempts to create sources …

…multiple times
Wykks · Jun 19, 2020 · beddec1 · beddec1
1 parent bc61b07
commit beddec1
Show file tree

Hide file tree

Showing 8 changed files with 30 additions and 27 deletions.
diff --git a/package.json b/package.json
@@ -1,6 +1,6 @@
 {
   "name": "ngx-mapbox-gl-srcs",
-  "version": "4.6.0",
+  "version": "4.6.1-beta.0",
   "scripts": {
     "ng": "ng",
     "start": "ng serve showcase",

diff --git a/projects/ngx-mapbox-gl/package.json b/projects/ngx-mapbox-gl/package.json
@@ -1,6 +1,6 @@
 {
   "name": "ngx-mapbox-gl",
-  "version": "4.6.0",
+  "version": "4.6.1-beta.0",
   "description": "A Angular binding of mapbox-gl-js",
   "license": "MIT",
   "repository": {

diff --git a/projects/ngx-mapbox-gl/src/lib/map/map.service.ts b/projects/ngx-mapbox-gl/src/lib/map/map.service.ts
@@ -68,8 +68,6 @@ export class MapService {
 
   private mapCreated = new AsyncSubject<void>();
   private mapLoaded = new AsyncSubject<void>();
-  private layerIdsToRemove: string[] = [];
-  private sourceIdsToRemove: string[] = [];
   private markersToRemove: MapboxGl.Marker[] = [];
   private popupsToRemove: MapboxGl.Popup[] = [];
   private imageIdsToRemove: string[] = [];
@@ -253,7 +251,11 @@ export class MapService {
   }
 
   removeLayer(layerId: string) {
-    this.layerIdsToRemove.push(layerId);
+    this.zone.runOutsideAngular(() => {
+      if (this.mapInstance.getLayer(layerId) != null) {
+        this.mapInstance.removeLayer(layerId);
+      }
+    });
   }
 
   addMarker(marker: SetupMarker) {
@@ -403,7 +405,10 @@ export class MapService {
   }
 
   removeSource(sourceId: string) {
-    this.sourceIdsToRemove.push(sourceId);
+    this.zone.runOutsideAngular(() => {
+      this.findLayersBySourceId(sourceId).forEach((layer) => this.mapInstance.removeLayer(layer.id));
+      this.mapInstance.removeSource(sourceId);
+    });
   }
 
   setAllLayerPaintProperty(
@@ -480,8 +485,6 @@ export class MapService {
 
   applyChanges() {
     this.zone.runOutsideAngular(() => {
-      this.removeLayers();
-      this.removeSources();
       this.removeMarkers();
       this.removePopups();
       this.removeImages();
@@ -513,20 +516,6 @@ export class MapService {
     this.subscription.add(subChanges);
   }
 
-  private removeLayers() {
-    for (const layerId of this.layerIdsToRemove) {
-      this.mapInstance.removeLayer(layerId);
-    }
-    this.layerIdsToRemove = [];
-  }
-
-  private removeSources() {
-    for (const sourceId of this.sourceIdsToRemove) {
-      this.mapInstance.removeSource(sourceId);
-    }
-    this.sourceIdsToRemove = [];
-  }
-
   private removeMarkers() {
     for (const marker of this.markersToRemove) {
       marker.remove();
@@ -548,6 +537,15 @@ export class MapService {
     this.imageIdsToRemove = [];
   }
 
+  private findLayersBySourceId(sourceId: string): MapboxGl.Layer[] {
+    const layers = this.mapInstance.getStyle().layers;
+    if (layers == null) {
+      return [];
+    }
+
+    return layers.filter((l) => l.source === sourceId);
+  }
+
   private hookEvents(events: MapEvent) {
     this.mapInstance.on('load', () => {
       this.mapLoaded.next(undefined);

diff --git a/projects/ngx-mapbox-gl/src/lib/source/canvas-source.component.ts b/projects/ngx-mapbox-gl/src/lib/source/canvas-source.component.ts
@@ -24,7 +24,7 @@ export class CanvasSourceComponent implements OnInit, OnDestroy, OnChanges, Canv
   constructor(private MapService: MapService) {}
 
   ngOnInit() {
-    this.MapService.mapLoaded$.subscribe(() => {
+    const sub1 = this.MapService.mapLoaded$.subscribe(() => {
       this.init();
       const sub = fromEvent(<any>this.MapService.mapInstance, 'styledata')
         .pipe(filter(() => !this.MapService.mapInstance.getSource(this.id)))
@@ -33,6 +33,7 @@ export class CanvasSourceComponent implements OnInit, OnDestroy, OnChanges, Canv
         });
       this.sub.add(sub);
     });
+    this.sub.add(sub1);
   }
 
   ngOnChanges(changes: SimpleChanges) {

diff --git a/projects/ngx-mapbox-gl/src/lib/source/geojson/geojson-source.component.ts b/projects/ngx-mapbox-gl/src/lib/source/geojson/geojson-source.component.ts
@@ -49,7 +49,7 @@ export class GeoJSONSourceComponent implements OnInit, OnDestroy, OnChanges, Geo
         features: []
       };
     }
-    this.MapService.mapLoaded$.subscribe(() => {
+    const sub1 = this.MapService.mapLoaded$.subscribe(() => {
       this.init();
       const sub = fromEvent(<any>this.MapService.mapInstance, 'styledata')
         .pipe(filter(() => !this.MapService.mapInstance.getSource(this.id)))
@@ -58,6 +58,7 @@ export class GeoJSONSourceComponent implements OnInit, OnDestroy, OnChanges, Geo
         });
       this.sub.add(sub);
     });
+    this.sub.add(sub1);
   }
 
   ngOnChanges(changes: SimpleChanges) {

diff --git a/projects/ngx-mapbox-gl/src/lib/source/raster-source.component.ts b/projects/ngx-mapbox-gl/src/lib/source/raster-source.component.ts
@@ -29,7 +29,7 @@ export class RasterSourceComponent implements OnInit, OnDestroy, OnChanges, Rast
   constructor(private MapService: MapService) {}
 
   ngOnInit() {
-    this.MapService.mapLoaded$.subscribe(() => {
+    const sub1 = this.MapService.mapLoaded$.subscribe(() => {
       this.init();
       const sub = fromEvent(<any>this.MapService.mapInstance, 'styledata')
         .pipe(filter(() => !this.MapService.mapInstance.getSource(this.id)))
@@ -38,6 +38,7 @@ export class RasterSourceComponent implements OnInit, OnDestroy, OnChanges, Rast
         });
       this.sub.add(sub);
     });
+    this.sub.add(sub1);
   }
 
   ngOnChanges(changes: SimpleChanges) {

diff --git a/projects/ngx-mapbox-gl/src/lib/source/vector-source.component.ts b/projects/ngx-mapbox-gl/src/lib/source/vector-source.component.ts
@@ -27,7 +27,7 @@ export class VectorSourceComponent implements OnInit, OnDestroy, OnChanges, Vect
   constructor(private MapService: MapService) {}
 
   ngOnInit() {
-    this.MapService.mapLoaded$.subscribe(() => {
+    const sub1 = this.MapService.mapLoaded$.subscribe(() => {
       this.init();
       const sub = fromEvent(<any>this.MapService.mapInstance, 'styledata')
         .pipe(filter(() => !this.MapService.mapInstance.getSource(this.id)))
@@ -36,6 +36,7 @@ export class VectorSourceComponent implements OnInit, OnDestroy, OnChanges, Vect
         });
       this.sub.add(sub);
     });
+    this.sub.add(sub1);
   }
 
   ngOnChanges(changes: SimpleChanges) {

diff --git a/projects/ngx-mapbox-gl/src/lib/source/video-source.component.ts b/projects/ngx-mapbox-gl/src/lib/source/video-source.component.ts
@@ -23,7 +23,7 @@ export class VideoSourceComponent implements OnInit, OnDestroy, OnChanges, Video
   constructor(private MapService: MapService) {}
 
   ngOnInit() {
-    this.MapService.mapLoaded$.subscribe(() => {
+    const sub1 = this.MapService.mapLoaded$.subscribe(() => {
       this.init();
       const sub = fromEvent(<any>this.MapService.mapInstance, 'styledata')
         .pipe(filter(() => !this.MapService.mapInstance.getSource(this.id)))
@@ -32,6 +32,7 @@ export class VideoSourceComponent implements OnInit, OnDestroy, OnChanges, Video
         });
       this.sub.add(sub);
     });
+    this.sub.add(sub1);
   }
 
   ngOnChanges(changes: SimpleChanges) {