NativeScript - Geolocation: right way to use getCurrentLocation promise function

Question

I'm writing a simple app which use nativescript-geolocation API. The function getCurrentLocation basically works fine, but when I moved to another file named maps-module.js and call it from main thread from file detail.js, the object location it return is NULL. After print to console the object, I realized that the variable returned_location was returned before the function finish finding location. I think its the multi-thread problems but I really don't know how to fix it. Here are my files.

detail.js

var Frame = require("ui/frame");
var Observable = require("data/observable");

var MapsModel = require("../../view-models/maps-model");


var defaultMapInfo = new MapsModel({
    latitude: "10.7743332",
    longitude: "106.6345204",
    zoom: "0",
    bearing: "0",
    tilt: "0",
    padding: "0"
});

var page;
var mapView;

exports.pageLoaded = function(args) {
    page = args.object;
    var data = page.navigationContext;
    page.bindingContext = defaultMapInfo;
}

exports.onBackTap = function () {
    console.log("Back to home");
    var topmost = Frame.topmost();
    topmost.goBack();
}

function onMapReady(args) {
    mapView = args.object;
    mapView.settings.zoomGesturesEnabled = true;
}

function onMarkerSelect(args) {
    console.log("Clicked on " + args.marker.title);
}

function onCameraChanged(args) {
    console.log("Camera changed: " + JSON.stringify(args.camera)); 
}

function getCurPos(args) {
    var returned_location = defaultMapInfo.getCurrentPosition(); // variable is returned before function finished
    console.dir(returned_location);
}


exports.onMapReady = onMapReady;
exports.onMarkerSelect = onMarkerSelect;
exports.onCameraChanged = onCameraChanged;
exports.getCurPos = getCurPos;

maps-module.js

var Observable = require("data/observable");

var Geolocation = require("nativescript-geolocation");
var Gmap = require("nativescript-google-maps-sdk");

function Map(info) {
    info = info || {};
    var _currentPosition;

    var viewModel = new Observable.fromObject({
        latitude: info.latitude || "",
        longitude: info.longitude || "",
        zoom: info.zoom || "",
        bearing: info.bearing || "",
        tilt: info.bearing || "",
        padding: info.padding || "",
    });

    viewModel.getCurrentPosition = function() {
        if (!Geolocation.isEnabled()) {
            Geolocation.enableLocationRequest();
        }

        if (Geolocation.isEnabled()) {
            var location = Geolocation.getCurrentLocation({
                desiredAccuracy: 3, 
                updateDistance: 10, 
                maximumAge: 20000, 
                timeout: 20000
            })
            .then(function(loc) {
                if (loc) {
                    console.log("Current location is: " + loc["latitude"] + ", " + loc["longitude"]);
                    return Gmap.Position.positionFromLatLng(loc["latitude"], loc["longitude"]);
                }
            }, function(e){
                console.log("Error: " + e.message);
            });

            if (location)
                console.dir(location);
        }
    }

    return viewModel;
}

module.exports = Map;

Answer 1

If Shiva Prasad's footnote ...

"geolocation.enableLocationRequest() is also an asynchorous method"

... is correct, then the Promise returned by geolocation.enableLocationRequest() must be handled appropriately and the code will change quite considerably.

Try this :

viewModel.getCurrentPosition = function(options) {
    var settings = Object.assign({
        'desiredAccuracy': 3,
        'updateDistance': 10,
        'maximumAge': 20000,
        'timeout': 20000
    }, options || {});

    var p = Promise.resolve() // Start promise chain with a resolved native Promise.
    .then(function() {
        if (!Geolocation.isEnabled()) {
            return Geolocation.enableLocationRequest(); // return a Promise
        } else {
            // No need to return anything here.
            // `undefined` will suffice at next step in the chain.
        }
    })
    .then(function() {
        if (Geolocation.isEnabled()) {
            return Geolocation.getCurrentLocation(settings); // return a Promise
        } else { // <<< necessary to handle case where Geolocation didn't enable.
            throw new Error('Geolocation could not be enabled');
        }
    })
    .then(function(loc) {
        if (loc) {
            console.log("Current location is: " + loc.latitude + ", " + loc.longitude);
            return Gmap.Position.positionFromLatLng(loc.latitude, loc.longitude);
        } else { // <<< necessary to handle case where loc was not derived.
            throw new Error('Geolocation enabled, but failed to derive current location');
        }
    })
    .catch(function(e) {
        console.error(e);
        throw e; // Rethrow the error otherwise it is considered caught and the promise chain will continue down its success path.
        // Alternatively, return a manually-coded default `loc` object.
    });

    // Now race `p` against a timeout in case enableLocationRequest() hangs.
    return Promise.race(p, new Promise(function(resolve, reject) {
        setTimeout(function() {
            reject(new Error('viewModel.getCurrentPosition() timed out'));
        }, settings.timeout);
    }));
}
return viewModel;

Notes :

1. Starting the chain with a resolved native Promise gives much the same effect as wrapping in new Promise(...) but is cleaner chiefly because unexpected throws within the chain are guaranteed to deliver an Error object down the chain's error path without needing to try/catch/reject(). Also, in the two lines labelled "return a Promise", we don't need to care whether we return a Promise or a value; either will be assimilated by the native Promise chain.

2. Two else clauses are included to cater for failure cases which would not automatically throw.

3. The Promise.race() shouldn't be necessary but is a safeguard against the issue reported here. It's possible that the inbuilt 'timeout' mechanism will suffice. This extra timeout mechanism is a "belt-and-braces" measure.

4. A mechanism is included to override the hard-coded defaults in viewModel.getCurrentPosition by passing an options object. To run with the defaults, simply call viewModel.getCurrentPosition(). This feature was introduced primarily to allow settings.timeout to be reused in the Promise.race().

EDIT:

Thanks @grantwparks for the information that Geolocation.isEnabled() also returns Promise.

So now we can start the Promise chain with p = Geolocation.isEnabled().... and test the asynchronously delivered Boolean. If false then attempt to enable.

From that point on, the Promise chain's success path will be followed if geolocation was initially enabled or if it has been enabled. Further tests for geolocation being enabled disappear.

This should work:

viewModel.getCurrentPosition = function(options) {
    var settings = Object.assign({
        'desiredAccuracy': 3,
        'updateDistance': 10,
        'maximumAge': 20000,
        'timeout': 20000
    }, options || {});

    var p = Geolocation.isEnabled() // returned Promise resolves to true|false.
    .then(function(isEnabled) {
        if (isEnabled) {
            // No need to return anything here.
            // `undefined` will suffice at next step in the chain.
        } else {
            return Geolocation.enableLocationRequest(); // returned Promise will cause main chain to follow success path if Geolocation.enableLocationRequest() was successful, or error path if it failed;
        }
    })
    .then(function() {
        return Geolocation.getCurrentLocation(settings); // return Promise
    })
    .then(function(loc) {
        if (loc) {
            console.log("Current location is: " + loc.latitude + ", " + loc.longitude);
            return Gmap.Position.positionFromLatLng(loc.latitude, loc.longitude);
        } else { // <<< necessary to handle case where loc was not derived.
            throw new Error('Geolocation enabled, but failed to derive current location');
        }
    })
    .catch(function(e) {
        console.error(e);
        throw e; // Rethrow the error otherwise it is considered caught and the promise chain will continue down its success path.
        // Alternatively, return a manually-coded default `loc` object.
    });

    // Now race `p` against a timeout in case Geolocation.isEnabled() or Geolocation.enableLocationRequest() hangs.
    return Promise.race(p, new Promise(function(resolve, reject) {
        setTimeout(function() {
            reject(new Error('viewModel.getCurrentPosition() timed out'));
        }, settings.timeout);
    }));
}
return viewModel;

Answer 2

Since getting location is an Async process, your viewModel.getCurrentPosition should return a promise, and would look something like this,

viewModel.getCurrentPosition() {
    return new Promise((resolve, reject) => {
        geolocation
            .getCurrentLocation({
                desiredAccuracy: enums.Accuracy.high,
                updateDistance: 0.1,
                maximumAge: 5000,
                timeout: 20000
            })
            .then(r => {
                resolve(r);
            })
            .catch(e => {
                reject(e);
            });
    });
}

and then when you use it, it would look like this

defaultMapInfo.getCurrentPosition()
    .then(latlng => {
       // do something with latlng {latitude: 12.34, longitude: 56.78}
    }.catch(error => {
       // couldn't get location
    }
}

Hope that helps :)

Update: BTW, geolocation.enableLocationRequest() is also an asynchorous method.