Skip to content

Commit 72b8c38

Browse files
committed
refactor(ios): introduce RNMBXMapAndMapViewComponent protocol for type-safe mapView access
This commit addresses the issue raised in PR #3963 regarding the optional mapView property access pattern that could lead to nil access crashes. Key Changes: - Created new RNMBXMapAndMapViewComponent protocol that guarantees non-nil MapView parameter in addToMap and removeFromMap methods - Kept existing RNMBXMapComponent protocol for components that don't require direct MapView access - Updated RNMBXMapView to handle both protocol types and validate mapView presence before calling RNMBXMapAndMapViewComponent methods - Created RNMBXMapAndMapViewComponentBase as a base class for components requiring MapView Components migrated to RNMBXMapAndMapViewComponent: - RNMBXCustomLocationProvider: Needs mapView for location provider setup - RNMBXCamera: Accesses mapView.viewport for status observers - RNMBXViewport: Directly manages viewport state - RNMBXNativeUserLocation: Configures location puck via mapView.location - RNMBXInteractiveElement: Base class that accesses mapView.mapboxMap.style - RNMBXSource: Manages sources via mapView.mapboxMap.style - RNMBXPointAnnotation: Inherits from RNMBXInteractiveElement Architecture Benefits: - Type system enforces mapView availability at compile time - Clear contract: components explicitly declare mapView requirement - Eliminates defensive nil checks in component implementations - Centralizes nil checking in RNMBXMapView with proper error logging - Maintains invariant: if addedToMap is true, mapView must be non-nil This approach is superior to PR #3963's defensive nil checks because: 1. It makes the requirement explicit via protocol 2. Failures are logged at the framework level, not silently ignored 3. Type safety prevents accidental nil access 4. Components can safely assume mapView is valid in lifecycle methods Related: #3963
1 parent 8dd5acb commit 72b8c38

8 files changed

+214
-102
lines changed

ios/RNMBX/RNMBXCamera.swift

Lines changed: 64 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,17 @@ public enum RemovalReason {
1818
public protocol RNMBXMapComponent: AnyObject {
1919
func addToMap(_ map: RNMBXMapView, style: Style)
2020
func removeFromMap(_ map: RNMBXMapView, reason: RemovalReason) -> Bool
21-
21+
22+
func waitForStyleLoad() -> Bool
23+
}
24+
25+
/// Protocol for components that require a valid MapView instance for both add and remove operations.
26+
/// Use this protocol when your component needs to interact with the native MapView directly.
27+
/// The MapView parameter is guaranteed to be non-nil when these methods are called.
28+
public protocol RNMBXMapAndMapViewComponent: AnyObject {
29+
func addToMap(_ map: RNMBXMapView, mapView: MapView, style: Style)
30+
func removeFromMap(_ map: RNMBXMapView, mapView: MapView, reason: RemovalReason) -> Bool
31+
2232
func waitForStyleLoad() -> Bool
2333
}
2434

@@ -85,7 +95,7 @@ class CameraUpdateQueue {
8595
open class RNMBXMapComponentBase : UIView, RNMBXMapComponent {
8696
private weak var _map: RNMBXMapView! = nil
8797
private var _mapCallbacks: [(RNMBXMapView) -> Void] = []
88-
98+
8999
weak var map : RNMBXMapView? {
90100
return _map;
91101
}
@@ -103,28 +113,70 @@ open class RNMBXMapComponentBase : UIView, RNMBXMapComponent {
103113
_mapCallbacks.append(callback)
104114
}
105115
}
106-
116+
107117
public func waitForStyleLoad() -> Bool {
108118
return false
109119
}
110-
120+
111121
public func addToMap(_ map: RNMBXMapView, style: Style) {
112122
_mapCallbacks.forEach { callback in
113123
callback(map)
114124
}
115125
_mapCallbacks = []
116126
_map = map
117127
}
118-
128+
119129
public func removeFromMap(_ map: RNMBXMapView, reason: RemovalReason) -> Bool {
120130
_mapCallbacks = []
121131
_map = nil
122132
return true
123133
}
124134
}
125135

136+
/// Base class for components that require MapView to be non-nil
137+
open class RNMBXMapAndMapViewComponentBase : UIView, RNMBXMapAndMapViewComponent {
138+
private weak var _map: RNMBXMapView! = nil
139+
private var _mapCallbacks: [(RNMBXMapView) -> Void] = []
140+
141+
weak var map : RNMBXMapView? {
142+
return _map;
143+
}
144+
145+
func withMapView(_ callback: @escaping (_ mapView: MapView) -> Void) {
146+
withRNMBXMapView { mapView in
147+
callback(mapView.mapView)
148+
}
149+
}
150+
151+
func withRNMBXMapView(_ callback: @escaping (_ map: RNMBXMapView) -> Void) {
152+
if let map = _map {
153+
callback(map)
154+
} else {
155+
_mapCallbacks.append(callback)
156+
}
157+
}
158+
159+
public func waitForStyleLoad() -> Bool {
160+
return false
161+
}
162+
163+
public func addToMap(_ map: RNMBXMapView, mapView: MapView, style: Style) {
164+
_mapCallbacks.forEach { callback in
165+
callback(map)
166+
}
167+
_mapCallbacks = []
168+
_map = map
169+
}
170+
171+
public func removeFromMap(_ map: RNMBXMapView, mapView: MapView, reason: RemovalReason) -> Bool {
172+
_mapCallbacks = []
173+
_map = nil
174+
return true
175+
}
176+
}
177+
126178
@objc(RNMBXCamera)
127-
open class RNMBXCamera : RNMBXMapComponentBase {
179+
open class RNMBXCamera : RNMBXMapAndMapViewComponentBase {
128180
var cameraAnimator: BasicCameraAnimator?
129181
let cameraUpdateQueue = CameraUpdateQueue()
130182

@@ -519,18 +571,18 @@ open class RNMBXCamera : RNMBXMapComponentBase {
519571
_updateCamera()
520572
}
521573

522-
public override func addToMap(_ map: RNMBXMapView, style: Style) {
523-
super.addToMap(map, style: style)
574+
public override func addToMap(_ map: RNMBXMapView, mapView: MapView, style: Style) {
575+
super.addToMap(map, mapView: mapView, style: style)
524576
map.reactCamera = self
525577
}
526-
527-
public override func removeFromMap(_ map: RNMBXMapView, reason: RemovalReason) -> Bool {
578+
579+
public override func removeFromMap(_ map: RNMBXMapView, mapView: MapView, reason: RemovalReason) -> Bool {
528580
if (reason == .StyleChange) {
529581
return false
530582
}
531583

532-
map.mapView.viewport.removeStatusObserver(self)
533-
return super.removeFromMap(map, reason:reason)
584+
mapView.viewport.removeStatusObserver(self)
585+
return super.removeFromMap(map, mapView: mapView, reason: reason)
534586
}
535587

536588
@objc public func moveBy(x: Double, y: Double, animationMode: NSNumber?, animationDuration: NSNumber?, resolve: @escaping RCTPromiseResolveBlock, reject: @escaping RCTPromiseRejectBlock) {

ios/RNMBX/RNMBXCustomLocationProvider.swift

Lines changed: 17 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -3,70 +3,66 @@ import MapboxMaps
33
let TAG = "RNMBXCustomLocationProvider"
44

55
@objc
6-
public class RNMBXCustomLocationProvider: UIView, RNMBXMapComponent {
6+
public class RNMBXCustomLocationProvider: UIView, RNMBXMapAndMapViewComponent {
77
var map: RNMBXMapView? = nil
8-
8+
99
let changes : PropertyChanges<RNMBXCustomLocationProvider> = PropertyChanges()
10-
10+
1111
enum Property: String {
1212
case coordinate
1313
case heading
14-
14+
1515
func apply(locationProvider: RNMBXCustomLocationProvider) {
1616
switch self {
1717
case .coordinate: locationProvider.applyCoordinate()
1818
case .heading: locationProvider.applyHeading()
1919
}
2020
}
2121
}
22-
22+
2323
@objc
2424
public var coordinate: [Double] = [] {
2525
didSet { changed(.coordinate) }
2626
}
27-
27+
2828
@objc
2929
public var heading: NSNumber = 0.0 {
3030
didSet { changed(.heading) }
3131
}
32-
32+
3333
func changed(_ property: Property) {
3434
changes.add(name: property.rawValue, update: property.apply)
3535
}
36-
36+
3737
@objc
3838
override public func didSetProps(_ props: [String]) {
3939
if customLocationProvider != nil {
4040
changes.apply(self)
4141
}
4242
}
43-
43+
4444
var customLocationProvider: CustomLocationProvider? = nil
4545
#if RNMBX_11
4646
#else
4747
var defaultLocationProvider: LocationProvider?
4848
#endif
4949

50-
public func addToMap(_ map: RNMBXMapView, style: Style) {
50+
public func addToMap(_ map: RNMBXMapView, mapView: MapView, style: Style) {
5151
self.map = map
52-
if let mapView = map.mapView {
53-
installCustomeLocationProviderIfNeeded(mapView: mapView)
54-
changes.apply(self)
55-
}
52+
installCustomeLocationProviderIfNeeded(mapView: mapView)
53+
changes.apply(self)
5654
}
57-
55+
5856
private func applyCoordinate() {
5957
updateCoordinate(latitude: coordinate[1], longitude: coordinate[0])
6058
}
61-
59+
6260
private func applyHeading() {
6361
updateHeading(heading: heading.doubleValue)
6462
}
65-
66-
public func removeFromMap(_ map: RNMBXMapView, reason: RemovalReason) -> Bool {
67-
if let mapView = map.mapView {
68-
removeCustomLocationProvider(mapView: mapView)
69-
}
63+
64+
public func removeFromMap(_ map: RNMBXMapView, mapView: MapView, reason: RemovalReason) -> Bool {
65+
removeCustomLocationProvider(mapView: mapView)
7066
self.map = nil
7167
return true
7268
}

ios/RNMBX/RNMBXInteractiveElement.swift

Lines changed: 22 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,63 +1,70 @@
11
import MapboxMaps
22

33
@objc
4-
public class RNMBXInteractiveElement : UIView, RNMBXMapComponent {
4+
public class RNMBXInteractiveElement : UIView, RNMBXMapAndMapViewComponent {
55
weak var map : RNMBXMapView? = nil
6+
weak var mapView : MapView? = nil
67

78
static let hitboxDefault = 44.0
89

910
@objc public var draggable: Bool = false
10-
11+
1112
@objc public var hasPressListener: Bool = false
12-
13+
1314
@objc public var hitbox : [String:NSNumber] = [
1415
"width": NSNumber(value: hitboxDefault),
1516
"height": NSNumber(value: hitboxDefault)
1617
]
17-
18+
1819
@objc public var id: String! = nil {
1920
willSet {
2021
if id != nil && newValue != id {
2122
Logger.log(level:.warn, message: "Changing id from: \(optional: id) to \(optional: newValue), changing of id is not supported")
22-
if let map = map { removeFromMap(map, reason: .ComponentChange) }
23+
if let map = map, let mapView = mapView {
24+
removeFromMap(map, mapView: mapView, reason: .ComponentChange)
25+
}
2326
}
2427
}
2528
didSet {
2629
if oldValue != nil && oldValue != id {
27-
if let map = map { addToMap(map, style: map.mapboxMap.style) }
30+
if let map = map, let mapView = mapView {
31+
addToMap(map, mapView: mapView, style: mapView.mapboxMap.style)
32+
}
2833
}
2934
}
3035
}
31-
36+
3237
@objc public var onDragStart: RCTBubblingEventBlock? = nil
33-
38+
3439
@objc public var onPress: RCTBubblingEventBlock? = nil
35-
40+
3641
func getLayerIDs() -> [String] {
3742
return []
3843
}
3944

4045
func isDraggable() -> Bool {
4146
return draggable
4247
}
43-
48+
4449
func isTouchable() -> Bool {
4550
return hasPressListener
4651
}
47-
48-
// MARK: - RNMBXMapComponent
49-
public func addToMap(_ map: RNMBXMapView, style: Style) {
52+
53+
// MARK: - RNMBXMapAndMapViewComponent
54+
public func addToMap(_ map: RNMBXMapView, mapView: MapView, style: Style) {
5055
if (self.id == nil) {
5156
Logger.log(level: .error, message: "id is required on \(self) but not specified")
5257
}
5358
self.map = map
59+
self.mapView = mapView
5460
}
5561

56-
public func removeFromMap(_ map: RNMBXMapView, reason: RemovalReason) -> Bool {
62+
public func removeFromMap(_ map: RNMBXMapView, mapView: MapView, reason: RemovalReason) -> Bool {
5763
self.map = nil
64+
self.mapView = nil
5865
return true
5966
}
60-
67+
6168
public func waitForStyleLoad() -> Bool {
6269
return true
6370
}

0 commit comments

Comments
 (0)