feat: remove deprecated RNMBXMapView's mapView & mapboxMap APIs #3963

PRESIDENT810 · 2025-09-04T22:10:17Z

Description

In our App, the deprecated APIs (RNMBXMapView.mapView & RNMBXMapView.mapboxMap) cause the app to crash because rnmapbox is trying to unwrap them while they are still nil.

This PR removes those two deprecated APIs and changes all their usage to the safe withMapView & withMapboxMap API calls. After this fix our app no longer crashes.

Checklist

I've read CONTRIBUTING.md
I updated the doc/other generated code with running yarn generate in the root folder
I have tested the new feature on /example app.
- In V11 mode/ios
- In New Architecture mode/ios
- [] In V11 mode/android
- [] In New Architecture mode/android
I added/updated a sample - if a new feature was implemented (/example)

Screenshot OR Video

Component to reproduce the issue you're fixing

Kinda hard since the crash is triggered by an edge case in our app and we don't know how to create a minimal reproducing example yet but after I patched this package our app no longer crashes.

In our App, the deprecated APIs (`RNMBXMapView.mapView` & `RNMBXMapView.mapboxMap`) cause the app to crash because rnmapbox is trying to unwrap them while they are still `nil`. This PR removes those two deprecated APIs and changes all their usage to the safe `withMapView` & `withMapboxMap` API calls.

mfazekas · 2025-09-05T04:49:59Z

@PRESIDENT810 thanks much for the PR

Unfortunately it's a bit complicated. Like in case of remove? withMapView could keep stuff alive and create a leak. So one thing we have to be carefully withMapView is leaks the other is concurrency, as the code will be called from other thread. And can cause deadlock and other issues.

Do you perhaps has stack traces? So we can address those cases, where you hit by the nil mapboxMap?

PRESIDENT810 · 2025-09-05T18:23:56Z

@mfazekas Here's the stacktrace

          Crashed: com.apple.main-thread
0  <our app>                       0x2591ccc closure #5 in RNMBXMapView.setupEvents() + 1117 (RNMBXMapView.swift:1117)
1  <our app>                       0x258e554 specialized closure #1 in RNMBXMapView.onEvery<A>(event:handler:) + 305072
2  <our app>                       0x2597c38 partial apply for specialized closure #1 in RNMBXMapView.onNext<A>(event:handler:) + 343700 (<compiler-generated>:343700)
3  MapboxMaps                     0xb03cc closure #1 in MapboxObservable.onNext<A>(event:handler:) + 84
4  MapboxMaps                     0xb1950 partial apply for closure #1 in MapboxObservable.onNext<A>(event:handler:) + 20 (<compiler-generated>:20)
5  MapboxMaps                     0xb0524 @objc MapboxObservable.BlockObserver.notify(for:) + 166 (MapboxObservable.swift:166)
6  MapboxCoreMaps                 0x297f4 mapbox::bindgen::mapbox::maps::Observer::notify(mapbox::maps::Event const&) + 52
7  MapboxCoreMaps                 0x95644 mapbox::maps::ObserverWrapper::notify(mbgl::ObservableEvent const&) + 692
8  MapboxCoreMaps                 0x560d18 std::__1::__function::__func<mbgl::Observable::dispatchEvent(mbgl::ObservableEvent const&, mbgl::DispatchMode)::$_0, std::__1::allocator<mbgl::Observable::dispatchEvent(mbgl::ObservableEvent const&, mbgl::DispatchMode)::$_0>, void ()>::operator()() + 156
9  MapboxCommon                   0x4dc98 mapbox::common::platform::taskScope() + 1360
10 CoreFoundation                 0xf92c __CFRUNLOOP_IS_CALLING_OUT_TO_A_SOURCE0_PERFORM_FUNCTION__ + 28
11 CoreFoundation                 0xf744 __CFRunLoopDoSource0 + 172
12 CoreFoundation                 0xf604 __CFRunLoopDoSources0 + 332
13 CoreFoundation                 0xff20 __CFRunLoopRun + 840
14 CoreFoundation                 0x11adc CFRunLoopRunSpecific + 572
15 GraphicsServices               0x1454 GSEventRunModal + 168
16 UIKitCore                      0x135274 -[UIApplication _run] + 816
17 UIKitCore                      0x100a28 UIApplicationMain + 336
18 <our app>                       0xf9b4 main + 36 (main.m:36)

I can see the crash is caused by 5th closure in RNMBXMapView.setupEvents with "Swift runtime failure: Unexpectedly found nil while implicitly unwrapping an optional value" but I can't see what triggers it.

For leaks, I think I used [weak self] and tried my best to avoid circular reference here, and for concurrency, I think the callers to withMapView & withMapboxMap will not wait for anything. If RNMBXMapView._mapView is nil, then the closure is appended to waiters and the function returns immediately. It's basically promising to execute that closure as soon as _mapView is ready, and when it's ready, closures in waiter will execute serially, which I don't think would cause deadlocks.

PRESIDENT810 · 2025-10-01T22:45:57Z

ping @mfazekas
We've been using this patch in prod for a several weeks with good results - would love if this could merge to main so that we don't have to maintain our own patch. assuming others would benefit from this as well; happy to make more improvements in the future

mfazekas · 2025-10-11T09:28:47Z

ios/RNMBX/RNMBXAtmosphere.swift


-    guard let mapboxMap = map.mapboxMap else {
-      return false
+    map.withMapboxMap { [weak self] _mapboxMap in


Not sure about this. Maybe we should store the style we've added ourself to? Doesn't make much sense to wait for a style here. It could be other style, then we've added to, so...

Also not sure if a race like this is possible:

item added to map with style A

style removed from map

item tries to removeFromMap so it's waits for style

style B is added to map

item is added to style B

withMapboxMap is executed item removed from style B

this can't happen as 6.) will be called before 4.) but it's something I need to think about on every change like this.

mfazekas · 2025-10-11T09:29:16Z

ios/RNMBX/RNMBXCamera.swift

-      if let center = camera.center {
-        try center.validate()
-      }
+    map.withMapView { _mapView in


mfazekas · 2025-10-11T09:31:10Z

ios/RNMBX/RNMBXCamera.swift

    }

-    map.mapView.viewport.removeStatusObserver(self)
+    map.withMapView { _mapView in


Same as the other remove, we don't want to try to remove from other mapViewpot we've added ourselves to

mfazekas · 2025-10-11T09:32:07Z

ios/RNMBX/RNMBXCustomLocationProvider.swift

    self.map = map
-    if let mapView = map.mapView {
-      installCustomeLocationProviderIfNeeded(mapView: mapView)
+    map.withMapView{ _mapView in


So we have a valid style, we should have a map too. Maybe add a map parameter as well?!

mfazekas · 2025-10-11T09:36:51Z

ios/RNMBX/RNMBXImageSource.swift

        source.url = url
-        self.doUpdate { (style) in
-          try! style.setSourceProperty(for: id, property: "url", value: url)
+        self.map?.withMapboxMap { [weak self] _mapboxMap in


same concern here as well. We've added ourself to style. So we don't want to wait for it again. Maybe we should save the style here as well

mfazekas · 2025-10-11T09:38:14Z

ios/RNMBX/RNMBXMapViewManager.swift

-      resolver(["data": NSNumber(value: result)])
-    } else {
-      resolver(nil)
+    view.withMapboxMap { [weak view] _mapboxMap in


mfazekas · 2025-10-11T09:38:27Z

ios/RNMBX/RNMBXMapViewManager.swift

-          ])
-        case .failure(let error):
-          rejecter("queryRenderedFeaturesInRect", "failed to query features", error)
+    map.withMapboxMap { [weak map] _mapboxMap in


mfazekas · 2025-10-11T09:38:40Z

ios/RNMBX/RNMBXMapViewManager.swift

-          rejecter(
-            "querySourceFeatures",
-            "failed to query source features: \(error.localizedDescription)", error)
+    map.withMapboxMap { _mapboxMap in


mfazekas · 2025-10-11T09:45:10Z

@PRESIDENT810 thanks again, and sorry for taking this long.

So generally there is 2 cases in the changes.
1.) Things like MapView.queryRenderedFeaturesInRect: withMapboxMap is good here, as we might have called queryRenderedFeaturesInRect before the map init has finished. Waiting for an underlying map view is great here.
2.) Components added to a specific map/style - I think they'd be better to store the map/style. I don't like withMapboxMap there as it might mean a different map/style we've already added the component.

PRESIDENT810 temporarily deployed to CI with Mapbox Tokens September 4, 2025 22:10 — with GitHub Actions Inactive

PRESIDENT810 had a problem deploying to CI with Mapbox Tokens September 4, 2025 22:10 — with GitHub Actions Failure

PRESIDENT810 temporarily deployed to CI with Mapbox Tokens September 4, 2025 22:10 — with GitHub Actions Inactive

PRESIDENT810 had a problem deploying to CI with Mapbox Tokens September 4, 2025 22:10 — with GitHub Actions Failure

PRESIDENT810 temporarily deployed to CI with Mapbox Tokens September 4, 2025 22:10 — with GitHub Actions Inactive

PRESIDENT810 had a problem deploying to CI with Mapbox Tokens September 4, 2025 22:10 — with GitHub Actions Failure

PRESIDENT810 changed the title ~~feat: deprecate RNMBXMapView's mapView & mapboxMap APIs~~ feat: remove deprecated RNMBXMapView's mapView & mapboxMap APIs Sep 4, 2025

fix

8a7528e

PRESIDENT810 had a problem deploying to CI with Mapbox Tokens September 15, 2025 21:46 — with GitHub Actions Failure

mfazekas reviewed Oct 11, 2025

View reviewed changes

ios/RNMBX/RNMBXCamera.swift

if let center = camera.center {

try center.validate()

}

map.withMapView { _mapView in

Copy link

Contributor

mfazekas Oct 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

mfazekas reviewed Oct 11, 2025

View reviewed changes

Uh oh!

feat: remove deprecated RNMBXMapView's mapView & mapboxMap APIs #3963

Are you sure you want to change the base?

feat: remove deprecated RNMBXMapView's mapView & mapboxMap APIs #3963

Uh oh!

Conversation

PRESIDENT810 commented Sep 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Checklist

Screenshot OR Video

Component to reproduce the issue you're fixing

Uh oh!

mfazekas commented Sep 5, 2025

Uh oh!

PRESIDENT810 commented Sep 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

PRESIDENT810 commented Oct 1, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mfazekas commented Oct 11, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

PRESIDENT810 commented Sep 4, 2025 •

edited

Loading

PRESIDENT810 commented Sep 5, 2025 •

edited

Loading