From 72b8c38e9c79afc327c11f0b42262acd51ccf95c Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 8 Nov 2025 19:19:39 +0000 Subject: [PATCH 1/7] 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 --- ios/RNMBX/RNMBXCamera.swift | 76 +++++++++++++++++---- ios/RNMBX/RNMBXCustomLocationProvider.swift | 38 +++++------ ios/RNMBX/RNMBXInteractiveElement.swift | 37 ++++++---- ios/RNMBX/RNMBXMapView.swift | 74 ++++++++++++++++++-- ios/RNMBX/RNMBXNativeUserLocation.swift | 17 ++--- ios/RNMBX/RNMBXPointAnnotation.swift | 14 ++-- ios/RNMBX/RNMBXSource.swift | 34 ++++----- ios/RNMBX/RNMBXViewport.swift | 26 +++---- 8 files changed, 214 insertions(+), 102 deletions(-) diff --git a/ios/RNMBX/RNMBXCamera.swift b/ios/RNMBX/RNMBXCamera.swift index 69c7e2777a..8626ca0f26 100644 --- a/ios/RNMBX/RNMBXCamera.swift +++ b/ios/RNMBX/RNMBXCamera.swift @@ -18,7 +18,17 @@ public enum RemovalReason { public protocol RNMBXMapComponent: AnyObject { func addToMap(_ map: RNMBXMapView, style: Style) func removeFromMap(_ map: RNMBXMapView, reason: RemovalReason) -> Bool - + + func waitForStyleLoad() -> Bool +} + +/// Protocol for components that require a valid MapView instance for both add and remove operations. +/// Use this protocol when your component needs to interact with the native MapView directly. +/// The MapView parameter is guaranteed to be non-nil when these methods are called. +public protocol RNMBXMapAndMapViewComponent: AnyObject { + func addToMap(_ map: RNMBXMapView, mapView: MapView, style: Style) + func removeFromMap(_ map: RNMBXMapView, mapView: MapView, reason: RemovalReason) -> Bool + func waitForStyleLoad() -> Bool } @@ -85,7 +95,7 @@ class CameraUpdateQueue { open class RNMBXMapComponentBase : UIView, RNMBXMapComponent { private weak var _map: RNMBXMapView! = nil private var _mapCallbacks: [(RNMBXMapView) -> Void] = [] - + weak var map : RNMBXMapView? { return _map; } @@ -103,11 +113,11 @@ open class RNMBXMapComponentBase : UIView, RNMBXMapComponent { _mapCallbacks.append(callback) } } - + public func waitForStyleLoad() -> Bool { return false } - + public func addToMap(_ map: RNMBXMapView, style: Style) { _mapCallbacks.forEach { callback in callback(map) @@ -115,7 +125,7 @@ open class RNMBXMapComponentBase : UIView, RNMBXMapComponent { _mapCallbacks = [] _map = map } - + public func removeFromMap(_ map: RNMBXMapView, reason: RemovalReason) -> Bool { _mapCallbacks = [] _map = nil @@ -123,8 +133,50 @@ open class RNMBXMapComponentBase : UIView, RNMBXMapComponent { } } +/// Base class for components that require MapView to be non-nil +open class RNMBXMapAndMapViewComponentBase : UIView, RNMBXMapAndMapViewComponent { + private weak var _map: RNMBXMapView! = nil + private var _mapCallbacks: [(RNMBXMapView) -> Void] = [] + + weak var map : RNMBXMapView? { + return _map; + } + + func withMapView(_ callback: @escaping (_ mapView: MapView) -> Void) { + withRNMBXMapView { mapView in + callback(mapView.mapView) + } + } + + func withRNMBXMapView(_ callback: @escaping (_ map: RNMBXMapView) -> Void) { + if let map = _map { + callback(map) + } else { + _mapCallbacks.append(callback) + } + } + + public func waitForStyleLoad() -> Bool { + return false + } + + public func addToMap(_ map: RNMBXMapView, mapView: MapView, style: Style) { + _mapCallbacks.forEach { callback in + callback(map) + } + _mapCallbacks = [] + _map = map + } + + public func removeFromMap(_ map: RNMBXMapView, mapView: MapView, reason: RemovalReason) -> Bool { + _mapCallbacks = [] + _map = nil + return true + } +} + @objc(RNMBXCamera) -open class RNMBXCamera : RNMBXMapComponentBase { +open class RNMBXCamera : RNMBXMapAndMapViewComponentBase { var cameraAnimator: BasicCameraAnimator? let cameraUpdateQueue = CameraUpdateQueue() @@ -519,18 +571,18 @@ open class RNMBXCamera : RNMBXMapComponentBase { _updateCamera() } - public override func addToMap(_ map: RNMBXMapView, style: Style) { - super.addToMap(map, style: style) + public override func addToMap(_ map: RNMBXMapView, mapView: MapView, style: Style) { + super.addToMap(map, mapView: mapView, style: style) map.reactCamera = self } - - public override func removeFromMap(_ map: RNMBXMapView, reason: RemovalReason) -> Bool { + + public override func removeFromMap(_ map: RNMBXMapView, mapView: MapView, reason: RemovalReason) -> Bool { if (reason == .StyleChange) { return false } - map.mapView.viewport.removeStatusObserver(self) - return super.removeFromMap(map, reason:reason) + mapView.viewport.removeStatusObserver(self) + return super.removeFromMap(map, mapView: mapView, reason: reason) } @objc public func moveBy(x: Double, y: Double, animationMode: NSNumber?, animationDuration: NSNumber?, resolve: @escaping RCTPromiseResolveBlock, reject: @escaping RCTPromiseRejectBlock) { diff --git a/ios/RNMBX/RNMBXCustomLocationProvider.swift b/ios/RNMBX/RNMBXCustomLocationProvider.swift index 248f0d394c..1a1cae8f9c 100644 --- a/ios/RNMBX/RNMBXCustomLocationProvider.swift +++ b/ios/RNMBX/RNMBXCustomLocationProvider.swift @@ -3,15 +3,15 @@ import MapboxMaps let TAG = "RNMBXCustomLocationProvider" @objc -public class RNMBXCustomLocationProvider: UIView, RNMBXMapComponent { +public class RNMBXCustomLocationProvider: UIView, RNMBXMapAndMapViewComponent { var map: RNMBXMapView? = nil - + let changes : PropertyChanges = PropertyChanges() - + enum Property: String { case coordinate case heading - + func apply(locationProvider: RNMBXCustomLocationProvider) { switch self { case .coordinate: locationProvider.applyCoordinate() @@ -19,54 +19,50 @@ public class RNMBXCustomLocationProvider: UIView, RNMBXMapComponent { } } } - + @objc public var coordinate: [Double] = [] { didSet { changed(.coordinate) } } - + @objc public var heading: NSNumber = 0.0 { didSet { changed(.heading) } } - + func changed(_ property: Property) { changes.add(name: property.rawValue, update: property.apply) } - + @objc override public func didSetProps(_ props: [String]) { if customLocationProvider != nil { changes.apply(self) } } - + var customLocationProvider: CustomLocationProvider? = nil #if RNMBX_11 #else var defaultLocationProvider: LocationProvider? #endif - public func addToMap(_ map: RNMBXMapView, style: Style) { + public func addToMap(_ map: RNMBXMapView, mapView: MapView, style: Style) { self.map = map - if let mapView = map.mapView { - installCustomeLocationProviderIfNeeded(mapView: mapView) - changes.apply(self) - } + installCustomeLocationProviderIfNeeded(mapView: mapView) + changes.apply(self) } - + private func applyCoordinate() { updateCoordinate(latitude: coordinate[1], longitude: coordinate[0]) } - + private func applyHeading() { updateHeading(heading: heading.doubleValue) } - - public func removeFromMap(_ map: RNMBXMapView, reason: RemovalReason) -> Bool { - if let mapView = map.mapView { - removeCustomLocationProvider(mapView: mapView) - } + + public func removeFromMap(_ map: RNMBXMapView, mapView: MapView, reason: RemovalReason) -> Bool { + removeCustomLocationProvider(mapView: mapView) self.map = nil return true } diff --git a/ios/RNMBX/RNMBXInteractiveElement.swift b/ios/RNMBX/RNMBXInteractiveElement.swift index 88b9a83f69..1f0f21c24a 100644 --- a/ios/RNMBX/RNMBXInteractiveElement.swift +++ b/ios/RNMBX/RNMBXInteractiveElement.swift @@ -1,38 +1,43 @@ import MapboxMaps @objc -public class RNMBXInteractiveElement : UIView, RNMBXMapComponent { +public class RNMBXInteractiveElement : UIView, RNMBXMapAndMapViewComponent { weak var map : RNMBXMapView? = nil + weak var mapView : MapView? = nil static let hitboxDefault = 44.0 @objc public var draggable: Bool = false - + @objc public var hasPressListener: Bool = false - + @objc public var hitbox : [String:NSNumber] = [ "width": NSNumber(value: hitboxDefault), "height": NSNumber(value: hitboxDefault) ] - + @objc public var id: String! = nil { willSet { if id != nil && newValue != id { Logger.log(level:.warn, message: "Changing id from: \(optional: id) to \(optional: newValue), changing of id is not supported") - if let map = map { removeFromMap(map, reason: .ComponentChange) } + if let map = map, let mapView = mapView { + removeFromMap(map, mapView: mapView, reason: .ComponentChange) + } } } didSet { if oldValue != nil && oldValue != id { - if let map = map { addToMap(map, style: map.mapboxMap.style) } + if let map = map, let mapView = mapView { + addToMap(map, mapView: mapView, style: mapView.mapboxMap.style) + } } } } - + @objc public var onDragStart: RCTBubblingEventBlock? = nil - + @objc public var onPress: RCTBubblingEventBlock? = nil - + func getLayerIDs() -> [String] { return [] } @@ -40,24 +45,26 @@ public class RNMBXInteractiveElement : UIView, RNMBXMapComponent { func isDraggable() -> Bool { return draggable } - + func isTouchable() -> Bool { return hasPressListener } - - // MARK: - RNMBXMapComponent - public func addToMap(_ map: RNMBXMapView, style: Style) { + + // MARK: - RNMBXMapAndMapViewComponent + public func addToMap(_ map: RNMBXMapView, mapView: MapView, style: Style) { if (self.id == nil) { Logger.log(level: .error, message: "id is required on \(self) but not specified") } self.map = map + self.mapView = mapView } - public func removeFromMap(_ map: RNMBXMapView, reason: RemovalReason) -> Bool { + public func removeFromMap(_ map: RNMBXMapView, mapView: MapView, reason: RemovalReason) -> Bool { self.map = nil + self.mapView = nil return true } - + public func waitForStyleLoad() -> Bool { return true } diff --git a/ios/RNMBX/RNMBXMapView.swift b/ios/RNMBX/RNMBXMapView.swift index bd59417ab3..c2abfeeefe 100644 --- a/ios/RNMBX/RNMBXMapView.swift +++ b/ios/RNMBX/RNMBXMapView.swift @@ -60,11 +60,11 @@ public class RNMBXMapViewFactory { } class FeatureEntry { - let feature: RNMBXMapComponent + let feature: AnyObject // Can be RNMBXMapComponent or RNMBXMapAndMapViewComponent let view: UIView var addedToMap: Bool = false - init(feature:RNMBXMapComponent, view: UIView, addedToMap: Bool = false) { + init(feature: AnyObject, view: UIView, addedToMap: Bool = false) { self.feature = feature self.view = view self.addedToMap = addedToMap @@ -269,7 +269,27 @@ open class RNMBXMapView: UIView, RCTInvalidating { @objc public func addToMap(_ subview: UIView) { withMapView { mapView in - if let mapComponent = subview as? RNMBXMapComponent { + // Check for RNMBXMapAndMapViewComponent first (requires MapView) + if let mapAndMapViewComponent = subview as? RNMBXMapAndMapViewComponent { + let style = mapView.mapboxMap.style + var addToMap = false + if mapAndMapViewComponent.waitForStyleLoad() { + if (self.styleLoadWaiters.hasInited()) { + addToMap = true + } + } else { + addToMap = true + } + + let entry = FeatureEntry(feature: mapAndMapViewComponent, view: subview, addedToMap: false) + if (addToMap) { + mapAndMapViewComponent.addToMap(self, mapView: mapView, style: style) + entry.addedToMap = true + } + self.features.append(entry) + } + // Fallback to RNMBXMapComponent (doesn't require MapView) + else if let mapComponent = subview as? RNMBXMapComponent { let style = mapView.mapboxMap.style var addToMap = false if mapComponent.waitForStyleLoad() { @@ -296,7 +316,26 @@ open class RNMBXMapView: UIView, RCTInvalidating { } @objc public func removeFromMap(_ subview: UIView) { - if let mapComponent = subview as? RNMBXMapComponent { + // Check for RNMBXMapAndMapViewComponent first (requires MapView) + if let mapAndMapViewComponent = subview as? RNMBXMapAndMapViewComponent { + var entryIndex = features.firstIndex { $0.view == subview } + if let entryIndex = entryIndex { + var entry = features[entryIndex] + if (entry.addedToMap) { + // mapView should always be non-nil here if our invariants hold + guard let mapView = _mapView else { + Logger.error("RNMBXMapView.removeFromMap: CRITICAL - mapView is nil for component that requires it: \(type(of: subview))") + features.remove(at: entryIndex) + return + } + mapAndMapViewComponent.removeFromMap(self, mapView: mapView, reason: .OnDestroy) + entry.addedToMap = false + } + features.remove(at: entryIndex) + } + } + // Fallback to RNMBXMapComponent (doesn't require MapView) + else if let mapComponent = subview as? RNMBXMapComponent { var entryIndex = features.firstIndex { $0.view == subview } if let entryIndex = entryIndex { var entry = features[entryIndex] @@ -842,16 +881,39 @@ open class RNMBXMapView: UIView, RCTInvalidating { private func removeAllFeaturesFromMap(reason: RemovalReason) { features.forEach { entry in if (entry.addedToMap) { - entry.feature.removeFromMap(self, reason: reason) + // Handle RNMBXMapAndMapViewComponent + if let mapAndMapViewComponent = entry.feature as? RNMBXMapAndMapViewComponent { + guard let mapView = _mapView else { + Logger.error("RNMBXMapView.removeAllFeaturesFromMap: mapView is nil") + return + } + mapAndMapViewComponent.removeFromMap(self, mapView: mapView, reason: reason) + } + // Handle RNMBXMapComponent + else if let mapComponent = entry.feature as? RNMBXMapComponent { + mapComponent.removeFromMap(self, reason: reason) + } entry.addedToMap = false } } } private func addFeaturesToMap(style: Style) { + guard let mapView = _mapView else { + Logger.error("RNMBXMapView.addFeaturesToMap: mapView is nil") + return + } + features.forEach { entry in if (!entry.addedToMap) { - entry.feature.addToMap(self, style: style) + // Handle RNMBXMapAndMapViewComponent + if let mapAndMapViewComponent = entry.feature as? RNMBXMapAndMapViewComponent { + mapAndMapViewComponent.addToMap(self, mapView: mapView, style: style) + } + // Handle RNMBXMapComponent + else if let mapComponent = entry.feature as? RNMBXMapComponent { + mapComponent.addToMap(self, style: style) + } entry.addedToMap = true } } diff --git a/ios/RNMBX/RNMBXNativeUserLocation.swift b/ios/RNMBX/RNMBXNativeUserLocation.swift index 534e2154c2..51f322ea20 100644 --- a/ios/RNMBX/RNMBXNativeUserLocation.swift +++ b/ios/RNMBX/RNMBXNativeUserLocation.swift @@ -1,7 +1,7 @@ import MapboxMaps @objc -public class RNMBXNativeUserLocation: UIView, RNMBXMapComponent { +public class RNMBXNativeUserLocation: UIView, RNMBXMapAndMapViewComponent { weak var map : RNMBXMapView! = nil var imageManager: ImageManager? = nil @@ -200,20 +200,17 @@ public class RNMBXNativeUserLocation: UIView, RNMBXMapComponent { } } - public func addToMap(_ map: RNMBXMapView, style: Style) { + public func addToMap(_ map: RNMBXMapView, mapView: MapView, style: Style) { self.map = map - + _fetchImages(map) _apply() } - public func removeFromMap(_ map: RNMBXMapView, reason: RemovalReason) -> Bool { - if let location = map.mapView.location { - location.options.puckType = nil - location.options.puckType = .none - } else { - Logger.error("RNMBXNativeUserLocation.removeFromMap: location is nil") - } + public func removeFromMap(_ map: RNMBXMapView, mapView: MapView, reason: RemovalReason) -> Bool { + let location = mapView.location + location.options.puckType = nil + location.options.puckType = .none removeSubscriptions() self.map = nil diff --git a/ios/RNMBX/RNMBXPointAnnotation.swift b/ios/RNMBX/RNMBXPointAnnotation.swift index fbe524cca8..ce2254a2d5 100644 --- a/ios/RNMBX/RNMBXPointAnnotation.swift +++ b/ios/RNMBX/RNMBXPointAnnotation.swift @@ -248,18 +248,16 @@ public class RNMBXPointAnnotation : RNMBXInteractiveElement { } } - // MARK: - RNMBXMapComponent - - public override func addToMap(_ map: RNMBXMapView, style: Style) { - super.addToMap(map, style: style) - self.map = map + // MARK: - RNMBXMapAndMapViewComponent + + public override func addToMap(_ map: RNMBXMapView, mapView: MapView, style: Style) { + super.addToMap(map, mapView: mapView, style: style) addIfPossible() } - public override func removeFromMap(_ map: RNMBXMapView, reason: RemovalReason) -> Bool { + public override func removeFromMap(_ map: RNMBXMapView, mapView: MapView, reason: RemovalReason) -> Bool { removeIfAdded() - self.map = nil - return true + return super.removeFromMap(map, mapView: mapView, reason: reason) } // MARK: - RNMBXInteractiveElement diff --git a/ios/RNMBX/RNMBXSource.swift b/ios/RNMBX/RNMBXSource.swift index 284fdfe4ef..4d5845284d 100644 --- a/ios/RNMBX/RNMBXSource.swift +++ b/ios/RNMBX/RNMBXSource.swift @@ -38,27 +38,27 @@ public class RNMBXSource : RNMBXInteractiveElement { @objc public func insertReactSubviewInternal(_ subview: UIView!, at atIndex: Int) { if let layer = subview as? RNMBXSourceConsumer { - if let map = map { - layer.addToMap(map, style: map.mapboxMap.style) + if let map = map, let mapView = mapView { + layer.addToMap(map, style: mapView.mapboxMap.style) } layers.append(layer) } else if let component = subview as? RNMBXMapComponent { - if let map = map { - component.addToMap(map, style: map.mapboxMap.style) + if let map = map, let mapView = mapView { + component.addToMap(map, style: mapView.mapboxMap.style) } components.append(component) } } - + @objc public override func removeReactSubview(_ subview: UIView!) { removeReactSubviewInternal(subview) super.removeReactSubview(subview) } - + @objc public func removeReactSubviewInternal(_ subview: UIView!) { if let layer : RNMBXSourceConsumer = subview as? RNMBXSourceConsumer { - if let map = map { - layer.removeFromMap(map, style: map.mapboxMap.style) + if let map = map, let mapView = mapView { + layer.removeFromMap(map, style: mapView.mapboxMap.style) } layers.removeAll { $0 as AnyObject === layer } } else if let component = subview as? RNMBXMapComponent { @@ -75,9 +75,9 @@ public class RNMBXSource : RNMBXInteractiveElement { } // MARK: - RNMBXInteractiveElement - - public override func addToMap(_ map: RNMBXMapView, style: Style) { - self.map = map + + public override func addToMap(_ map: RNMBXMapView, mapView: MapView, style: Style) { + super.addToMap(map, mapView: mapView, style: style) if style.sourceExists(withId: self.id) { if (!existing) { @@ -101,22 +101,22 @@ public class RNMBXSource : RNMBXInteractiveElement { } for layer in self.layers { - layer.addToMap(map, style: map.mapboxMap.style) + layer.addToMap(map, style: style) } for component in self.components { - component.addToMap(map, style: map.mapboxMap.style) + component.addToMap(map, style: style) } } - public override func removeFromMap(_ map: RNMBXMapView, reason: RemovalReason) -> Bool { - self.map = nil + public override func removeFromMap(_ map: RNMBXMapView, mapView: MapView, reason: RemovalReason) -> Bool { + super.removeFromMap(map, mapView: mapView, reason: reason) for layer in self.layers { - layer.removeFromMap(map, style: map.mapboxMap.style) + layer.removeFromMap(map, style: mapView.mapboxMap.style) } if self.ownsSource { - let style = map.mapboxMap.style + let style = mapView.mapboxMap.style logged("StyleSource.removeFromMap", info: { "id: \(optional: self.id)"}) { try style.removeSource(withId: id) } diff --git a/ios/RNMBX/RNMBXViewport.swift b/ios/RNMBX/RNMBXViewport.swift index c0b4173941..af0e75cadd 100644 --- a/ios/RNMBX/RNMBXViewport.swift +++ b/ios/RNMBX/RNMBXViewport.swift @@ -6,13 +6,13 @@ typealias ViewportManager = Viewport #endif @objc(RNMBXViewport) -open class RNMBXViewport : UIView, RNMBXMapComponent, ViewportStatusObserver { +open class RNMBXViewport : UIView, RNMBXMapAndMapViewComponent, ViewportStatusObserver { var mapView: MapView? = nil - + // MARK: React properties @objc public var onStatusChanged: RCTBubblingEventBlock? = nil - + @objc public var hasStatusChanged: Bool = false { didSet { @@ -21,7 +21,7 @@ open class RNMBXViewport : UIView, RNMBXMapComponent, ViewportStatusObserver { } } } - + func applyHasStatusChanged(mapView: MapView) { if (hasStatusChanged) { mapView.viewport.addStatusObserver(self) @@ -29,7 +29,7 @@ open class RNMBXViewport : UIView, RNMBXMapComponent, ViewportStatusObserver { mapView.viewport.removeStatusObserver(self) } } - + public func viewportStatusDidChange(from fromStatus: ViewportStatus, to toStatus: ViewportStatus, reason: ViewportStatusChangeReason) @@ -42,7 +42,7 @@ open class RNMBXViewport : UIView, RNMBXMapComponent, ViewportStatusObserver { "reason": reasonToString(reason) ]]) } - + @objc public var transitionsToIdleUponUserInteraction: NSNumber? = nil { didSet { @@ -51,20 +51,20 @@ open class RNMBXViewport : UIView, RNMBXMapComponent, ViewportStatusObserver { } } } - + public func waitForStyleLoad() -> Bool { true } - public func addToMap(_ map: RNMBXMapView, style: Style) { - mapView = map.mapView - applyHasStatusChanged(mapView: mapView!) - apply(mapView: map.mapView) + public func addToMap(_ map: RNMBXMapView, mapView: MapView, style: Style) { + self.mapView = mapView + applyHasStatusChanged(mapView: mapView) + apply(mapView: mapView) } - public func removeFromMap(_ map: RNMBXMapView, reason: RemovalReason) -> Bool { + public func removeFromMap(_ map: RNMBXMapView, mapView: MapView, reason: RemovalReason) -> Bool { if (hasStatusChanged) { - map.mapView.viewport.removeStatusObserver(self) + mapView.viewport.removeStatusObserver(self) } self.mapView = nil return true From 9ba182ab032a18dc906020697d880ea968992e25 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 8 Nov 2025 20:09:05 +0000 Subject: [PATCH 2/7] refactor(ios): improve protocol hierarchy with inheritance and default implementations Refinements to the RNMBXMapComponent protocol architecture: 1. Extracted common protocol requirement into base protocol: - Created RNMBXMapComponentProtocol with waitForStyleLoad() method - Both RNMBXMapComponent and RNMBXMapAndMapViewComponent inherit from it - Eliminates duplication of waitForStyleLoad() in both protocols 2. Added default implementation via Swift protocol extension: - RNMBXMapComponentProtocol extension provides default waitForStyleLoad() -> false - Components that need style load can override, others use default - Removed redundant implementations from: * RNMBXMapComponentBase * RNMBXMapAndMapViewComponentBase * RNMBXCustomLocationProvider * RNMBXImages Benefits: - DRY: Single definition of waitForStyleLoad() requirement - Convenience: Default implementation reduces boilerplate - Clarity: Explicit protocol inheritance shows relationship - Type safety: Compiler enforces common interface This follows Swift best practices for protocol-oriented programming. --- ios/RNMBX/RNMBXCamera.swift | 29 ++++++++++++--------- ios/RNMBX/RNMBXCustomLocationProvider.swift | 4 +-- ios/RNMBX/RNMBXImages.swift | 5 +--- 3 files changed, 19 insertions(+), 19 deletions(-) diff --git a/ios/RNMBX/RNMBXCamera.swift b/ios/RNMBX/RNMBXCamera.swift index 8626ca0f26..b49c5515c5 100644 --- a/ios/RNMBX/RNMBXCamera.swift +++ b/ios/RNMBX/RNMBXCamera.swift @@ -15,21 +15,30 @@ public enum RemovalReason { case ViewRemoval, StyleChange, OnDestroy, ComponentChange, Reorder } -public protocol RNMBXMapComponent: AnyObject { +/// Base protocol for all map components +public protocol RNMBXMapComponentProtocol: AnyObject { + func waitForStyleLoad() -> Bool +} + +/// Default implementation: most components don't need to wait for style load +extension RNMBXMapComponentProtocol { + public func waitForStyleLoad() -> Bool { + return false + } +} + +/// Protocol for components that can work without direct MapView access +public protocol RNMBXMapComponent: RNMBXMapComponentProtocol { func addToMap(_ map: RNMBXMapView, style: Style) func removeFromMap(_ map: RNMBXMapView, reason: RemovalReason) -> Bool - - func waitForStyleLoad() -> Bool } /// Protocol for components that require a valid MapView instance for both add and remove operations. /// Use this protocol when your component needs to interact with the native MapView directly. /// The MapView parameter is guaranteed to be non-nil when these methods are called. -public protocol RNMBXMapAndMapViewComponent: AnyObject { +public protocol RNMBXMapAndMapViewComponent: RNMBXMapComponentProtocol { func addToMap(_ map: RNMBXMapView, mapView: MapView, style: Style) func removeFromMap(_ map: RNMBXMapView, mapView: MapView, reason: RemovalReason) -> Bool - - func waitForStyleLoad() -> Bool } enum CameraMode: Int { @@ -114,9 +123,7 @@ open class RNMBXMapComponentBase : UIView, RNMBXMapComponent { } } - public func waitForStyleLoad() -> Bool { - return false - } + // Uses default implementation from RNMBXMapComponentProtocol extension public func addToMap(_ map: RNMBXMapView, style: Style) { _mapCallbacks.forEach { callback in @@ -156,9 +163,7 @@ open class RNMBXMapAndMapViewComponentBase : UIView, RNMBXMapAndMapViewComponent } } - public func waitForStyleLoad() -> Bool { - return false - } + // Uses default implementation from RNMBXMapComponentProtocol extension public func addToMap(_ map: RNMBXMapView, mapView: MapView, style: Style) { _mapCallbacks.forEach { callback in diff --git a/ios/RNMBX/RNMBXCustomLocationProvider.swift b/ios/RNMBX/RNMBXCustomLocationProvider.swift index 1a1cae8f9c..95b37072a8 100644 --- a/ios/RNMBX/RNMBXCustomLocationProvider.swift +++ b/ios/RNMBX/RNMBXCustomLocationProvider.swift @@ -67,9 +67,7 @@ public class RNMBXCustomLocationProvider: UIView, RNMBXMapAndMapViewComponent { return true } - public func waitForStyleLoad() -> Bool { - false - } + // Uses default implementation from RNMBXMapComponentProtocol extension (returns false) } #if RNMBX_11 diff --git a/ios/RNMBX/RNMBXImages.swift b/ios/RNMBX/RNMBXImages.swift index a59d8db597..c585492065 100644 --- a/ios/RNMBX/RNMBXImages.swift +++ b/ios/RNMBX/RNMBXImages.swift @@ -74,11 +74,8 @@ open class RNMBXImages : UIView, RNMBXMapComponent { } // MARK: - RNMBXMapComponent + // Uses default implementation from RNMBXMapComponentProtocol extension (returns false) - public func waitForStyleLoad() -> Bool { - return false - } - public func addToMap(_ map: RNMBXMapView, style: Style) { self.style = style imageManager = map.imageManager From 7f342f5b8c2a25f86ecd5ea653f87d291ff44e81 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 8 Nov 2025 20:13:32 +0000 Subject: [PATCH 3/7] refactor(ios): make RNMBXMapAndMapViewComponent inherit from RNMBXMapComponent Changed RNMBXMapAndMapViewComponent to inherit from RNMBXMapComponent instead of RNMBXMapComponentProtocol, establishing a proper subtype relationship. Key Changes: 1. Protocol Inheritance: - RNMBXMapAndMapViewComponent now extends RNMBXMapComponent - This makes semantic sense: "a component that needs MapView IS A map component" - Ensures compatibility with existing code that checks for RNMBXMapComponent 2. Default Implementation Safety Guard: - Added protocol extension with default implementations of base protocol methods - These implementations log errors and trigger assertions if called - Forces developers to use the mapView-aware versions - Catches mistakes at runtime (or compile time in debug builds) 3. Updated RNMBXSource to Handle Both Protocols: - Changed components array type to RNMBXMapComponentProtocol (base protocol) - Check for more specific protocol first (RNMBXMapAndMapViewComponent) - Then fallback to RNMBXMapComponent for components that don't need mapView - Applied this pattern to insertReactSubviewInternal, removeReactSubviewInternal, addToMap, and removeFromMap methods Benefits: - Type system correctly models the relationship (MapAndMapViewComponent IS A MapComponent) - Existing code that checks "as? RNMBXMapComponent" still works (matches both types) - Safety: Can't accidentally call wrong method signature due to default implementations - Future-proof: New code can store both types in arrays typed as RNMBXMapComponent RNMBXMapView already checks for the more specific protocol first, so no changes needed there - the inheritance makes the code more correct without breaking anything. --- ios/RNMBX/RNMBXCamera.swift | 20 ++++++++++++++++++- ios/RNMBX/RNMBXSource.swift | 40 ++++++++++++++++++++++++++++++++----- 2 files changed, 54 insertions(+), 6 deletions(-) diff --git a/ios/RNMBX/RNMBXCamera.swift b/ios/RNMBX/RNMBXCamera.swift index b49c5515c5..a9fc85f635 100644 --- a/ios/RNMBX/RNMBXCamera.swift +++ b/ios/RNMBX/RNMBXCamera.swift @@ -36,11 +36,29 @@ public protocol RNMBXMapComponent: RNMBXMapComponentProtocol { /// Protocol for components that require a valid MapView instance for both add and remove operations. /// Use this protocol when your component needs to interact with the native MapView directly. /// The MapView parameter is guaranteed to be non-nil when these methods are called. -public protocol RNMBXMapAndMapViewComponent: RNMBXMapComponentProtocol { +/// +/// This protocol inherits from RNMBXMapComponent to ensure compatibility with existing code, +/// but provides default implementations of the base protocol methods that throw errors, +/// forcing implementers to use the mapView-aware versions. +public protocol RNMBXMapAndMapViewComponent: RNMBXMapComponent { func addToMap(_ map: RNMBXMapView, mapView: MapView, style: Style) func removeFromMap(_ map: RNMBXMapView, mapView: MapView, reason: RemovalReason) -> Bool } +/// Default implementations for RNMBXMapAndMapViewComponent that prevent accidental use of base protocol methods +extension RNMBXMapAndMapViewComponent { + public func addToMap(_ map: RNMBXMapView, style: Style) { + Logger.error("CRITICAL: addToMap(_:style:) called on RNMBXMapAndMapViewComponent. Use addToMap(_:mapView:style:) instead. Component: \(type(of: self))") + assertionFailure("RNMBXMapAndMapViewComponent must use addToMap(_:mapView:style:) instead") + } + + public func removeFromMap(_ map: RNMBXMapView, reason: RemovalReason) -> Bool { + Logger.error("CRITICAL: removeFromMap(_:reason:) called on RNMBXMapAndMapViewComponent. Use removeFromMap(_:mapView:reason:) instead. Component: \(type(of: self))") + assertionFailure("RNMBXMapAndMapViewComponent must use removeFromMap(_:mapView:reason:) instead") + return false + } +} + enum CameraMode: Int { case flight = 1 case ease = 2 diff --git a/ios/RNMBX/RNMBXSource.swift b/ios/RNMBX/RNMBXSource.swift index 4d5845284d..2018719e8c 100644 --- a/ios/RNMBX/RNMBXSource.swift +++ b/ios/RNMBX/RNMBXSource.swift @@ -3,7 +3,7 @@ @objc public class RNMBXSource : RNMBXInteractiveElement { var layers: [RNMBXSourceConsumer] = [] - var components: [RNMBXMapComponent] = [] + var components: [RNMBXMapComponentProtocol] = [] // Use base protocol to store both types var source : Source? = nil @@ -42,7 +42,15 @@ public class RNMBXSource : RNMBXInteractiveElement { layer.addToMap(map, style: mapView.mapboxMap.style) } layers.append(layer) - } else if let component = subview as? RNMBXMapComponent { + } + // Check for more specific protocol first (RNMBXMapAndMapViewComponent is a subtype of RNMBXMapComponent) + else if let mapAndMapViewComponent = subview as? RNMBXMapAndMapViewComponent { + if let map = map, let mapView = mapView { + mapAndMapViewComponent.addToMap(map, mapView: mapView, style: mapView.mapboxMap.style) + } + components.append(mapAndMapViewComponent) + } + else if let component = subview as? RNMBXMapComponent { if let map = map, let mapView = mapView { component.addToMap(map, style: mapView.mapboxMap.style) } @@ -61,11 +69,19 @@ public class RNMBXSource : RNMBXInteractiveElement { layer.removeFromMap(map, style: mapView.mapboxMap.style) } layers.removeAll { $0 as AnyObject === layer } - } else if let component = subview as? RNMBXMapComponent { + } + // Check for more specific protocol first (RNMBXMapAndMapViewComponent is a subtype of RNMBXMapComponent) + else if let mapAndMapViewComponent = subview as? RNMBXMapAndMapViewComponent { + if let map = map, let mapView = mapView { + mapAndMapViewComponent.removeFromMap(map, mapView: mapView, reason: .ViewRemoval) + } + components.removeAll { $0 as AnyObject === mapAndMapViewComponent } + } + else if let component = subview as? RNMBXMapComponent { if let map = map { component.removeFromMap(map, reason: .ViewRemoval) } - layers.removeAll { $0 as AnyObject === component } + components.removeAll { $0 as AnyObject === component } } } @@ -104,7 +120,12 @@ public class RNMBXSource : RNMBXInteractiveElement { layer.addToMap(map, style: style) } for component in self.components { - component.addToMap(map, style: style) + // Check for more specific protocol first + if let mapAndMapViewComponent = component as? RNMBXMapAndMapViewComponent { + mapAndMapViewComponent.addToMap(map, mapView: mapView, style: style) + } else if let mapComponent = component as? RNMBXMapComponent { + mapComponent.addToMap(map, style: style) + } } } @@ -115,6 +136,15 @@ public class RNMBXSource : RNMBXInteractiveElement { layer.removeFromMap(map, style: mapView.mapboxMap.style) } + for component in self.components { + // Check for more specific protocol first + if let mapAndMapViewComponent = component as? RNMBXMapAndMapViewComponent { + mapAndMapViewComponent.removeFromMap(map, mapView: mapView, reason: reason) + } else if let mapComponent = component as? RNMBXMapComponent { + mapComponent.removeFromMap(map, reason: reason) + } + } + if self.ownsSource { let style = mapView.mapboxMap.style logged("StyleSource.removeFromMap", info: { "id: \(optional: self.id)"}) { From 7f2d4da5557621d13cf7abbe597dec3cfaff7b93 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mikl=C3=B3s=20Fazekas?= Date: Sun, 9 Nov 2025 11:01:21 +0100 Subject: [PATCH 4/7] ios: build fix --- ios/RNMBX/RNMBXNativeUserLocation.swift | 9 ++++++--- ios/RNMBX/RNMBXShapeSource.swift | 9 +++++---- 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/ios/RNMBX/RNMBXNativeUserLocation.swift b/ios/RNMBX/RNMBXNativeUserLocation.swift index 51f322ea20..a0473d16c8 100644 --- a/ios/RNMBX/RNMBXNativeUserLocation.swift +++ b/ios/RNMBX/RNMBXNativeUserLocation.swift @@ -208,9 +208,12 @@ public class RNMBXNativeUserLocation: UIView, RNMBXMapAndMapViewComponent { } public func removeFromMap(_ map: RNMBXMapView, mapView: MapView, reason: RemovalReason) -> Bool { - let location = mapView.location - location.options.puckType = nil - location.options.puckType = .none + if let location = mapView.location { + location.options.puckType = nil + location.options.puckType = .none + } else { + Logger.error("[RNMBXNativeUserLocation] removeFromMap, location is null") + } removeSubscriptions() self.map = nil diff --git a/ios/RNMBX/RNMBXShapeSource.swift b/ios/RNMBX/RNMBXShapeSource.swift index fcabaf3fd1..c6e993d4c4 100644 --- a/ios/RNMBX/RNMBXShapeSource.swift +++ b/ios/RNMBX/RNMBXShapeSource.swift @@ -55,15 +55,16 @@ public class RNMBXShapeSource : RNMBXSource { } } - public override func addToMap(_ map: RNMBXMapView, style: Style) { - super.addToMap(map, style: style) + + public override func addToMap(_ map: RNMBXMapView, mapView: MapView, style: Style) { + super.addToMap(map, mapView:mapView, style: style) } - public override func removeFromMap(_ map: RNMBXMapView, reason: RemovalReason) -> Bool { + public override func removeFromMap(_ map: RNMBXMapView, mapView: MapView, reason: RemovalReason) -> Bool { if (reason == .ViewRemoval) { shapeAnimator?.unsubscribe(consumer: self) } - return super.removeFromMap(map, reason: reason) + return super.removeFromMap(map, mapView:mapView, reason: reason) } @objc public var cluster : NSNumber? From 41a08d1a79f1ed10d94881a458cfd28ed83742be Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mikl=C3=B3s=20Fazekas?= Date: Sun, 9 Nov 2025 11:18:31 +0100 Subject: [PATCH 5/7] Update ios/RNMBX/RNMBXCamera.swift --- ios/RNMBX/RNMBXCamera.swift | 1 - 1 file changed, 1 deletion(-) diff --git a/ios/RNMBX/RNMBXCamera.swift b/ios/RNMBX/RNMBXCamera.swift index a9fc85f635..b1bad2f588 100644 --- a/ios/RNMBX/RNMBXCamera.swift +++ b/ios/RNMBX/RNMBXCamera.swift @@ -49,7 +49,6 @@ public protocol RNMBXMapAndMapViewComponent: RNMBXMapComponent { extension RNMBXMapAndMapViewComponent { public func addToMap(_ map: RNMBXMapView, style: Style) { Logger.error("CRITICAL: addToMap(_:style:) called on RNMBXMapAndMapViewComponent. Use addToMap(_:mapView:style:) instead. Component: \(type(of: self))") - assertionFailure("RNMBXMapAndMapViewComponent must use addToMap(_:mapView:style:) instead") } public func removeFromMap(_ map: RNMBXMapView, reason: RemovalReason) -> Bool { From b61fd875690e195fad1759c5310e4d7a93b55161 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mikl=C3=B3s=20Fazekas?= Date: Sun, 9 Nov 2025 11:18:57 +0100 Subject: [PATCH 6/7] Update ios/RNMBX/RNMBXCamera.swift --- ios/RNMBX/RNMBXCamera.swift | 1 - 1 file changed, 1 deletion(-) diff --git a/ios/RNMBX/RNMBXCamera.swift b/ios/RNMBX/RNMBXCamera.swift index b1bad2f588..0414b0c86f 100644 --- a/ios/RNMBX/RNMBXCamera.swift +++ b/ios/RNMBX/RNMBXCamera.swift @@ -53,7 +53,6 @@ extension RNMBXMapAndMapViewComponent { public func removeFromMap(_ map: RNMBXMapView, reason: RemovalReason) -> Bool { Logger.error("CRITICAL: removeFromMap(_:reason:) called on RNMBXMapAndMapViewComponent. Use removeFromMap(_:mapView:reason:) instead. Component: \(type(of: self))") - assertionFailure("RNMBXMapAndMapViewComponent must use removeFromMap(_:mapView:reason:) instead") return false } } From 6c04cee6c2f852e4bf2d6ccbe7583a1fc60570e8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mikl=C3=B3s=20Fazekas?= Date: Sun, 9 Nov 2025 11:34:36 +0100 Subject: [PATCH 7/7] Update ios/RNMBX/RNMBXCamera.swift --- ios/RNMBX/RNMBXCamera.swift | 2 -- 1 file changed, 2 deletions(-) diff --git a/ios/RNMBX/RNMBXCamera.swift b/ios/RNMBX/RNMBXCamera.swift index 0414b0c86f..e51c8b663d 100644 --- a/ios/RNMBX/RNMBXCamera.swift +++ b/ios/RNMBX/RNMBXCamera.swift @@ -139,8 +139,6 @@ open class RNMBXMapComponentBase : UIView, RNMBXMapComponent { } } - // Uses default implementation from RNMBXMapComponentProtocol extension - public func addToMap(_ map: RNMBXMapView, style: Style) { _mapCallbacks.forEach { callback in callback(map)