From edc195f03a67da5b4c39d29b3cd8bf040081e890 Mon Sep 17 00:00:00 2001 From: Jakub Kasprzyk Date: Sun, 20 Sep 2026 18:07:24 +0200 Subject: [PATCH 1/6] fix: buffer MapViewRef calls until the native map exists Every MapViewRef method rejected with "MapView is not mounted" when it was called from a consumer's mount effect. Nitro delivers the hybridRef view prop one JS -> UI -> JS round trip after the commit that mounts the view, so the handle React publishes during that commit has nothing behind it yet, and the camera silently never moved. MapViewCommands buffers calls made in that window and replays them in the order they were made as soon as hybridRef arrives. Calls still buffered when the view unmounts reject, and calls made afterwards reject from JS rather than reaching a released native object. Android needed a second buffer. MapView.getMapAsync answers later than the Nitro view becomes reachable, and until then the adapter's camera methods returned early on a null GoogleMap while HybridMapView had already resolved the promise, so flushing the JS buffer alone would have turned a loud rejection into a silent success. DeferredGoogleMap holds that work until the map exists, configureMap drains it after replaying the region/camera props, and the three camera commands now resolve when they reach the map instead of immediately. fetchCamera and getVisibleRegion no longer answer with a placeholder camera or an all-zero region. iOS needs no change: its adapter owns a map view from the moment it is installed. Closes #131 --- README.md | 35 ++++ docs/architecture.md | 21 +++ example/App.tsx | 90 +++++++-- example/examples/index.ts | 2 + example/examples/mountEffectCamera.ts | 47 +++++ example/examples/types.ts | 1 + .../nitro/nitromaps/DeferredGoogleMap.kt | 74 ++++++++ .../nitromaps/GoogleMapProviderAdapter.kt | 136 +++++--------- .../margelo/nitro/nitromaps/HybridMapView.kt | 9 +- .../nitro/nitromaps/MapProviderAdapter.kt | 6 +- .../com/margelo/nitro/nitromaps/RunOnMain.kt | 16 ++ package/src/components/MapView.tsx | 65 ++++--- .../native/__tests__/mapViewCommands.test.ts | 172 ++++++++++++++++++ package/src/native/mapViewCommands.ts | 98 ++++++++++ package/src/types/ref.ts | 31 +++- 15 files changed, 662 insertions(+), 141 deletions(-) create mode 100644 example/examples/mountEffectCamera.ts create mode 100644 package/android/src/main/java/com/margelo/nitro/nitromaps/DeferredGoogleMap.kt create mode 100644 package/android/src/main/java/com/margelo/nitro/nitromaps/RunOnMain.kt create mode 100644 package/src/native/__tests__/mapViewCommands.test.ts create mode 100644 package/src/native/mapViewCommands.ts diff --git a/README.md b/README.md index 1005466f..ec40c59b 100644 --- a/README.md +++ b/README.md @@ -252,6 +252,40 @@ function ControlledMap() { } ``` +#### When the ref is usable + +The native map is created after React commits, so `mapRef.current` is populated +before there is anything native behind it. Calls made in that window are held +and replayed, in the order they were made, as soon as the native map exists: + +```tsx +import { useEffect, useRef } from 'react'; +import { + MapView, + type Coordinate, + type MapViewRef, +} from 'react-native-better-maps'; + +function FittedMap({ points }: { points: Coordinate[] }) { + const mapRef = useRef(null); + + useEffect(() => { + // Runs before the native map exists, and still moves the camera. + mapRef.current?.fitToCoordinates(points, undefined, true); + }, [points]); + + return ; +} +``` + +So no call needs `setTimeout`, a retry, or an `onMapReady` handler to be safe. +`onMapReady` reports something later and different - that the map finished +loading its tiles - and is the right hook for showing your own UI on top of a +map that has actually drawn. + +A call still waiting when the map view unmounts rejects, as does any call made +afterwards. + ## Map providers `MapView` accepts an optional `provider` prop: @@ -702,6 +736,7 @@ See [example/.env.example](example/.env.example) for the supported environment v | Provider throws before rendering | Check the [supported platforms](#supported-platforms) table. `openstreetmap` and `mapbox` are reserved for future support but do not render yet. | | Expo Go does not load native maps | Use a development build after `expo prebuild`; native Nitro modules are not available in Expo Go. | | Marker animations affect gesture smoothness | For very large marker sets, prefer clustering, shorter durations, or disable marker/cluster entering animations. | +| `MapView is not mounted` from a ref call | The map view has unmounted. Calls made before the native map exists are held and replayed, so a freshly mounted map is not the cause. | ## Development diff --git a/docs/architecture.md b/docs/architecture.md index 37f38d8a..429040e2 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -92,6 +92,27 @@ Map and overlay callbacks are wired through Nitro listeners on the HybridView. C | `mapPadding` | Edge insets in density-independent pixels. Applied via `layoutMargins` (iOS) or `setPadding` (Android). | | `fitToCoordinates(coords, padding?, animated?)` | Imperative ref method; fits camera to a set of coordinates with optional padding. | +### Imperative ref readiness + +`MapViewRef` hands out a working handle during the commit that mounts the view, +which is earlier than the native map can exist. Two buffers close that gap, and +neither uses a timer: + +- **JS** — Nitro delivers the `hybridRef` view prop one JS -> UI -> JS round trip + after the mount transaction, so `MapViewCommands` (`package/src/native/mapViewCommands.ts`) + holds every call made before it arrives and replays them in call order. Calls + left waiting when the view unmounts are rejected, and later calls reject + without reaching native. +- **Android** — `MapView.getMapAsync` answers later still, so + `DeferredGoogleMap` holds camera work until the `GoogleMap` exists, and + `configureMap` drains it after replaying the `region`/`camera` props. Without + it the adapter would accept a camera call and quietly do nothing. iOS has no + equivalent window: `MKMapView`/`GMSMapView` exist as soon as the adapter is + installed. + +`onMapReady` is a separate, later signal - the map finished loading tiles - and +is not a precondition for using the ref. + ### Platform gaps (Phase 8) - **Provider availability** — `apple` and `google` are implemented on iOS, and `google` is implemented on Android. `openstreetmap` and `mapbox` are planned provider adapters. diff --git a/example/App.tsx b/example/App.tsx index ba8cb5c5..45d72b91 100644 --- a/example/App.tsx +++ b/example/App.tsx @@ -6,7 +6,7 @@ import { useRef, useState, type ReactNode, - type Ref, + type RefObject, } from 'react'; import { StatusBar } from 'expo-status-bar'; import { @@ -89,6 +89,18 @@ const ANIMATION_OPTIONS: AnimationOption[] = [ { id: 'none', label: 'Off', value: false }, ]; +/** + * Identity of the native map view. `MapScene` is keyed on it so its mount + * effects run exactly when the map they drive is created. + */ +function mapSceneKey( + scenario: MapScenario, + animationOption: AnimationOption, + provider: SupportedExampleProvider, +): string { + return `${scenario.id}:${animationOption.id}:${provider}`; +} + function getSupportedMapProviders(): SupportedExampleProvider[] { switch (Platform.OS) { case 'ios': @@ -514,14 +526,59 @@ const ScenarioDock = memo(function ScenarioDock({ ); }); +type MountCameraFitOptions = { + scenario: MapScenario; + mapRef?: RefObject; + mapPadding?: EdgePadding; + onResult: (result: string) => void; +}; + +/** + * Fits the camera to the scenario markers from a mount effect - the earliest a + * consumer can reach the ref, and earlier than the native map can exist. The + * promise result is reported verbatim so a silent failure cannot hide. + */ +function useMountCameraFit({ + scenario, + mapRef, + mapPadding, + onResult, +}: MountCameraFitOptions) { + useEffect(() => { + if (scenario.advanced?.fitToCoordinatesOnMount !== true) { + return; + } + + const coordinates = (scenario.markers ?? []).map( + (marker) => marker.coordinate, + ); + const handle = mapRef?.current; + if (handle == null) { + onResult('Mount fit · no handle'); + return; + } + + onResult('Mount fit · pending'); + handle + .fitToCoordinates(coordinates, mapPadding, true) + .then(() => onResult('Mount fit · resolved')) + .catch((error: Error) => + onResult(`Mount fit · rejected: ${error.message}`), + ); + // Mount only: this scene is keyed to the native map view it drives. + // eslint-disable-next-line react-hooks/exhaustive-deps + }, []); +} + type MapSceneProps = { - ref?: Ref; + ref?: RefObject; scenario: MapScenario; provider: SupportedExampleProvider; mapType: MapType; mapPadding?: EdgePadding; animationOption: AnimationOption; onMapReady: () => void; + onMountFitResult: (result: string) => void; onClusterPress: (markerIds: string[], coordinate: Coordinate) => void; onMarkerPress: (id: string) => void; onMarkerDragEnd: (id: string, coordinate: Coordinate) => void; @@ -541,6 +598,7 @@ const MapScene = memo(function MapScene({ mapPadding, animationOption, onMapReady, + onMountFitResult, onClusterPress, onMarkerPress, onMarkerDragEnd, @@ -551,6 +609,13 @@ const MapScene = memo(function MapScene({ onRegionChange, onRegionChangeComplete, }: MapSceneProps) { + useMountCameraFit({ + scenario, + mapRef: ref, + mapPadding, + onResult: onMountFitResult, + }); + const commonMapProps = { style: styles.map, mapType, @@ -584,7 +649,6 @@ const MapScene = memo(function MapScene({ return ( - ); + return ; }); type StatusHeaderProps = { @@ -841,9 +898,16 @@ export default function App() { [], ); + const handleMountFitResult = useCallback((result: string) => { + setStatus(result); + }, []); + const handleMapReady = useCallback(() => { setMapReady(true); - setStatus(scenario.name); + // The mount-effect result is the point of that scenario; do not bury it. + if (scenario.advanced?.fitToCoordinatesOnMount !== true) { + setStatus(scenario.name); + } if ( scenario.advanced?.fitToCoordinatesOnReady && @@ -898,12 +962,14 @@ export default function App() { ) -> Unit>() + + /** Publishes the map and drains everything waiting for it, in call order. */ + fun attach(map: GoogleMap) { + runOnMain { + this.map = map + drain(Result.success(map)) + } + } + + /** Drops the map for good and rejects everything waiting for it, now or later. */ + fun release() { + runOnMain { + isReleased = true + map = null + drain(Result.failure(IllegalStateException(MAP_RELEASED_MESSAGE))) + } + } + + /** + * Resolves with the result of [block], running it as soon as the map exists. + * + * Rejects with whatever [block] throws, and with an [IllegalStateException] + * once [release] has happened - a released map never arrives, so the caller + * is never left waiting on one. + */ + fun promise(block: (GoogleMap) -> T): Promise { + val promise = Promise() + + runOnMain { + val deliver: (Result) -> Unit = { result -> + result + .mapCatching(block) + .onSuccess { value -> promise.resolve(value) } + .onFailure { error -> promise.reject(error) } + } + + val currentMap = map + when { + currentMap != null -> deliver(Result.success(currentMap)) + isReleased -> deliver(Result.failure(IllegalStateException(MAP_RELEASED_MESSAGE))) + else -> waiting += deliver + } + } + + return promise + } + + private fun drain(result: Result) { + val queued = waiting.toList() + waiting.clear() + for (deliver in queued) { + deliver(result) + } + } +} diff --git a/package/android/src/main/java/com/margelo/nitro/nitromaps/GoogleMapProviderAdapter.kt b/package/android/src/main/java/com/margelo/nitro/nitromaps/GoogleMapProviderAdapter.kt index fa588a72..2dc88531 100644 --- a/package/android/src/main/java/com/margelo/nitro/nitromaps/GoogleMapProviderAdapter.kt +++ b/package/android/src/main/java/com/margelo/nitro/nitromaps/GoogleMapProviderAdapter.kt @@ -4,8 +4,6 @@ import android.Manifest import android.content.ComponentCallbacks import android.content.pm.PackageManager import android.content.res.Configuration -import android.os.Handler -import android.os.Looper import android.util.Log import android.view.View import android.view.ViewTreeObserver @@ -38,7 +36,7 @@ class GoogleMapProviderAdapter( private var pendingPolylines: Array? = null private var pendingPolygons: Array? = null private var pendingCircles: Array? = null - private val mainHandler = Handler(Looper.getMainLooper()) + private val deferredMap = DeferredGoogleMap() private val googleMapIdAtCreation: String? = normalizeGoogleMapId(initialGoogleMapId) @@ -118,7 +116,7 @@ class GoogleMapProviderAdapter( set(value) { _camera = value if (value != null && !isUserGesture) { - updateMapCamera(value, animated = false) + applyCameraProp(value) } } @@ -318,66 +316,42 @@ class GoogleMapProviderAdapter( syncMarkerPressHandlers() } - override fun fetchCamera(): Promise = - promiseOnMain { - googleMap?.cameraPosition?.toCamera() ?: fallbackCamera() - } + override fun fetchCamera(): Promise = deferredMap.promise { map -> map.cameraPosition.toCamera() } - /** The camera the caller last asked for, used until the map itself can answer. */ - private fun fallbackCamera(): Camera { - val camera = _camera - if (camera != null) { - return camera + override fun applyCamera(camera: Camera): Promise = + deferredMap.promise { map -> + updateMapCamera(map, camera, animated = false) } - return Camera( - center = - Coordinate( - latitude = _region?.latitude ?: 0.0, - longitude = _region?.longitude ?: 0.0, - ), - zoom = 10.0, - heading = null, - pitch = null, - altitude = null, - ) - } - - override fun applyCamera(camera: Camera) { - updateMapCamera(camera, animated = false) - } - override fun animateCamera( camera: Camera, duration: Double?, - ) { + ): Promise { val animationDuration = duration ?: 0.25 - updateMapCamera(camera, animated = true, durationMs = (animationDuration * 1000).toInt()) + return deferredMap.promise { map -> + updateMapCamera(map, camera, animated = true, durationMs = (animationDuration * 1000).toInt()) + } } - override fun getVisibleRegion(): Promise = - promiseOnMain { - googleMap?.projection?.toNitroVisibleRegion() ?: emptyVisibleRegion() - } + override fun getVisibleRegion(): Promise = deferredMap.promise { map -> map.projection.toNitroVisibleRegion() } override fun fitToCoordinates( coordinates: Array, padding: EdgePadding?, animated: Boolean?, - ) { - // Filtered before the main-thread hop: a throw out of `LatLngBounds` inside - // `runOnMain` lands on the looper, where the JS caller cannot catch it. + ): Promise { + // Filtered up front: `LatLngBounds` throws on a coordinate outside the world, + // and a skipped one deserves a warning rather than a rejected promise. val validCoordinates = coordinates.filter { it.isValid() } val skipped = coordinates.size - validCoordinates.size if (skipped > 0) { Log.w(NITRO_MAPS_LOG_TAG, "fitToCoordinates skipped $skipped coordinate(s) outside the world.") } if (validCoordinates.isEmpty()) { - return + return Promise.resolved(Unit) } - runOnMain { - val map = googleMap ?: return@runOnMain + return deferredMap.promise { map -> val builder = LatLngBounds.Builder() for (coordinate in validCoordinates) { builder.include(LatLng(coordinate.latitude, coordinate.longitude)) @@ -385,7 +359,8 @@ class GoogleMapProviderAdapter( val bounds = builder.build() val paddingPx = padding.toPaddingPixels() - val runUpdate = { + // `newLatLngBounds` throws on a map that has no size yet. + runWhenMapViewLaidOut { val update = CameraUpdateFactory.newLatLngBounds(bounds, paddingPx) if (animated == true) { map.animateCamera(update) @@ -393,8 +368,6 @@ class GoogleMapProviderAdapter( map.moveCamera(update) } } - - runWhenMapViewLaidOut(runUpdate) } } @@ -535,7 +508,18 @@ class GoogleMapProviderAdapter( applyRegion(region) } } - _camera?.let { updateMapCamera(it, animated = false) } + _camera?.let { camera -> updateMapCamera(map, camera, animated = false) } + + // Last, so an imperative call made while the map was still loading wins over + // the `region`/`camera` props it was issued after. + deferredMap.attach(map) + } + + private fun applyCameraProp(camera: Camera) { + runOnMain { + val map = googleMap ?: return@runOnMain + updateMapCamera(map, camera, animated = false) + } } private fun syncMarkerPressHandlers() { @@ -635,35 +619,34 @@ class GoogleMapProviderAdapter( } private fun updateMapCamera( + map: GoogleMap, camera: Camera, animated: Boolean, durationMs: Int = 0, ) { - // Checked before the main-thread hop: `runOnMain` posts to the looper when called from - // anywhere else, so a throw out of `CameraPosition` would surface as an uncaught main-looper - // exception the JS caller cannot catch. + // Every camera path ends here - the `camera` prop, its replay in `configureMap`, and + // `setCamera`/`animateCamera` - so this one check covers them all. An invalid camera is + // skipped rather than thrown: the prop path has no promise to reject, so a throw out of + // `CameraPosition` would surface as an uncaught main-thread exception. if (!camera.isValid()) { Log.w(NITRO_MAPS_LOG_TAG, "Ignored an invalid camera: $camera.") return } - runOnMain { - val map = googleMap ?: return@runOnMain - val target = camera.toCameraPosition(map.cameraPosition) - if (map.cameraPosition.approximatelyEquals(target)) { - return@runOnMain - } + val target = camera.toCameraPosition(map.cameraPosition) + if (map.cameraPosition.approximatelyEquals(target)) { + return + } - val update = CameraUpdateFactory.newCameraPosition(target) - if (animated) { - if (durationMs > 0) { - map.animateCamera(update, durationMs, null) - } else { - map.animateCamera(update) - } + val update = CameraUpdateFactory.newCameraPosition(target) + if (animated) { + if (durationMs > 0) { + map.animateCamera(update, durationMs, null) } else { - map.moveCamera(update) + map.animateCamera(update) } + } else { + map.moveCamera(update) } } @@ -713,26 +696,6 @@ class GoogleMapProviderAdapter( overlayController.setViewportSize(view.width, view.height) } - private fun runOnMain(block: () -> Unit) { - if (Looper.myLooper() == Looper.getMainLooper()) { - block() - } else { - mainHandler.post(block) - } - } - - private fun promiseOnMain(block: () -> T): Promise { - val promise = Promise() - runOnMain { - try { - promise.resolve(block()) - } catch (error: Throwable) { - promise.reject(error) - } - } - return promise - } - private fun handleRegionWillChange(userInteracting: Boolean) { if (userInteracting && !isUserGesture) { isUserGesture = true @@ -804,6 +767,8 @@ class GoogleMapProviderAdapter( * detaching from the window deliberately does not come here. */ private fun destroyMapView() { + deferredMap.release() + if (lifecycle.isDestroyed) { return } @@ -817,8 +782,3 @@ class GoogleMapProviderAdapter( } private fun normalizeGoogleMapId(value: String?): String? = value?.trim()?.takeIf { it.isNotEmpty() } - -private fun emptyVisibleRegion(): VisibleRegion { - val zero = Coordinate(latitude = 0.0, longitude = 0.0) - return VisibleRegion(nearLeft = zero, nearRight = zero, farLeft = zero, farRight = zero) -} diff --git a/package/android/src/main/java/com/margelo/nitro/nitromaps/HybridMapView.kt b/package/android/src/main/java/com/margelo/nitro/nitromaps/HybridMapView.kt index d060cc16..6efb96fb 100644 --- a/package/android/src/main/java/com/margelo/nitro/nitromaps/HybridMapView.kt +++ b/package/android/src/main/java/com/margelo/nitro/nitromaps/HybridMapView.kt @@ -288,8 +288,7 @@ class HybridMapView( override fun applyCamera(camera: Camera): Promise { val mounted = adapter ?: return notMountedRejection() - mounted.applyCamera(camera) - return Promise.resolved(Unit) + return mounted.applyCamera(camera) } override fun animateCamera( @@ -297,8 +296,7 @@ class HybridMapView( duration: Double?, ): Promise { val mounted = adapter ?: return notMountedRejection() - mounted.animateCamera(camera, duration) - return Promise.resolved(Unit) + return mounted.animateCamera(camera, duration) } override fun getVisibleRegion(): Promise { @@ -312,8 +310,7 @@ class HybridMapView( animated: Boolean?, ): Promise { val mounted = adapter ?: return notMountedRejection() - mounted.fitToCoordinates(coordinates, padding, animated) - return Promise.resolved(Unit) + return mounted.fitToCoordinates(coordinates, padding, animated) } override fun onDropView() { diff --git a/package/android/src/main/java/com/margelo/nitro/nitromaps/MapProviderAdapter.kt b/package/android/src/main/java/com/margelo/nitro/nitromaps/MapProviderAdapter.kt index e9916c9b..8432683a 100644 --- a/package/android/src/main/java/com/margelo/nitro/nitromaps/MapProviderAdapter.kt +++ b/package/android/src/main/java/com/margelo/nitro/nitromaps/MapProviderAdapter.kt @@ -45,12 +45,12 @@ interface MapProviderAdapter { fun fetchCamera(): Promise - fun applyCamera(camera: Camera) + fun applyCamera(camera: Camera): Promise fun animateCamera( camera: Camera, duration: Double?, - ) + ): Promise fun getVisibleRegion(): Promise @@ -58,7 +58,7 @@ interface MapProviderAdapter { coordinates: Array, padding: EdgePadding?, animated: Boolean?, - ) + ): Promise /** * Destroys the underlying native map and unregisters everything the adapter owns. diff --git a/package/android/src/main/java/com/margelo/nitro/nitromaps/RunOnMain.kt b/package/android/src/main/java/com/margelo/nitro/nitromaps/RunOnMain.kt new file mode 100644 index 00000000..64a1981d --- /dev/null +++ b/package/android/src/main/java/com/margelo/nitro/nitromaps/RunOnMain.kt @@ -0,0 +1,16 @@ +package com.margelo.nitro.nitromaps + +import android.os.Handler +import android.os.Looper + +private val mainHandler = Handler(Looper.getMainLooper()) + +/** Runs [block] on the main thread, inline when the caller is already there. */ +internal fun runOnMain(block: () -> Unit) { + if (Looper.myLooper() == Looper.getMainLooper()) { + block() + return + } + + mainHandler.post(block) +} diff --git a/package/src/components/MapView.tsx b/package/src/components/MapView.tsx index 7c6f9af7..128fa881 100644 --- a/package/src/components/MapView.tsx +++ b/package/src/components/MapView.tsx @@ -1,15 +1,16 @@ import { useCallback, + useEffect, useImperativeHandle, useMemo, - useRef, + useState, type Ref, - type RefObject, } from 'react'; import { useValidCamera } from '../camera/useValidCamera'; import { useCollectedOverlays } from '../hooks/useCollectedOverlays'; import { useNitroCallback } from '../hooks/useNitroCallback'; import { useStableValue } from '../hooks/useStableValue'; +import { MapViewCommands } from '../native/mapViewCommands'; import { NativeMapView } from '../native/MapViewNative'; import type { MapView as NativeMapViewHybrid, @@ -32,20 +33,6 @@ import type { MapViewProps, PoiPressEvent } from '../types/map'; import type { MapViewRef } from '../types/ref'; import { normalizeEnteringAnimation } from '../utils/enteringAnimation'; -const MAP_VIEW_NOT_MOUNTED_ERROR = 'MapView is not mounted'; - -function withHybridRef( - hybridRef: RefObject, - run: (hybrid: NativeMapViewHybrid) => T, -): T { - const hybrid = hybridRef.current; - if (hybrid == null) { - return Promise.reject(new Error(MAP_VIEW_NOT_MOUNTED_ERROR)) as T; - } - - return run(hybrid); -} - export function MapView({ ref, style, @@ -87,7 +74,20 @@ export function MapView({ onCirclePress: onCirclePressProp, }: MapViewProps & { ref?: Ref }) { const resolvedProvider = resolveMapProvider(provider); - const hybridRef = useRef(null); + // Identity of the native view. Both are creation-time SDK configuration, so + // changing either remounts it and the handle we hold becomes a dead object. + const nativeViewKey = `${resolvedProvider}:${googleMapId ?? ''}`; + const [commands, setCommands] = useState( + () => new MapViewCommands(), + ); + const [commandsKey, setCommandsKey] = useState(nativeViewKey); + if (commandsKey !== nativeViewKey) { + // Start a fresh channel for the incoming view so calls made during the swap + // are buffered again instead of dispatched to the outgoing one. The effect + // below rejects whatever the old channel still held. + setCommandsKey(nativeViewKey); + setCommands(new MapViewCommands()); + } const { markers: collectedMarkers, polylines: collectedPolylines, @@ -151,9 +151,17 @@ export function MapView({ | ((event: PoiPressEvent) => void) | undefined; - const handleHybridRef = useCallback((nativeRef: NativeMapViewHybrid) => { - hybridRef.current = nativeRef; - }, []); + const handleHybridRef = useCallback( + (nativeRef: NativeMapViewHybrid) => { + commands.attach(nativeRef); + }, + [commands], + ); + + useEffect(() => { + commands.mount(); + return () => commands.unmount(); + }, [commands]); const handleMarkerPress = useCallback( (id: string) => { @@ -256,18 +264,15 @@ export function MapView({ useImperativeHandle( ref, () => ({ - getCamera: () => - withHybridRef(hybridRef, (hybrid) => hybrid.fetchCamera()), + getCamera: () => commands.run((hybrid) => hybrid.fetchCamera()), setCamera: (nextCamera) => - withHybridRef(hybridRef, (hybrid) => hybrid.applyCamera(nextCamera)), + commands.run((hybrid) => hybrid.applyCamera(nextCamera)), animateCamera: (nextCamera, duration) => - withHybridRef(hybridRef, (hybrid) => - hybrid.animateCamera(nextCamera, duration), - ), + commands.run((hybrid) => hybrid.animateCamera(nextCamera, duration)), getVisibleRegion: () => - withHybridRef(hybridRef, (hybrid) => hybrid.getVisibleRegion()), + commands.run((hybrid) => hybrid.getVisibleRegion()), fitToCoordinates: (coordinates, padding, animated) => - withHybridRef(hybridRef, (hybrid) => + commands.run((hybrid) => hybrid.fitToCoordinates( resolveFitCoordinates(coordinates), padding, @@ -275,12 +280,12 @@ export function MapView({ ), ), }), - [], + [commands], ); return ( => { + target.calls.push(name); + return name; + }; +} + +describe('MapViewCommands', () => { + test('runs against the handle once it is attached', async () => { + const commands = new MapViewCommands(); + const target = fakeHybrid(); + commands.attach(target); + + await expect(commands.run(record('fit'))).resolves.toBe('fit'); + expect(target.calls).toEqual(['fit']); + }); + + test('buffers calls made before the handle arrives', async () => { + const commands = new MapViewCommands(); + const target = fakeHybrid(); + + const pending = commands.run(record('fit')); + expect(target.calls).toEqual([]); + + commands.attach(target); + + await expect(pending).resolves.toBe('fit'); + expect(target.calls).toEqual(['fit']); + }); + + test('replays buffered calls in the order they were made', async () => { + const commands = new MapViewCommands(); + const target = fakeHybrid(); + + const results = Promise.all([ + commands.run(record('first')), + commands.run(record('second')), + commands.run(record('third')), + ]); + + commands.attach(target); + + await expect(results).resolves.toEqual(['first', 'second', 'third']); + expect(target.calls).toEqual(['first', 'second', 'third']); + }); + + test('propagates a rejection from a buffered command', async () => { + const commands = new MapViewCommands(); + + const pending = commands.run(async () => { + throw new Error('native failure'); + }); + + commands.attach(fakeHybrid()); + + await expect(pending).rejects.toThrow('native failure'); + }); + + test('settles the whole buffer when one command throws instead of rejecting', async () => { + const commands = new MapViewCommands(); + const target = fakeHybrid(); + + const throwing = commands.run(() => { + throw new Error('nitro argument conversion failed'); + }); + const behind = commands.run(record('behind')); + + commands.attach(target); + + await expect(throwing).rejects.toThrow('nitro argument conversion failed'); + await expect(behind).resolves.toBe('behind'); + expect(target.calls).toEqual(['behind']); + }); + + test('replaces a stale handle when a second one is attached', async () => { + const commands = new MapViewCommands(); + const stale = fakeHybrid(); + const fresh = fakeHybrid(); + + commands.attach(stale); + commands.attach(fresh); + + await expect(commands.run(record('fit'))).resolves.toBe('fit'); + expect(stale.calls).toEqual([]); + expect(fresh.calls).toEqual(['fit']); + }); + + test('rejects buffered calls when the view unmounts first', async () => { + const commands = new MapViewCommands(); + const target = fakeHybrid(); + + const pending = commands.run(record('fit')); + commands.unmount(); + + await expect(pending).rejects.toThrow( + MAP_VIEW_UNMOUNTED_BEFORE_READY_ERROR, + ); + expect(target.calls).toEqual([]); + }); + + test('never touches the handle for a call cancelled at unmount', async () => { + const commands = new MapViewCommands(); + const target = fakeHybrid(); + + const pending = commands.run(record('fit')); + commands.unmount(); + await expect(pending).rejects.toThrow( + MAP_VIEW_UNMOUNTED_BEFORE_READY_ERROR, + ); + + commands.attach(target); + + expect(target.calls).toEqual([]); + }); + + test('rejects calls made after unmount', async () => { + const commands = new MapViewCommands(); + const target = fakeHybrid(); + commands.attach(target); + commands.unmount(); + + await expect(commands.run(record('fit'))).rejects.toThrow( + MAP_VIEW_NOT_MOUNTED_ERROR, + ); + expect(target.calls).toEqual([]); + }); + + test('keeps working through a StrictMode unmount/mount cycle', async () => { + const commands = new MapViewCommands(); + const target = fakeHybrid(); + + const discarded = commands.run(record('discarded')); + commands.unmount(); + commands.mount(); + + const kept = commands.run(record('kept')); + commands.attach(target); + + await expect(discarded).rejects.toThrow( + MAP_VIEW_UNMOUNTED_BEFORE_READY_ERROR, + ); + await expect(kept).resolves.toBe('kept'); + expect(target.calls).toEqual(['kept']); + }); + + test('keeps the handle when the effect is re-run after attaching', async () => { + const commands = new MapViewCommands(); + const target = fakeHybrid(); + + commands.attach(target); + commands.unmount(); + commands.mount(); + + await expect(commands.run(record('fit'))).resolves.toBe('fit'); + expect(target.calls).toEqual(['fit']); + }); +}); diff --git a/package/src/native/mapViewCommands.ts b/package/src/native/mapViewCommands.ts new file mode 100644 index 00000000..5b6ea45a --- /dev/null +++ b/package/src/native/mapViewCommands.ts @@ -0,0 +1,98 @@ +export const MAP_VIEW_NOT_MOUNTED_ERROR = 'MapView is not mounted'; + +export const MAP_VIEW_UNMOUNTED_BEFORE_READY_ERROR = + 'MapView was unmounted before the native map became available'; + +interface BufferedCommand { + /** Runs the command against the arrived target and settles the caller's promise. */ + flush(target: Target): void; + /** Rejects the caller's promise without ever touching a target. */ + cancel(error: Error): void; +} + +/** + * Imperative command channel to a native view whose handle arrives late. + * + * Nitro delivers the `hybridRef` prop one JS -> UI -> JS round trip after the + * commit that mounted the view, so every call made from a consumer's mount + * effect is issued before there is anything to call. Commands made in that + * window are buffered here and replayed, in the order they were made, as soon + * as {@linkcode MapViewCommands.attach} hands over the handle. + */ +export class MapViewCommands { + private target: Target | null = null; + private buffered: BufferedCommand[] = []; + private isUnmounted = false; + + /** + * Runs {@linkcode command} against the native handle, buffering it when the + * handle has not arrived yet. + * + * Rejects with {@linkcode MAP_VIEW_NOT_MOUNTED_ERROR} once the view has + * unmounted, and with {@linkcode MAP_VIEW_UNMOUNTED_BEFORE_READY_ERROR} when + * the view unmounts while the command is still buffered. + */ + run(command: (target: Target) => Promise): Promise { + if (this.isUnmounted) { + return Promise.reject(new Error(MAP_VIEW_NOT_MOUNTED_ERROR)); + } + + const target = this.target; + if (target != null) { + return command(target); + } + + return new Promise((resolve, reject) => { + this.buffered.push({ + // A buffered caller already holds a promise, so a command that throws + // instead of rejecting has to be turned into a rejection here. Letting + // it escape would abandon every command queued behind it. + flush: (arrivedTarget) => { + try { + command(arrivedTarget).then(resolve, reject); + } catch (error) { + reject(error); + } + }, + cancel: reject, + }); + }); + } + + /** Publishes the native handle and replays everything buffered so far. */ + attach(target: Target): void { + this.target = target; + + const buffered = this.buffered; + this.buffered = []; + for (const command of buffered) { + command.flush(target); + } + } + + /** + * Re-arms the channel after {@linkcode MapViewCommands.unmount}. StrictMode + * tears an effect down and sets it up again while the view itself stays + * mounted, so the handle survives that cycle - it belongs to the native view, + * not to the effect. + */ + mount(): void { + this.isUnmounted = false; + } + + /** Rejects every buffered command and makes later calls reject too. */ + unmount(): void { + this.isUnmounted = true; + + if (this.buffered.length === 0) { + return; + } + + const buffered = this.buffered; + this.buffered = []; + const error = new Error(MAP_VIEW_UNMOUNTED_BEFORE_READY_ERROR); + for (const command of buffered) { + command.cancel(error); + } + } +} diff --git a/package/src/types/ref.ts b/package/src/types/ref.ts index fa24206d..67c5b0eb 100644 --- a/package/src/types/ref.ts +++ b/package/src/types/ref.ts @@ -4,6 +4,23 @@ import type { EdgePadding, VisibleRegion } from './region'; /** * Imperative handle for controlling the map view. + * + * Every method is usable as soon as React has attached the ref. A call made + * before the native map exists - from a mount effect, for example - is held and + * replayed once it does, in the order the calls were made, so waiting for + * {@linkcode MapViewProps.onMapReady} or a timer is never necessary. + * + * A call that is still waiting when the map view unmounts rejects, as does any + * call made afterwards. Nothing silently does nothing. + * + * @example + * ```tsx + * const mapRef = useRef(null); + * + * useEffect(() => { + * mapRef.current?.fitToCoordinates(points); + * }, []); + * ``` */ export interface MapViewRef { /** Returns the current camera position. */ @@ -12,13 +29,23 @@ export interface MapViewRef { /** Sets the camera position immediately. */ setCamera(camera: Camera): Promise; - /** Animates the camera to the given position. */ + /** + * Animates the camera to the given position. Resolves once the animation has + * been handed to the native map, not when it finishes. + * + * @param duration Animation duration in seconds. Defaults to `0.25`. + */ animateCamera(camera: Camera, duration?: number): Promise; /** Returns the currently visible geographic region. */ getVisibleRegion(): Promise; - /** Fits the camera to show all given coordinates with optional edge padding. */ + /** + * Fits the camera to show all given coordinates with optional edge padding. + * An empty {@linkcode coordinates} list is a no-op. + * + * @param animated Pass it explicitly: omitted, iOS animates and Android jumps. + */ fitToCoordinates( coordinates: Coordinate[], padding?: EdgePadding, From b770f3dc6f3c31297203acddf3a0354bf9131bdc Mon Sep 17 00:00:00 2001 From: Jakub Kasprzyk Date: Tue, 22 Sep 2026 10:51:43 +0200 Subject: [PATCH 2/6] fix: reject deferred map work after the adapter is released `DeferredGoogleMap.attach` restored the map without checking the terminal released state, and `promise` tests the map before that state, so a `getMapAsync` callback arriving after `destroyMapView` revived the queue and ran work against a map whose `MapView` was already destroyed. The example scene's mount-fit probe also reported its own rejection over the status of the scene that replaced it; its effect now drops results once the scene is gone. --- example/App.tsx | 21 +++++++++++++++---- .../nitro/nitromaps/DeferredGoogleMap.kt | 11 +++++++++- 2 files changed, 27 insertions(+), 5 deletions(-) diff --git a/example/App.tsx b/example/App.tsx index 45d72b91..6c3c3403 100644 --- a/example/App.tsx +++ b/example/App.tsx @@ -559,12 +559,25 @@ function useMountCameraFit({ } onResult('Mount fit · pending'); + // A scene that is swapped out while the call is in flight must not report + // its own rejection over the incoming scene's status. + let isCurrentScene = true; handle .fitToCoordinates(coordinates, mapPadding, true) - .then(() => onResult('Mount fit · resolved')) - .catch((error: Error) => - onResult(`Mount fit · rejected: ${error.message}`), - ); + .then(() => { + if (isCurrentScene) { + onResult('Mount fit · resolved'); + } + }) + .catch((error: Error) => { + if (isCurrentScene) { + onResult(`Mount fit · rejected: ${error.message}`); + } + }); + + return () => { + isCurrentScene = false; + }; // Mount only: this scene is keyed to the native map view it drives. // eslint-disable-next-line react-hooks/exhaustive-deps }, []); diff --git a/package/android/src/main/java/com/margelo/nitro/nitromaps/DeferredGoogleMap.kt b/package/android/src/main/java/com/margelo/nitro/nitromaps/DeferredGoogleMap.kt index 6d5a6881..96a5235e 100644 --- a/package/android/src/main/java/com/margelo/nitro/nitromaps/DeferredGoogleMap.kt +++ b/package/android/src/main/java/com/margelo/nitro/nitromaps/DeferredGoogleMap.kt @@ -18,9 +18,18 @@ internal class DeferredGoogleMap { private var isReleased = false private val waiting = mutableListOf<(Result) -> Unit>() - /** Publishes the map and drains everything waiting for it, in call order. */ + /** + * Publishes the map and drains everything waiting for it, in call order. + * + * Ignored once [release] has happened: `getMapAsync` can deliver after the + * adapter destroyed its `MapView`, and that map must not come back to life. + */ fun attach(map: GoogleMap) { runOnMain { + if (isReleased) { + return@runOnMain + } + this.map = map drain(Result.success(map)) } From e081cc89406ba67ef385139bf285fdc82a135f87 Mon Sep 17 00:00:00 2001 From: Jakub Kasprzyk Date: Tue, 22 Sep 2026 11:05:22 +0200 Subject: [PATCH 3/6] fix: resolve fitToCoordinates once the camera actually moves `runWhenViewLaidOut` runs inline only when the map view already has a size; otherwise it registers a layout listener and returns, so the promise reported success for a camera that had not moved yet and a throw from the later callback could not reject it. `DeferredGoogleMap.promiseCompletion` hands the work a completion instead, and `fitToCoordinates` settles from inside the layout callback. A command that throws instead of rejecting now rejects on both paths of `MapViewCommands.run`; previously only the buffered one did, so whether a caller could catch the failure depended on timing alone. The channel itself moves into `useMapViewCommands`, which keeps the readiness concern in one named place. The README and `MapViewRef` examples handle the rejection they document, and the README says that StrictMode's extra teardown is what triggers it in development. --- README.md | 9 +++- .../nitro/nitromaps/DeferredGoogleMap.kt | 51 ++++++++++++++++--- .../nitromaps/GoogleMapProviderAdapter.kt | 21 +++++--- package/src/components/MapView.tsx | 25 ++------- package/src/hooks/useMapViewCommands.ts | 32 ++++++++++++ .../native/__tests__/mapViewCommands.test.ts | 11 ++++ package/src/native/mapViewCommands.ts | 28 ++++++---- package/src/types/ref.ts | 4 +- 8 files changed, 132 insertions(+), 49 deletions(-) create mode 100644 package/src/hooks/useMapViewCommands.ts diff --git a/README.md b/README.md index ec40c59b..b41501f6 100644 --- a/README.md +++ b/README.md @@ -271,7 +271,9 @@ function FittedMap({ points }: { points: Coordinate[] }) { useEffect(() => { // Runs before the native map exists, and still moves the camera. - mapRef.current?.fitToCoordinates(points, undefined, true); + mapRef.current + ?.fitToCoordinates(points, undefined, true) + .catch((error: Error) => console.warn(error.message)); }, [points]); return ; @@ -284,7 +286,10 @@ loading its tiles - and is the right hook for showing your own UI on top of a map that has actually drawn. A call still waiting when the map view unmounts rejects, as does any call made -afterwards. +afterwards, so handle the rejection the way the example above does. Development +builds wrapped in React's `` see this on every mount: React tears +the effect down and sets it up again, the first call is rejected by that +teardown, and the second one does the work. ## Map providers diff --git a/package/android/src/main/java/com/margelo/nitro/nitromaps/DeferredGoogleMap.kt b/package/android/src/main/java/com/margelo/nitro/nitromaps/DeferredGoogleMap.kt index 96a5235e..e42ea6c3 100644 --- a/package/android/src/main/java/com/margelo/nitro/nitromaps/DeferredGoogleMap.kt +++ b/package/android/src/main/java/com/margelo/nitro/nitromaps/DeferredGoogleMap.kt @@ -54,14 +54,51 @@ internal class DeferredGoogleMap { fun promise(block: (GoogleMap) -> T): Promise { val promise = Promise() - runOnMain { - val deliver: (Result) -> Unit = { result -> - result - .mapCatching(block) - .onSuccess { value -> promise.resolve(value) } - .onFailure { error -> promise.reject(error) } + whenAvailable { result -> + result + .mapCatching(block) + .onSuccess { value -> promise.resolve(value) } + .onFailure { error -> promise.reject(error) } + } + + return promise + } + + /** + * Like [promise], but for work that finishes in a later native callback: the + * promise settles when [block] calls the completion it is handed, not when + * [block] returns. + * + * A camera update that has to wait for the view's first layout pass finishes + * long after the call that scheduled it, and resolving before it ran would + * report success for a camera that has not moved. + */ + fun promiseCompletion(block: (GoogleMap, complete: (Result) -> Unit) -> Unit): Promise { + val promise = Promise() + + whenAvailable { result -> + // Everything funnels through `complete`, so the promise settles exactly + // once whether the work finished, failed, or never started. + var isSettled = false + val complete: (Result) -> Unit = { outcome -> + if (!isSettled) { + isSettled = true + outcome + .onSuccess { promise.resolve(Unit) } + .onFailure { error -> promise.reject(error) } + } } + result + .mapCatching { map -> block(map, complete) } + .onFailure { error -> complete(Result.failure(error)) } + } + + return promise + } + + private fun whenAvailable(deliver: (Result) -> Unit) { + runOnMain { val currentMap = map when { currentMap != null -> deliver(Result.success(currentMap)) @@ -69,8 +106,6 @@ internal class DeferredGoogleMap { else -> waiting += deliver } } - - return promise } private fun drain(result: Result) { diff --git a/package/android/src/main/java/com/margelo/nitro/nitromaps/GoogleMapProviderAdapter.kt b/package/android/src/main/java/com/margelo/nitro/nitromaps/GoogleMapProviderAdapter.kt index 2dc88531..8c52a9b7 100644 --- a/package/android/src/main/java/com/margelo/nitro/nitromaps/GoogleMapProviderAdapter.kt +++ b/package/android/src/main/java/com/margelo/nitro/nitromaps/GoogleMapProviderAdapter.kt @@ -351,7 +351,7 @@ class GoogleMapProviderAdapter( return Promise.resolved(Unit) } - return deferredMap.promise { map -> + return deferredMap.promiseCompletion { map, complete -> val builder = LatLngBounds.Builder() for (coordinate in validCoordinates) { builder.include(LatLng(coordinate.latitude, coordinate.longitude)) @@ -359,14 +359,19 @@ class GoogleMapProviderAdapter( val bounds = builder.build() val paddingPx = padding.toPaddingPixels() - // `newLatLngBounds` throws on a map that has no size yet. + // `newLatLngBounds` throws on a map that has no size yet, so the camera + // update waits for the first layout pass -- and so does the promise. runWhenMapViewLaidOut { - val update = CameraUpdateFactory.newLatLngBounds(bounds, paddingPx) - if (animated == true) { - map.animateCamera(update) - } else { - map.moveCamera(update) - } + complete( + runCatching { + val update = CameraUpdateFactory.newLatLngBounds(bounds, paddingPx) + if (animated == true) { + map.animateCamera(update) + } else { + map.moveCamera(update) + } + }, + ) } } } diff --git a/package/src/components/MapView.tsx b/package/src/components/MapView.tsx index 128fa881..d71c57dc 100644 --- a/package/src/components/MapView.tsx +++ b/package/src/components/MapView.tsx @@ -1,16 +1,14 @@ import { useCallback, - useEffect, useImperativeHandle, useMemo, - useState, type Ref, } from 'react'; import { useValidCamera } from '../camera/useValidCamera'; import { useCollectedOverlays } from '../hooks/useCollectedOverlays'; +import { useMapViewCommands } from '../hooks/useMapViewCommands'; import { useNitroCallback } from '../hooks/useNitroCallback'; import { useStableValue } from '../hooks/useStableValue'; -import { MapViewCommands } from '../native/mapViewCommands'; import { NativeMapView } from '../native/MapViewNative'; import type { MapView as NativeMapViewHybrid, @@ -74,20 +72,10 @@ export function MapView({ onCirclePress: onCirclePressProp, }: MapViewProps & { ref?: Ref }) { const resolvedProvider = resolveMapProvider(provider); - // Identity of the native view. Both are creation-time SDK configuration, so - // changing either remounts it and the handle we hold becomes a dead object. + // Both are creation-time SDK configuration, so changing either remounts the + // native view. const nativeViewKey = `${resolvedProvider}:${googleMapId ?? ''}`; - const [commands, setCommands] = useState( - () => new MapViewCommands(), - ); - const [commandsKey, setCommandsKey] = useState(nativeViewKey); - if (commandsKey !== nativeViewKey) { - // Start a fresh channel for the incoming view so calls made during the swap - // are buffered again instead of dispatched to the outgoing one. The effect - // below rejects whatever the old channel still held. - setCommandsKey(nativeViewKey); - setCommands(new MapViewCommands()); - } + const commands = useMapViewCommands(nativeViewKey); const { markers: collectedMarkers, polylines: collectedPolylines, @@ -158,11 +146,6 @@ export function MapView({ [commands], ); - useEffect(() => { - commands.mount(); - return () => commands.unmount(); - }, [commands]); - const handleMarkerPress = useCallback( (id: string) => { callbackRegistry.current.get(overlayCallbackKey(OverlayType.Marker, id))?.onPress?.(); diff --git a/package/src/hooks/useMapViewCommands.ts b/package/src/hooks/useMapViewCommands.ts new file mode 100644 index 00000000..cefb5f09 --- /dev/null +++ b/package/src/hooks/useMapViewCommands.ts @@ -0,0 +1,32 @@ +import { useEffect, useState } from 'react'; +import { MapViewCommands } from '../native/mapViewCommands'; +import type { MapView as NativeMapViewHybrid } from '../native/specs/MapView.nitro'; + +/** + * Owns the imperative command channel for one native map view. + * + * The channel outlives a render but not the view it talks to: when + * {@linkcode nativeViewKey} changes, the native view is remounted and the + * handle the old channel holds becomes a dead object, so a fresh channel takes + * over and calls made during the swap are buffered for the incoming view. + */ +export function useMapViewCommands(nativeViewKey: string) { + const [commands, setCommands] = useState( + () => new MapViewCommands(), + ); + const [commandsKey, setCommandsKey] = useState(nativeViewKey); + + if (commandsKey !== nativeViewKey) { + setCommandsKey(nativeViewKey); + setCommands(new MapViewCommands()); + } + + // Also rejects whatever the outgoing channel still held: React runs this + // cleanup for the old instance before arming the new one. + useEffect(() => { + commands.mount(); + return () => commands.unmount(); + }, [commands]); + + return commands; +} diff --git a/package/src/native/__tests__/mapViewCommands.test.ts b/package/src/native/__tests__/mapViewCommands.test.ts index b5b40b67..6f856cdd 100644 --- a/package/src/native/__tests__/mapViewCommands.test.ts +++ b/package/src/native/__tests__/mapViewCommands.test.ts @@ -71,6 +71,17 @@ describe('MapViewCommands', () => { await expect(pending).rejects.toThrow('native failure'); }); + test('rejects rather than throwing when an attached command throws', async () => { + const commands = new MapViewCommands(); + commands.attach(fakeHybrid()); + + await expect( + commands.run(() => { + throw new Error('nitro argument conversion failed'); + }), + ).rejects.toThrow('nitro argument conversion failed'); + }); + test('settles the whole buffer when one command throws instead of rejecting', async () => { const commands = new MapViewCommands(); const target = fakeHybrid(); diff --git a/package/src/native/mapViewCommands.ts b/package/src/native/mapViewCommands.ts index 5b6ea45a..1759eb60 100644 --- a/package/src/native/mapViewCommands.ts +++ b/package/src/native/mapViewCommands.ts @@ -3,6 +3,23 @@ export const MAP_VIEW_NOT_MOUNTED_ERROR = 'MapView is not mounted'; export const MAP_VIEW_UNMOUNTED_BEFORE_READY_ERROR = 'MapView was unmounted before the native map became available'; +/** + * Keeps a command's failure on the promise it returned. A native call that + * throws instead of rejecting would otherwise surface differently depending on + * whether the handle had arrived yet - the same call, caught or not caught + * purely by timing. + */ +function runCommand( + command: (target: Target) => Promise, + target: Target, +): Promise { + try { + return command(target); + } catch (error) { + return Promise.reject(error); + } +} + interface BufferedCommand { /** Runs the command against the arrived target and settles the caller's promise. */ flush(target: Target): void; @@ -39,20 +56,13 @@ export class MapViewCommands { const target = this.target; if (target != null) { - return command(target); + return runCommand(command, target); } return new Promise((resolve, reject) => { this.buffered.push({ - // A buffered caller already holds a promise, so a command that throws - // instead of rejecting has to be turned into a rejection here. Letting - // it escape would abandon every command queued behind it. flush: (arrivedTarget) => { - try { - command(arrivedTarget).then(resolve, reject); - } catch (error) { - reject(error); - } + runCommand(command, arrivedTarget).then(resolve, reject); }, cancel: reject, }); diff --git a/package/src/types/ref.ts b/package/src/types/ref.ts index 67c5b0eb..892eb315 100644 --- a/package/src/types/ref.ts +++ b/package/src/types/ref.ts @@ -18,7 +18,9 @@ import type { EdgePadding, VisibleRegion } from './region'; * const mapRef = useRef(null); * * useEffect(() => { - * mapRef.current?.fitToCoordinates(points); + * mapRef.current + * ?.fitToCoordinates(points) + * .catch((error: Error) => console.warn(error.message)); * }, []); * ``` */ From 0e74a0c32188377340fb1cef4fa7dc98a541060f Mon Sep 17 00:00:00 2001 From: Jakub Kasprzyk Date: Tue, 22 Sep 2026 16:55:15 +0200 Subject: [PATCH 4/6] fix: close the command channel before the native view is removed The channel was closed from a passive effect, which React runs after it has already removed the host view. In that window the channel still held a live handle and had not been told it was unmounted, so a retained imperative handle reached the dead native object instead of the rejection the API documents. Closing it from the layout phase, where cleanup runs before the view is detached, removes the window. --- package/src/hooks/useMapViewCommands.ts | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/package/src/hooks/useMapViewCommands.ts b/package/src/hooks/useMapViewCommands.ts index cefb5f09..51aa2e95 100644 --- a/package/src/hooks/useMapViewCommands.ts +++ b/package/src/hooks/useMapViewCommands.ts @@ -1,4 +1,4 @@ -import { useEffect, useState } from 'react'; +import { useLayoutEffect, useState } from 'react'; import { MapViewCommands } from '../native/mapViewCommands'; import type { MapView as NativeMapViewHybrid } from '../native/specs/MapView.nitro'; @@ -21,9 +21,14 @@ export function useMapViewCommands(nativeViewKey: string) { setCommands(new MapViewCommands()); } - // Also rejects whatever the outgoing channel still held: React runs this - // cleanup for the old instance before arming the new one. - useEffect(() => { + // Layout phase, not passive: the cleanup has to close the channel while the + // native view is still there. React removes the view before passive effects + // run, and until the channel knows it is gone a retained handle would reach + // the dead native object instead of being rejected here. + // + // It also rejects whatever the outgoing channel still held, because React + // runs this cleanup for the old instance before arming the new one. + useLayoutEffect(() => { commands.mount(); return () => commands.unmount(); }, [commands]); From 8c22474d6dcbdcd4ab3ba1a7145626dcfe26cfba Mon Sep 17 00:00:00 2001 From: Jakub Kasprzyk Date: Wed, 23 Sep 2026 17:21:11 +0200 Subject: [PATCH 5/6] fix: reject an invalid camera passed to setCamera or animateCamera The camera prop can only skip a camera the map cannot use, but setCamera and animateCamera have a promise to report it on. They resolved anyway - on Android only once the Google map arrived - for a camera that never moved the map, which is the silent no-op MapViewRef rules out. They now reject straight away, before the call is queued, so the rejection does not wait for the native map and is the same on Android and on both iOS providers. The native guards stay behind it as a backstop and still only skip, since the prop path has no promise to reject. --- README.md | 1 + docs/architecture.md | 3 +- .../nitromaps/GoogleMapProviderAdapter.kt | 6 +- .../__tests__/runWithValidCamera.test.ts | 57 +++++++++++++++++++ package/src/camera/runWithValidCamera.ts | 26 +++++++++ package/src/components/MapView.tsx | 9 ++- package/src/types/ref.ts | 12 +++- 7 files changed, 107 insertions(+), 7 deletions(-) create mode 100644 package/src/camera/__tests__/runWithValidCamera.test.ts create mode 100644 package/src/camera/runWithValidCamera.ts diff --git a/README.md b/README.md index b41501f6..3346a720 100644 --- a/README.md +++ b/README.md @@ -614,6 +614,7 @@ A coordinate that arrives as `NaN` or out of range is dropped instead of being f - An invalid `region` is ignored, and the map keeps the region it already had. - An invalid `camera` is ignored the same way, and a pitch past the range the SDKs draw is pulled back to it rather than rejected. +- `setCamera` and `animateCamera` reject an invalid camera instead of ignoring it, straight away and on both platforms - unlike a prop, they have a promise to report it on. A pitch past the drawable range is pulled back for them too. - An overlay whose coordinates, ring length or radius cannot be drawn is skipped; its neighbours still render. - Anything supplied through `region`, `camera`, or a `` / `` / `` / `` child is reported through `console.warn` in development. diff --git a/docs/architecture.md b/docs/architecture.md index 429040e2..f991a28c 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -102,7 +102,8 @@ neither uses a timer: after the mount transaction, so `MapViewCommands` (`package/src/native/mapViewCommands.ts`) holds every call made before it arrives and replays them in call order. Calls left waiting when the view unmounts are rejected, and later calls reject - without reaching native. + without reaching native. `setCamera`/`animateCamera` check the camera before + it is queued, so an invalid one rejects at once instead of waiting here. - **Android** — `MapView.getMapAsync` answers later still, so `DeferredGoogleMap` holds camera work until the `GoogleMap` exists, and `configureMap` drains it after replaying the `region`/`camera` props. Without diff --git a/package/android/src/main/java/com/margelo/nitro/nitromaps/GoogleMapProviderAdapter.kt b/package/android/src/main/java/com/margelo/nitro/nitromaps/GoogleMapProviderAdapter.kt index 8c52a9b7..4258ae8b 100644 --- a/package/android/src/main/java/com/margelo/nitro/nitromaps/GoogleMapProviderAdapter.kt +++ b/package/android/src/main/java/com/margelo/nitro/nitromaps/GoogleMapProviderAdapter.kt @@ -630,9 +630,11 @@ class GoogleMapProviderAdapter( durationMs: Int = 0, ) { // Every camera path ends here - the `camera` prop, its replay in `configureMap`, and - // `setCamera`/`animateCamera` - so this one check covers them all. An invalid camera is + // `applyCamera`/`animateCamera` - so this one check covers them all. An invalid camera is // skipped rather than thrown: the prop path has no promise to reject, so a throw out of - // `CameraPosition` would surface as an uncaught main-thread exception. + // `CameraPosition` would surface as an uncaught main-thread exception. For the two + // imperative calls it is only a backstop: `MapViewRef` rejects an invalid camera in JS + // before the call is queued. if (!camera.isValid()) { Log.w(NITRO_MAPS_LOG_TAG, "Ignored an invalid camera: $camera.") return diff --git a/package/src/camera/__tests__/runWithValidCamera.test.ts b/package/src/camera/__tests__/runWithValidCamera.test.ts new file mode 100644 index 00000000..69330942 --- /dev/null +++ b/package/src/camera/__tests__/runWithValidCamera.test.ts @@ -0,0 +1,57 @@ +import { describe, expect, test } from 'bun:test'; +import { MapViewCommands } from '../../native/mapViewCommands'; +import { + INVALID_CAMERA_ERROR, + runWithValidCamera, +} from '../runWithValidCamera'; + +const validCamera = { + center: { latitude: 52.23, longitude: 21.01 }, + zoom: 12, +}; + +describe('runWithValidCamera', () => { + test('runs the call for a valid camera', async () => { + await expect( + runWithValidCamera(validCamera, async () => 'moved'), + ).resolves.toBe('moved'); + }); + + test('rejects an invalid camera without running the call', async () => { + let didRun = false; + + await expect( + runWithValidCamera( + { center: { latitude: Number.NaN, longitude: 21.01 } }, + async () => { + didRun = true; + }, + ), + ).rejects.toThrow(INVALID_CAMERA_ERROR); + expect(didRun).toBe(false); + }); + + test('rejects without queueing the call for the native map', async () => { + const commands = new MapViewCommands(); + const reached: string[] = []; + + const pending = runWithValidCamera( + { ...validCamera, zoom: Number.POSITIVE_INFINITY }, + () => + commands.run(async (calls) => { + calls.push('camera'); + }), + ); + // Had the call been buffered, the arriving handle would replay it. + commands.attach(reached); + + await expect(pending).rejects.toThrow(INVALID_CAMERA_ERROR); + expect(reached).toEqual([]); + }); + + test('leaves a finite pitch past 90 to the map, which clamps it', async () => { + await expect( + runWithValidCamera({ ...validCamera, pitch: 120 }, async () => 'moved'), + ).resolves.toBe('moved'); + }); +}); diff --git a/package/src/camera/runWithValidCamera.ts b/package/src/camera/runWithValidCamera.ts new file mode 100644 index 00000000..014e8995 --- /dev/null +++ b/package/src/camera/runWithValidCamera.ts @@ -0,0 +1,26 @@ +import type { Camera } from '../types/camera'; +import { isValidCamera } from './isValidCamera'; + +export const INVALID_CAMERA_ERROR = + 'Camera rejected: invalid center coordinate, or a zoom, heading, pitch or altitude the map cannot use'; + +/** + * Runs a `setCamera` or `animateCamera` call, or rejects it when the map cannot + * use the camera. The `camera` prop can only skip such a camera, but these + * calls have a promise to report it on, and resolving one for a camera that + * never moved the map is the silent no-op `MapViewRef` rules out. + * + * Checked before the call is queued, so it rejects straight away - also before + * the native map exists - and the same way on every platform and provider. The + * native guards behind it only skip. + */ +export function runWithValidCamera( + camera: Camera, + run: () => Promise, +): Promise { + if (!isValidCamera(camera)) { + return Promise.reject(new Error(INVALID_CAMERA_ERROR)); + } + + return run(); +} diff --git a/package/src/components/MapView.tsx b/package/src/components/MapView.tsx index d71c57dc..bad1e690 100644 --- a/package/src/components/MapView.tsx +++ b/package/src/components/MapView.tsx @@ -4,6 +4,7 @@ import { useMemo, type Ref, } from 'react'; +import { runWithValidCamera } from '../camera/runWithValidCamera'; import { useValidCamera } from '../camera/useValidCamera'; import { useCollectedOverlays } from '../hooks/useCollectedOverlays'; import { useMapViewCommands } from '../hooks/useMapViewCommands'; @@ -249,9 +250,13 @@ export function MapView({ () => ({ getCamera: () => commands.run((hybrid) => hybrid.fetchCamera()), setCamera: (nextCamera) => - commands.run((hybrid) => hybrid.applyCamera(nextCamera)), + runWithValidCamera(nextCamera, () => + commands.run((hybrid) => hybrid.applyCamera(nextCamera)), + ), animateCamera: (nextCamera, duration) => - commands.run((hybrid) => hybrid.animateCamera(nextCamera, duration)), + runWithValidCamera(nextCamera, () => + commands.run((hybrid) => hybrid.animateCamera(nextCamera, duration)), + ), getVisibleRegion: () => commands.run((hybrid) => hybrid.getVisibleRegion()), fitToCoordinates: (coordinates, padding, animated) => diff --git a/package/src/types/ref.ts b/package/src/types/ref.ts index 892eb315..42e419e4 100644 --- a/package/src/types/ref.ts +++ b/package/src/types/ref.ts @@ -28,12 +28,20 @@ export interface MapViewRef { /** Returns the current camera position. */ getCamera(): Promise; - /** Sets the camera position immediately. */ + /** + * Sets the camera position immediately. + * + * Rejects straight away, and leaves the map where it is, for a camera the + * map cannot use: a center outside the world, or a zoom, heading, pitch or + * altitude that is `NaN`, infinite, or - for zoom and heading - too large for + * a 32-bit float. The `camera` prop skips such a camera instead. + */ setCamera(camera: Camera): Promise; /** * Animates the camera to the given position. Resolves once the animation has - * been handed to the native map, not when it finishes. + * been handed to the native map, not when it finishes, and rejects for a + * camera the map cannot use, as {@linkcode MapViewRef.setCamera} does. * * @param duration Animation duration in seconds. Defaults to `0.25`. */ From ea858939ff1416fdbeb94a97adc0f41d7545afa5 Mon Sep 17 00:00:00 2001 From: Jakub Kasprzyk Date: Fri, 25 Sep 2026 18:02:07 +0200 Subject: [PATCH 6/6] fix(android): reject a fit still waiting for layout when the map is released fitToCoordinates waits for the map view's first layout pass, because newLatLngBounds throws on a view without a size. Nothing took that layout listener down, so a map released before the pass left the promise pending forever, and the listener stayed on the window's ViewTreeObserver, holding the destroyed map. DeferredLayout now owns the wait and destroyMapView releases it: a waiting fit rejects, and the region and viewport waits are dropped. The listener is registered only while the view is attached to a window, on that window's observer. React Native detaches the view before it drops it, and a detached view hands out a stand-in observer the listener could never be removed from. --- docs/architecture.md | 5 +- .../margelo/nitro/nitromaps/DeferredLayout.kt | 128 ++++++++++++++++++ .../nitromaps/GoogleMapProviderAdapter.kt | 47 +++---- 3 files changed, 152 insertions(+), 28 deletions(-) create mode 100644 package/android/src/main/java/com/margelo/nitro/nitromaps/DeferredLayout.kt diff --git a/docs/architecture.md b/docs/architecture.md index f991a28c..942e46da 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -107,7 +107,10 @@ neither uses a timer: - **Android** — `MapView.getMapAsync` answers later still, so `DeferredGoogleMap` holds camera work until the `GoogleMap` exists, and `configureMap` drains it after replaying the `region`/`camera` props. Without - it the adapter would accept a camera call and quietly do nothing. iOS has no + it the adapter would accept a camera call and quietly do nothing. + `fitToCoordinates` then waits once more, in `DeferredLayout`, for the map + view's first layout pass, because `newLatLngBounds` throws on a view without a + size. It rejects if the view is released before that pass comes. iOS has no equivalent window: `MKMapView`/`GMSMapView` exist as soon as the adapter is installed. diff --git a/package/android/src/main/java/com/margelo/nitro/nitromaps/DeferredLayout.kt b/package/android/src/main/java/com/margelo/nitro/nitromaps/DeferredLayout.kt new file mode 100644 index 00000000..f7807a1d --- /dev/null +++ b/package/android/src/main/java/com/margelo/nitro/nitromaps/DeferredLayout.kt @@ -0,0 +1,128 @@ +package com.margelo.nitro.nitromaps + +import android.view.View +import android.view.ViewTreeObserver + +/** + * Runs work that needs [view] to have a size, holding it back until a layout + * pass gives it one: `newLatLngBounds` throws on a map view that has not been + * laid out yet. + * + * [release] cancels whatever is still waiting. A view torn down before its + * first layout pass never gets one, so without it that work would wait forever, + * along with any promise it was going to settle. + * + * The layout listener is registered only while [view] is attached to a window, + * and on that window's observer. A detached view hands out a stand-in observer + * instead, so a listener left behind when the view leaves its window - which + * React Native does before it drops the view - could no longer be taken off, + * and would keep the destroyed map alive for as long as the window lives. + * + * Main thread only, like the view it waits on. + */ +internal class DeferredLayout( + private val view: View, +) { + private class Entry( + val block: () -> Unit, + val onCancel: () -> Unit, + ) + + private val waiting = mutableListOf() + private var isReleased = false + + /** The window observer [layoutListener] is registered on, while it is. */ + private var registeredOn: ViewTreeObserver? = null + + private val layoutListener = ViewTreeObserver.OnGlobalLayoutListener { runIfLaidOut() } + + private val attachListener = + object : View.OnAttachStateChangeListener { + override fun onViewAttachedToWindow(v: View) { + startListening() + } + + override fun onViewDetachedFromWindow(v: View) { + stopListening() + } + } + + init { + view.addOnAttachStateChangeListener(attachListener) + } + + /** + * Runs [block] now if [view] already has a size, otherwise in the first + * layout pass that gives it one. [onCancel] runs instead if [release] comes + * first, and straight away once it has. + */ + fun run( + onCancel: () -> Unit = {}, + block: () -> Unit, + ) { + if (isReleased) { + onCancel() + return + } + + if (hasSize()) { + block() + return + } + + waiting += Entry(block, onCancel) + startListening() + } + + /** Stops listening for good and cancels everything still waiting, in call order. */ + fun release() { + if (isReleased) { + return + } + + isReleased = true + stopListening() + view.removeOnAttachStateChangeListener(attachListener) + for (entry in drain()) { + entry.onCancel() + } + } + + private fun runIfLaidOut() { + if (!hasSize()) { + return + } + + stopListening() + for (entry in drain()) { + entry.block() + } + } + + private fun startListening() { + if (registeredOn != null || waiting.isEmpty() || !view.isAttachedToWindow) { + return + } + + val observer = view.viewTreeObserver + observer.addOnGlobalLayoutListener(layoutListener) + registeredOn = observer + } + + private fun stopListening() { + val observer = registeredOn ?: return + registeredOn = null + // A window being torn down kills its observer, and the listener goes with it. + if (observer.isAlive) { + observer.removeOnGlobalLayoutListener(layoutListener) + } + } + + private fun drain(): List { + val drained = waiting.toList() + waiting.clear() + return drained + } + + private fun hasSize(): Boolean = view.width > 0 && view.height > 0 +} diff --git a/package/android/src/main/java/com/margelo/nitro/nitromaps/GoogleMapProviderAdapter.kt b/package/android/src/main/java/com/margelo/nitro/nitromaps/GoogleMapProviderAdapter.kt index a4d9c0a6..481594d3 100644 --- a/package/android/src/main/java/com/margelo/nitro/nitromaps/GoogleMapProviderAdapter.kt +++ b/package/android/src/main/java/com/margelo/nitro/nitromaps/GoogleMapProviderAdapter.kt @@ -6,7 +6,6 @@ import android.content.pm.PackageManager import android.content.res.Configuration import android.util.Log import android.view.View -import android.view.ViewTreeObserver import androidx.annotation.Keep import androidx.core.content.ContextCompat import com.facebook.proguard.annotations.DoNotStrip @@ -21,6 +20,8 @@ import com.google.android.gms.maps.model.LatLngBounds import com.google.android.gms.maps.model.MapStyleOptions import com.margelo.nitro.core.Promise +private const val MAP_RELEASED_BEFORE_LAYOUT_MESSAGE = "MapView was released before it was laid out" + @Keep @DoNotStrip class GoogleMapProviderAdapter( @@ -52,6 +53,7 @@ class GoogleMapProviderAdapter( ) private val lifecycle = MapViewLifecycleOwner(view) + private val deferredLayout = DeferredLayout(view) private var isAttachedToWindow = false @@ -360,8 +362,13 @@ class GoogleMapProviderAdapter( val bounds = builder.build() // `newLatLngBounds` throws on a map that has no size yet, so the camera - // update waits for the first layout pass -- and so does the promise. - runWhenMapViewLaidOut { + // update waits for the first layout pass -- and so does the promise, which + // rejects if the view is released before that pass comes. + runWhenMapViewLaidOut( + onCancel = { + complete(Result.failure(IllegalStateException(MAP_RELEASED_BEFORE_LAYOUT_MESSAGE))) + }, + ) { complete( runCatching { // Inside the callback: converting the insets needs the size the map was laid out with. @@ -624,7 +631,7 @@ class GoogleMapProviderAdapter( } } - runWhenMapViewLaidOut(runUpdate) + runWhenMapViewLaidOut(block = runUpdate) } private fun updateMapCamera( @@ -671,36 +678,21 @@ class GoogleMapProviderAdapter( mapView.addOnLayoutChangeListener { _, _, _, _, _, _, _, _, _ -> syncViewportSize() } - runWhenViewLaidOut(mapView, syncViewportSize) - } - - private fun runWhenMapViewLaidOut(block: () -> Unit) { - runWhenViewLaidOut(view, block) + runWhenMapViewLaidOut(block = syncViewportSize) } - private fun runWhenViewLaidOut( - target: View, + /** + * Runs [block] once the map view has a size - see [DeferredLayout]. [onCancel] runs + * instead if the map is destroyed before that. + */ + private fun runWhenMapViewLaidOut( + onCancel: () -> Unit = {}, block: () -> Unit, ) { - if (target.width > 0 && target.height > 0) { + deferredLayout.run(onCancel) { updateOverlayViewportSize() block() - return } - - target.viewTreeObserver.addOnGlobalLayoutListener( - object : ViewTreeObserver.OnGlobalLayoutListener { - override fun onGlobalLayout() { - if (target.width <= 0 || target.height <= 0) { - return - } - - target.viewTreeObserver.removeOnGlobalLayoutListener(this) - updateOverlayViewportSize() - block() - } - }, - ) } private fun updateOverlayViewportSize() { @@ -779,6 +771,7 @@ class GoogleMapProviderAdapter( */ private fun destroyMapView() { deferredMap.release() + deferredLayout.release() if (lifecycle.isDestroyed) { return