React-native-mapbox-gl: Update to Android Mapbox GL Native 5.0.0

[tobrun]

This ticket covers the actions needed to get react-native-mapbox-gl up-and-working with the latests Android Mapbox SDK version. We recently released the third beta version of 5.0.0 and are planning a stable release in the upcoming weeks.

Crash from 4.2.0-beta.4 and up

The https://github.com/mapbox/mapbox-gl-native/issues/7587 upstream ticket in gl-native covers a crash that was added in Android Mapbox SDK 4.2.0-beta and up. Since that version, we started to load the native library as part of the MapboxAccountManager creation. This requires, with current gl-native threading model, that this class must be created from the main thread. The code currently found in the repository doesn't take this in account. Loading classes from the main thread goes for every class that contains the following definition, or classes that call into components using:

static {
  System.loadLibrary("mapbox-gl");
}

Adapt API to 5.0.0 changes

MapboxAccountManager.java to Mapbox.java

With the 5.0.0 release, we are changing MapboxAccountManager to Mapbox as this component has grown in responsibility in the latest few SDK releases.

// 4.x.x
MapboxAccountManager.start(context, <your access token here>);
// 5.0.0
Mapbox.getInstance(context, <your access token here>));

If you need a reference to the set access token later on in the code you can do:

Mapbox.getAccessToken()

AndroidManifest.xml

The telemetry service package has moved. You'll need to change this from:

// 4.x.x
<service android:name="com.mapbox.mapboxsdk.telemetry.TelemetryService"/>
// 5.0.0
<service android:name="com.mapbox.services.android.telemetry.service.TelemetryService"/>

Activity lifecycle events

Part of supporting Android Nougat means we have to also support the new Multi-Window feature. This required us to move some of the logic to the onStart() and onStop() methods. When switching over to 5.0 Android Studio won't complain about these missing but when compiling your application will immediately crash if you don't add them to the activity:

@Override
protected void onStart() {
  super.onStart();
  mapView.onStart();
}

@Override
protected void onStop() {
  super.onStop();
  mapView.onStop();
}

Min/Max zoom level changes

Part of updating the API changed the name of setMinZoomLevel and setMaxZoomLevel. These API's have now been renamed to setMinZoomPreference and setMaxZoomPreference respectivly.

Mapbox attribution name changes

Not sure, if attributes are used in the react-native module atm but we've made significant improvements to our attribution naming. All attributes now start with mapbox_ for easier identifying and removing any potential conflicts with other libraries. In addition, many of the attributes are now named after their Java counterpart. Instead of listing all the changes that have occurred, you might find the attrs.xml file which has all the names. Some of the common attributes you might use in your XML map view are given in the table below:

| 4.x attribute names | 5.0 attribute names |
| ------------------- |:------------------------:|
| center_latitude | mapbox_cameraTargetLat |
| center_longitude | mapbox_cameraTargetLng |
| zoom | mapbox_cameraZoom |
| direction | mapbox_cameraBearing |
| tilt | mapbox_cameraTilt |
| style_url | mapbox_styleUrl |
| zoom_max | mapbox_cameraZoomMax |
| zoom_min | mapbox_cameraZoomMin |
| rotate_enabled | mapbox_uiRotateGestures|

Update dependencies and build tools

Updating to the latest Mapbox Android SDK requires also to update the underlying build tools and dependencies. We should also look into brining this SDK to the latest react-native version.

Known bugs

While looking into the upstream crash, I was hitting the bug that the map was not showing on first initialisation. I had to rotate the device before the map was shown:

Checklist

• [ ] update dependencies and tools
• [ ] change MapboxAccountManager to Mapbox and access only from main thread
• [ ] update API changes for 5.0.0
• [ ] expose new APIs as runtime and data driven styling
• [ ] resolve no map shown at initialisation

cc @mapbox/android @esamelson @K-Leon @gillessed

[esamelson]

@tobrun , thanks so much for all your work in identifying the initial crash I saw, and also outlining the steps to update this repo to be compatible with 5.0.0. I've just opened a PR (#517) which resolves the 4.2.0-beta.4 crash and at least makes this repo usable with the stable 4.2.x releases for now. I'm willing to help out with the 5.0.0 update as well and will open more PRs as I have time.

[gillessed]

@tobrun @esamelson Agreed. Thanks a lot for digging into that one. I'm also happy to help with the bump to 5.0.0.

[jeveloper]

@tobrun thanks, i saw the news on mapbox site with the release of 5.0 and respective ios update. Hows this lib coming along? RN 44 btw makes a few changes that may break it.
Love to see an actual release , there hasn't been one in half a year.

thank you ! and Mapbox folks

[amotzg]

We did some initial work for this on https://github.com/AmudAnan/react-native-mapbox-gl/tree/mbgl-android-v5.0.2
It compiles and run, but crash immediately.
Will be glad for feedback or direction for the next step (currently on another task, but plan to debug the crash and continue with wrapper updates for new API).
Also, I didn't PRed because it appeared there is no suitable branch yet, and work is not complete. Let me know where/if it can be PRed and I will.

[zugaldia]

We did some initial work for this on https://github.com/AmudAnan/react-native-mapbox-gl/tree/mbgl-android-v5.0.2 It compiles and run, but crash immediately.

Could you share the crash log? Happy to 👀 it and see if it looks like anything we've seen before.

[jeveloper]

FYI RN 44 came out today, fully removing mapview , just letting you know :)
it would be amazing to know general roadmap for this lib, if its worth investing time into and essp using in a product development. mapbox on its own is amazing but if the wrapper isnt going to be updated much like your ios/android sdks , then its a hard sell

thank you

[amotzg]

@zugaldia, I'm pretty sure it is incomplete version bump, rather than an SDK issue.
Attached are the full log and a sample project. The sample project include Android/build tools version changes, and code changes external to repository code. Markdown doc with project creation notes is also attached.

Recent RN versions moved package register from ReactActivity to ReactApplication. This is one of the changes in my sample project.
The following error log line might be related, trying to getSharedPreferences from wrong context or with no context:
C++ Exception in 'NativeModules': java.lang.NullPointerException: Attempt to invoke virtual method 'android.content.SharedPreferences android.content.Context.getSharedPreferences(java.lang.String, int)' on a null object reference

logcat
whole project
setup notes

[jeveloper]

the change you're referring to happened back in RN 32 or something like that, a while back

[zugaldia]

@amotzg Thanks for sharing. By looking at the logcat file it indeed looks like it isn't coming from the SDK. The problem is with an empty (null) context, likely the application context, that cannot access shared preferences (Context.getSharedPreferences()).

In case it helps, we have a migration guide for Android from 4.x to 5.x: https://github.com/mapbox/mapbox-gl-native/wiki/Android-4.x-to-5.0-update. I'd start by checking all the changes listed on that list have been addressed.

[zugaldia]

Additionally, we'd like to give this project some more love and we're gonna hire a dedicated engineer to join the team. There'll be an opening published on the website soon, meanwhile you can contact @pveugen directly. Please do share this with your networks!

[jackfengji]

@zugaldia @tobrun I made #553 and most of things are working in my app after upgrading.

The only problem is we cannot put metricsEnabled to constant. MapboxTelemetry has not been initialized when RN getConstants. I commented this out for now and we can discuss about how to do it.

[zugaldia]

@jackfengji Glad to see progress here!

The only problem is we cannot put metricsEnabled to constant. MapboxTelemetry has not been initialized when RN getConstants. I commented this out for now and we can discuss about how to do it.

Could you elaborate on why is this necessary? Why aren't you relying on the Mapbox object to take care of initializing telemetry? What's metricsEnabled used for? Thanks.

[jackfengji]

@zugaldia I'm not using this field in my app. But it's in the current version. We cannot put metricsEnabled in constant like what's in the SDK now because telemetry has not been initialized and it will crash because context is still null. I commented that line out for now. https://github.com/mapbox/react-native-mapbox-gl/pull/553/files#diff-511703cd599f72abb07f0ddde993736fR117

We could change the js file so it will get the value of metricsEnabled when needed instead of making it a constant.

[zugaldia]

@jackfengji thank you for the clarification.

We could change the js file so it will get the value of metricsEnabled when needed instead of making it a constant.

👍 A call to MapboxEventManager.getMapboxEventManager().isTelemetryEnabled() should always provide a valid value (once it's been initialized). Under the hood, it's using SharedPreferences to track individual users opt-in/opt-out state (hence the need for a Context).

[jackfengji]

@zugaldia I changed getMetricsEnabled to return a promise instead of value.

[zugaldia]

Job opening I was referring to earlier https://github.com/mapbox/react-native-mapbox-gl/issues/516#issuecomment-301079420 is now live: https://www.mapbox.com/jobs/689889. Help us find the right person for this project!

[dagatsoin]

Ouch #517 saved my life. I just reproduced it for the Cordova plugin I am working on. Thx you very much @esamelson

[nitaliano]

Our v6 rewrite will ship with the latest ios and android sdk, and v5 supports this.