Issue 915: Stop/start Location manager based on listeners registered.

__tests__/components/UserLocation.test.js

@@ -118,11 +118,12 @@ describe('UserLocation', () => {
       const {UNSAFE_queryByType} = render(
         <UserLocation onPress={onPressCallback} />,
       );
-
-      const shapeSource = UNSAFE_queryByType(ShapeSource);
-      fireEvent(shapeSource, 'onPress');
-      fireEvent(shapeSource, 'onPress');
-      expect(onPressCallback).toHaveBeenCalledTimes(2);
+      setTimeout(() => {
+        const shapeSource = UNSAFE_queryByType(ShapeSource);
+        fireEvent(shapeSource, 'onPress');
+        fireEvent(shapeSource, 'onPress');
+        expect(onPressCallback).toHaveBeenCalledTimes(2);
+      });
     });
 
     test('correctly unmounts', async () => {
@@ -171,20 +172,20 @@ describe('UserLocation', () => {
       };
 
       expect(ul.state).toStrictEqual(initialState);
-      expect(ul.locationManagerRunning).toStrictEqual(false);
+      expect(ul._isLocationManagerRequired).toStrictEqual(false);
     });
 
-    // TODO: replace object { running: boolean } argument with simple boolean
+    // TODO: replace object { required: boolean } argument with simple boolean
     describe('#setLocationManager', () => {
-      test('called with "running" true', async () => {
+      test('called with "required" true', async () => {
         const lastKnownLocation = [4.1036916, 51.5462244];
         const heading = 251.5358428955078;
 
-        expect(ul.locationManagerRunning).toStrictEqual(false);
+        expect(ul._isLocationManagerRequired).toStrictEqual(false);
 
-        await ul.setLocationManager({running: true});
+        await ul.setLocationManager({required: true});
 
-        expect(ul.locationManagerRunning).toStrictEqual(true);
+        expect(ul._isLocationManagerRequired).toStrictEqual(true);
         expect(locationManager.start).toHaveBeenCalledTimes(1);
         expect(locationManager.getLastKnownLocation).toHaveBeenCalledTimes(1);
         expect(ul.setState).toHaveBeenCalledTimes(1);
@@ -195,48 +196,49 @@ describe('UserLocation', () => {
         expect(locationManager.stop).not.toHaveBeenCalled();
       });
 
-      test('called with "running" false', async () => {
+      test('called with "required" false', async () => {
         // start
-        expect(ul.locationManagerRunning).toStrictEqual(false);
-        await ul.setLocationManager({running: true});
-        expect(ul.locationManagerRunning).toStrictEqual(true);
+        expect(ul._isLocationManagerRequired).toStrictEqual(false);
+        await ul.setLocationManager({required: true});
+        expect(ul._isLocationManagerRequired).toStrictEqual(true);
 
         // stop
-        await ul.setLocationManager({running: false});
+        await ul.setLocationManager({required: false});
 
-        expect(ul.locationManagerRunning).toStrictEqual(false);
+        expect(ul._isLocationManagerRequired).toStrictEqual(false);
         // only once from start
         expect(locationManager.start).toHaveBeenCalledTimes(1);
-        expect(locationManager.stop).toHaveBeenCalledTimes(1);
+        // stop should not be called
+        expect(locationManager.stop).not.toHaveBeenCalled();
       });
     });
 
-    describe('#needsLocationManagerRunning', () => {
+    describe('#isLocationManagerRequired', () => {
       test('returns true correctly', () => {
         // default props "onUpdate: undefined, visible: true"
-        expect(ul.needsLocationManagerRunning()).toStrictEqual(true);
+        expect(ul.isLocationManagerRequired()).toStrictEqual(true);
 
         ul.props = {
           onUpdate: () => {},
           visible: true,
         };
 
-        expect(ul.needsLocationManagerRunning()).toStrictEqual(true);
+        expect(ul.isLocationManagerRequired()).toStrictEqual(true);
 
         ul.props = {
           onUpdate: () => {},
           visible: false,
         };
 
-        expect(ul.needsLocationManagerRunning()).toStrictEqual(true);
+        expect(ul.isLocationManagerRequired()).toStrictEqual(true);
       });
 
       test('returns false correctly', () => {
         ul.props = {
           visible: false,
         };
 
-        expect(ul.needsLocationManagerRunning()).toStrictEqual(false);
+        expect(ul.isLocationManagerRequired()).toStrictEqual(false);
       });
     });
 

__tests__/modules/location/locationManager.test.js

@@ -69,6 +69,7 @@ describe('LocationManager', () => {
 
     describe('#addListener', () => {
       const myListener = jest.fn();
+      MapboxGL.LocationCallbackName = {Update: 'MapboxUserLocationUpdate'};
 
       afterEach(() => {
         locationManager._listeners = [];
@@ -88,17 +89,18 @@ describe('LocationManager', () => {
         expect(myListener).not.toHaveBeenCalled();
       });
 
-      test('calls listener with "lastKnownLocation"', () => {
+      test('listener is not called just after being added', () => {
         locationManager._lastKnownLocation = location;
 
         locationManager.addListener(myListener);
         expect(locationManager._listeners).toStrictEqual([myListener]);
-        expect(myListener).toHaveBeenCalledWith(location);
-        expect(myListener).toHaveBeenCalledTimes(1);
+        expect(myListener).not.toHaveBeenCalled();
       });
     });
 
     describe('#removeListener', () => {
+      MapboxGLLocationManager.stop = jest.fn();
+
       test('removes selected listener', () => {
         // just two different functions
         const listenerA = jest.fn(() => 'listenerA');
@@ -142,7 +144,7 @@ describe('LocationManager', () => {
       jest.spyOn(MapboxGLLocationManager, 'start');
       jest.spyOn(LocationModuleEventEmitter, 'addListener');
 
-      afterEach(() => {
+      beforeEach(() => {
         locationManager._isListening = false;
       });
 

docs/UserLocation.md

@@ -16,19 +16,19 @@
 | children | `any` | `none` | `false` | Custom location icon of type mapbox-gl-native components |
 
 ### methods
-#### setLocationManager({running})
+#### setLocationManager({required})
 
-Whether to start or stop the locationManager<br/><br/>Notice, that locationManager will start automatically when<br/>either `onUpdate` or `visible` are set
+Whether to start or stop listening to the locationManager<br/><br/>Notice, that listening will start automatically when<br/>either `onUpdate` or `visible` are set
 
 ##### arguments
 | Name | Type | Required | Description  |
 | ---- | :--: | :------: | :----------: |
-| `{running}` | `n/a` | `Yes` | undefined |
+| `{required}` | `n/a` | `Yes` | undefined |
 
 
-#### needsLocationManagerRunning()
+#### isLocationManagerRequired()
 
-If locationManager should be running
+If locationManager is required.
 
 ##### arguments
 | Name | Type | Required | Description  |

docs/docs.json

@@ -5164,13 +5164,13 @@
     "methods": [
       {
         "name": "setLocationManager",
-        "docblock": "Whether to start or stop the locationManager\n\nNotice, that locationManager will start automatically when\neither `onUpdate` or `visible` are set\n\n@async\n@param {Object} running - Object with key `running` and `boolean` value\n@return {Promise<void>}",
+        "docblock": "Whether to start or stop listening to the locationManager\n\nNotice, that listening will start automatically when\neither `onUpdate` or `visible` are set\n\n@async\n@param {Object} required - Object with key `required` and `boolean` value\n@return {Promise<void>}",
         "modifiers": [
           "async"
         ],
         "params": [
           {
-            "name": "{running}"
+            "name": "{required}"
           }
         ],
         "returns": {
@@ -5184,12 +5184,12 @@
             ]
           }
         },
-        "description": "Whether to start or stop the locationManager\n\nNotice, that locationManager will start automatically when\neither `onUpdate` or `visible` are set",
+        "description": "Whether to start or stop listening to the locationManager\n\nNotice, that listening will start automatically when\neither `onUpdate` or `visible` are set",
         "examples": []
       },
       {
-        "name": "needsLocationManagerRunning",
-        "docblock": "If locationManager should be running\n\n@return {boolean}",
+        "name": "isLocationManagerRequired",
+        "docblock": "If locationManager is required.\n\n@return {boolean}",
         "modifiers": [],
         "params": [],
         "returns": {
@@ -5198,7 +5198,7 @@
             "name": "boolean"
           }
         },
-        "description": "If locationManager should be running",
+        "description": "If locationManager is required.",
         "examples": []
       }
     ],

javascript/components/UserLocation.js

@@ -137,15 +137,13 @@ class UserLocation extends React.Component {
   // after component unmount
   _isMounted = null;
 
-  locationManagerRunning = false;
+  _isLocationManagerRequired = false;
 
   async componentDidMount() {
     this._isMounted = true;
 
-    locationManager.addListener(this._onLocationUpdate);
-
     await this.setLocationManager({
-      running: this.needsLocationManagerRunning(),
+      required: this.isLocationManagerRequired(),
     });
 
     if (this.renderMode === UserLocation.RenderMode.Native) {
@@ -157,7 +155,7 @@ class UserLocation extends React.Component {
 
   async componentDidUpdate(prevProps) {
     await this.setLocationManager({
-      running: this.needsLocationManagerRunning(),
+      required: this.isLocationManagerRequired(),
     });
 
     if (this.props.minDisplacement !== prevProps.minDisplacement) {
@@ -167,41 +165,39 @@ class UserLocation extends React.Component {
 
   async componentWillUnmount() {
     this._isMounted = false;
-    locationManager.removeListener(this._onLocationUpdate);
-    await this.setLocationManager({running: false});
+    await this.setLocationManager({required: false});
   }
 
   /**
-   * Whether to start or stop the locationManager
+   * Whether to start or stop listening to the locationManager
    *
-   * Notice, that locationManager will start automatically when
+   * Notice, that listening will start automatically when
    * either `onUpdate` or `visible` are set
    *
    * @async
-   * @param {Object} running - Object with key `running` and `boolean` value
+   * @param {Object} required - Object with key `required` and `boolean` value
    * @return {Promise<void>}
    */
-  async setLocationManager({running}) {
-    if (this.locationManagerRunning !== running) {
-      this.locationManagerRunning = running;
-      if (running) {
-        locationManager.start();
-
+  async setLocationManager({required}) {
+    if (this._isLocationManagerRequired !== required) {
+      this._isLocationManagerRequired = required;
+      if (required) {
+        locationManager.addListener(this._onLocationUpdate);
         const location = await locationManager.getLastKnownLocation();
         this._onLocationUpdate(location);
       } else {
-        locationManager.stop();
+        locationManager.removeListener(this._onLocationUpdate);
       }
     }
   }
 
   /**
    *
-   * If locationManager should be running
+   * If locationManager is required.
    *
    * @return {boolean}
    */
-  needsLocationManagerRunning() {
+  isLocationManagerRequired() {
     if (this.props.renderMode === UserLocation.RenderMode.Native) {
       return false;
     }

javascript/modules/location/locationManager.js

@@ -38,21 +38,24 @@ class LocationManager {
   }
 
   addListener(listener) {
+    if (!this._isListening) {
+      this.start();
+    }
     if (!this._listeners.includes(listener)) {
       this._listeners.push(listener);
-
-      if (this._lastKnownLocation) {
-        listener(this._lastKnownLocation);
-      }
     }
   }
 
   removeListener(listener) {
     this._listeners = this._listeners.filter((l) => l !== listener);
+    if (this._listeners.length === 0) {
+      this.stop();
+    }
   }
 
   removeAllListeners() {
     this._listeners = [];
+    this.stop();
   }
 
   start(displacement = 0) {