Return subscription for on method (#5080)

* Return subscription for "on" method * Fix documentation generatioan, update changelog
maplibre · Nov 20, 2024 · c46a804 · c46a804
1 parent 611aad3
commit c46a804
Show file tree

Hide file tree

Showing 17 changed files with 960 additions and 952 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,6 +1,8 @@
 ## main
 
 ### ✨ Features and improvements
+
+- ⚠️ Change the return type of `on` method to return a `Subscription` to allow for easy unsubscribe ([#5080](https://github.com/maplibre/maplibre-gl-js/pull/5080))
 - _...Add new stuff here..._
 
 ### 🐞 Bug fixes

diff --git a/src/geo/edge_insets.ts b/src/geo/edge_insets.ts
@@ -1,6 +1,6 @@
 import {interpolates} from '@maplibre/maplibre-gl-style-spec';
 import Point from '@mapbox/point-geometry';
-import {clamp} from '../util/util';
+import {clamp, Complete, RequireAtLeastOne} from '../util/util';
 
 /**
  * An `EdgeInset` object represents screen space padding applied to the edges of the viewport.
@@ -92,7 +92,7 @@ export class EdgeInsets {
      *
      * @returns state as json
      */
-    toJSON(): PaddingOptions {
+    toJSON(): Complete<PaddingOptions> {
         return {
             top: this.top,
             bottom: this.bottom,
@@ -126,7 +126,7 @@ export class EdgeInsets {
  * @see [Fit to the bounds of a LineString](https://maplibre.org/maplibre-gl-js/docs/examples/zoomto-linestring/)
  * @see [Fit a map to a bounding box](https://maplibre.org/maplibre-gl-js/docs/examples/fitbounds/)
  */
-export type PaddingOptions = {
+export type PaddingOptions = RequireAtLeastOne<{
     /**
      * Padding in pixels from the top of the map canvas.
      */
@@ -143,4 +143,4 @@ export type PaddingOptions = {
      * Padding in pixels from the right of the map canvas.
      */
     left: number;
-};
+}>;
diff --git a/src/geo/projection/globe_transform.ts b/src/geo/projection/globe_transform.ts
@@ -835,8 +835,8 @@ export class GlobeTransform implements ITransform {
         };
     }
 
-    calculateCenterFromCameraLngLatAlt(ll: LngLat, alt: number, bearing?: number, pitch?: number): {center: LngLat; elevation: number; zoom: number} {
-        return this._mercatorTransform.calculateCenterFromCameraLngLatAlt(ll, alt, bearing, pitch);
+    calculateCenterFromCameraLngLatAlt(lngLat: LngLatLike, alt: number, bearing?: number, pitch?: number): {center: LngLat; elevation: number; zoom: number} {
+        return this._mercatorTransform.calculateCenterFromCameraLngLatAlt(lngLat, alt, bearing, pitch);
     }
 
     /**

diff --git a/src/geo/projection/mercator_transform.ts b/src/geo/projection/mercator_transform.ts
@@ -533,7 +533,7 @@ export class MercatorTransform implements ITransform {
         return result;
     }
 
-    calculateCenterFromCameraLngLatAlt(lnglat: LngLat, alt: number, bearing?: number, pitch?: number): {center: LngLat; elevation: number; zoom: number} {
+    calculateCenterFromCameraLngLatAlt(lnglat: LngLatLike, alt: number, bearing?: number, pitch?: number): {center: LngLat; elevation: number; zoom: number} {
         const cameraBearing = bearing !== undefined ? bearing : this.bearing;
         const cameraPitch = pitch = pitch !== undefined ? pitch : this.pitch;
 

diff --git a/src/geo/transform_interface.ts b/src/geo/transform_interface.ts
@@ -366,7 +366,7 @@ export interface IReadonlyTransform extends ITransformGetters {
      * @param bearing - bearing of the camera, in degrees
      * @param pitch - pitch angle of the camera, in degrees
      */
-    calculateCenterFromCameraLngLatAlt(lngLat: LngLat, alt: number, bearing?: number, pitch?: number): {center: LngLat; elevation: number; zoom: number};
+    calculateCenterFromCameraLngLatAlt(lngLat: LngLatLike, alt: number, bearing?: number, pitch?: number): {center: LngLat; elevation: number; zoom: number};
 
     getRayDirectionFromPixel(p: Point): vec3;
 

diff --git a/src/source/source_cache.test.ts b/src/source/source_cache.test.ts
@@ -117,7 +117,8 @@ describe('SourceCache#addTile', () => {
 
     test('adds tile when uncached', () => new Promise<void>(done => {
         const tileID = new OverscaledTileID(0, 0, 0, 0, 0);
-        const sourceCache = createSourceCache({}).on('dataloading', (data) => {
+        const sourceCache = createSourceCache({});
+        sourceCache.on('dataloading', (data) => {
             expect(data.tile.tileID).toEqual(tileID);
             expect(data.tile.uses).toBe(1);
             done();
@@ -387,43 +388,47 @@ describe('SourceCache#removeTile', () => {
 
 describe('SourceCache / Source lifecycle', () => {
     test('does not fire load or change before source load event', () => new Promise<void>((done) => {
-        const sourceCache = createSourceCache({noLoad: true})
-            .on('data', () => { throw new Error('test failed: data event fired'); });
+        const sourceCache = createSourceCache({noLoad: true});
+        sourceCache.on('data', () => { throw new Error('test failed: data event fired'); });
         sourceCache.onAdd(undefined);
         setTimeout(() => done(), 1);
     }));
 
     test('forward load event', () => new Promise<void>(done => {
-        const sourceCache = createSourceCache({}).on('data', (e) => {
+        const sourceCache = createSourceCache({});
+        sourceCache.on('data', (e) => {
             if (e.sourceDataType === 'metadata') done();
         });
         sourceCache.onAdd(undefined);
     }));
 
     test('forward change event', () => new Promise<void>(done => {
-        const sourceCache = createSourceCache().on('data', (e) => {
+        const sourceCache = createSourceCache();
+        sourceCache.on('data', (e) => {
             if (e.sourceDataType === 'metadata') done();
         });
         sourceCache.onAdd(undefined);
         sourceCache.getSource().fire(new Event('data'));
     }));
 
     test('forward error event', () => new Promise<void>(done => {
-        const sourceCache = createSourceCache({error: 'Error loading source'}).on('error', (err) => {
+        const sourceCache = createSourceCache({error: 'Error loading source'});
+        sourceCache.on('error', (err) => {
             expect(err.error).toBe('Error loading source');
             done();
         });
         sourceCache.onAdd(undefined);
     }));
 
     test('suppress 404 errors', () => {
-        const sourceCache = createSourceCache({status: 404, message: 'Not found'})
-            .on('error', () => { throw new Error('test failed: error event fired'); });
+        const sourceCache = createSourceCache({status: 404, message: 'Not found'});
+        sourceCache.on('error', () => { throw new Error('test failed: error event fired'); });
         sourceCache.onAdd(undefined);
     });
 
     test('loaded() true after source error', () => new Promise<void>(done => {
-        const sourceCache = createSourceCache({error: 'Error loading source'}).on('error', () => {
+        const sourceCache = createSourceCache({error: 'Error loading source'});
+        sourceCache.on('error', () => {
             expect(sourceCache.loaded()).toBeTruthy();
             done();
         });
@@ -442,7 +447,8 @@ describe('SourceCache / Source lifecycle', () => {
             if (e.dataType === 'source' && e.sourceDataType === 'metadata') {
                 sourceCache.update(transform);
             }
-        }).on('error', () => {
+        });
+        sourceCache.on('error', () => {
             expect(sourceCache.loaded()).toBeTruthy();
             done();
         });
@@ -451,7 +457,8 @@ describe('SourceCache / Source lifecycle', () => {
     }));
 
     test('loaded() false after source begins loading following error', () => new Promise<void>(done => {
-        const sourceCache = createSourceCache({error: 'Error loading source'}).on('error', () => {
+        const sourceCache = createSourceCache({error: 'Error loading source'});
+        sourceCache.on('error', () => {
             sourceCache.on('dataloading', () => {
                 expect(sourceCache.loaded()).toBeFalsy();
                 done();
@@ -469,7 +476,8 @@ describe('SourceCache / Source lifecycle', () => {
             loaded() {
                 return false;
             }
-        }).on('error', () => {
+        });
+        sourceCache.on('error', () => {
             expect(sourceCache.loaded()).toBeFalsy();
             done();
         });

diff --git a/src/style/style.test.ts b/src/style/style.test.ts
@@ -2447,10 +2447,10 @@ describe('Style#query*Features', () => {
 
         onError = jest.fn();
 
-        style.on('error', onError)
-            .on('style.load', () => {
-                callback();
-            });
+        style.on('error', onError);
+        style.on('style.load', () => {
+            callback();
+        });
     }));
 
     test('querySourceFeatures emits an error on incorrect filter', () => {