React-native-background-geolocation: Order of parameters in the documentation does not match the code

[ca057]

Your Environment

• Plugin version: 2.6.1
• Platform: iOS
• OS version: iOS 10.2
• Device manufacturer / model: iPhone 6s (Simulator)
• React Native version (react-native -v): 0.39.2

Expected Behavior

According to the documentation, the order of the parameters of BackgroundGeolocation.getCurrentPosition-function is success, failure, {options} (see the documentation). I expect the function to be used like that.

Actual Behavior

If I order the parameters like the documentation states, I get an error saying "Cannot have a non-function arg after a function arg. Invariant Violation: Cannot have a non-function arg after a function arg". Checking the source code, the parameters are expected in a different order ({options}, success, failure).

Steps to Reproduce

Breaking code:

const successCallback = () => {};
const errorCallback = () => {};
const configuration = {
  timeout: 30,
  maximumAge: 5000,
  desiredAccuracy: 10,
};
BackgroundGeolocation.getCurrentPosition(successCallback, errorCallback, configuration);

Working code:

const successCallback = () => {};
const errorCallback = () => {};
const configuration = {
  timeout: 30,
  maximumAge: 5000,
  desiredAccuracy: 10,
};
BackgroundGeolocation.getCurrentPosition(configuration, successCallback, errorCallback);

Context

Accessing the current location inside a Promise-chain.

Debug logs

If this is a bug which needs to be fixed I would like to help out with a PR, either by changing the documentation or by reordering the parameters (which I would prefer, even if this is a breaking change). If this is the case, what's the preferred solution?

[christocracy]

Yes, I re-arranged the params in the JS api in the Premium version but forgot to copy that change here.

It will be fixed ASAP and the change will be backwards-compat.

It's more convenient from an API perspective to place the optional params last rather than first.

// new
  getCurrentPosition: function(success, failure, options) {
    var _success = emptyFn
       _failure = emptyFn,
       _options = {};

    // Detect old API: getCurrentPosition(options, success, failure)
    if (typeof(success) === 'object') {
      _options = success;
      if (typeof(options) === 'function') {
        _failure = options;
      }
      if (typeof(failure) === 'function') {
        _success = failure;
      }
    } else {  // New API getCurrentPosition(success, failure, options);
      _success = success || emptyFn;
      _failure = failure || emptyFn;
      _options = options || {};
    }
    RNBackgroundGeolocation.getCurrentPosition(_options, _success, _failure);
  },

[christocracy]

It is, of course, necessary for the native API to receive the {Object} parameters before Function parameters but that can be hidden from the consumer of the Javascript API.

[christocracy]

Can you try installing from the branch getCurrentPosition.

[ca057]

Yes, that's what I thought. Leave the native API as it is, but change the signature of the JS-API. Thanks for the fast response, I will shortly give it a try and will let you know if everything works out!

[ca057]

This is working perfectly, thanks for the fix!

[ca057]

Will this get merged to master at some point? If it is easier, I`d like to help out with a PR.

[christocracy]

I merged it yesterday. Forgot to ref this issue

https://github.com/transistorsoft/react-native-background-geolocation/pull/219