diff --git a/README.md b/README.md index 5a7a89d..1005466 100644 --- a/README.md +++ b/README.md @@ -574,14 +574,15 @@ setMarkers((current) => A coordinate that arrives as `NaN` or out of range is dropped instead of being forwarded to MapKit and the Google Maps SDK, which throw on it: - 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. - An overlay whose coordinates, ring length or radius cannot be drawn is skipped; its neighbours still render. -- Anything supplied through `region` or through a `` / `` / `` / `` child is reported through `console.warn` in development. +- Anything supplied through `region`, `camera`, or a `` / `` / `` / `` child is reported through `console.warn` in development. -Where the check runs depends on the entry point. `region` and `fitToCoordinates` are guarded natively on both platforms, so a `hybridRef` call cannot reach the SDKs either. Overlay descriptors are additionally filtered natively on Android, where an undrawable overlay throws inside the Fabric mount transaction and would otherwise take the whole screen down; those skips are reported to logcat rather than `console.warn`. +Where the check runs depends on the entry point. `region`, `camera` and `fitToCoordinates` are guarded natively on both platforms, so a `hybridRef` call - `setCamera` and `animateCamera` included - cannot reach the SDKs either. Overlay descriptors are additionally filtered natively on Android, where an undrawable overlay throws inside the Fabric mount transaction and would otherwise take the whole screen down; those skips are reported to logcat rather than `console.warn`. -Two gaps are worth knowing about: the `camera` prop is not validated anywhere, and descriptors passed through the bulk `markers` prop are checked on neither side - only the `` child is. +One gap is worth knowing about: descriptors passed through the bulk `markers` prop are checked on neither side - only the `` child is. -Valid means: latitude and longitude finite and within ±90 / ±180, region deltas finite and greater than 0, two coordinates for a polyline, three per polygon ring, and a finite radius of at least 0 for a circle. A region whose span would run past a pole is pulled back to what the map can show rather than rejected. +Valid means: latitude and longitude finite and within ±90 / ±180, region deltas finite and greater than 0, camera `zoom` / `heading` / `pitch` / `altitude` finite when supplied (with `zoom` and `heading` also small enough for the 32-bit float the SDKs keep them in), two coordinates for a polyline, three per polygon ring, and a finite radius of at least 0 for a circle. A region whose span would run past a pole is pulled back to what the map can show rather than rejected. ## Capability matrix diff --git a/package/android/src/main/java/com/margelo/nitro/nitromaps/Camera+CameraPosition.kt b/package/android/src/main/java/com/margelo/nitro/nitromaps/Camera+CameraPosition.kt index 55c271d..4323e0e 100644 --- a/package/android/src/main/java/com/margelo/nitro/nitromaps/Camera+CameraPosition.kt +++ b/package/android/src/main/java/com/margelo/nitro/nitromaps/Camera+CameraPosition.kt @@ -10,10 +10,23 @@ fun Camera.toCameraPosition(current: CameraPosition? = null): CameraPosition { .target(LatLng(center.latitude, center.longitude)) .zoom((zoom ?: current?.zoom?.toDouble() ?: 10.0).toFloat()) .bearing((heading ?: current?.bearing?.toDouble() ?: 0.0).toFloat()) - .tilt((pitch ?: current?.tilt?.toDouble() ?: 0.0).toFloat()) + .tilt(drawableTilt(pitch ?: current?.tilt?.toDouble() ?: 0.0)) .build() } +/** + * `CameraPosition.Builder.tilt` throws for anything outside 0..90, and that throw would unwind the + * Fabric mount transaction, so a pitch past the limit is pulled back to it. MapKit flattens such a + * camera rather than refusing it, which is the behaviour this matches. A non-finite pitch never + * reaches here - `Camera.isValid()` drops the whole camera first - but it is handled so this stays + * safe on its own. + */ +private fun drawableTilt(pitch: Double): Float = + if (pitch.isFinite()) pitch.coerceIn(MINIMUM_TILT, MAXIMUM_TILT).toFloat() else MINIMUM_TILT.toFloat() + +private const val MINIMUM_TILT = 0.0 +private const val MAXIMUM_TILT = 90.0 + fun CameraPosition.toCamera(): Camera { return Camera( center = Coordinate(latitude = target.latitude, longitude = target.longitude), diff --git a/package/android/src/main/java/com/margelo/nitro/nitromaps/Camera+Validity.kt b/package/android/src/main/java/com/margelo/nitro/nitromaps/Camera+Validity.kt new file mode 100644 index 0000000..0f7551d --- /dev/null +++ b/package/android/src/main/java/com/margelo/nitro/nitromaps/Camera+Validity.kt @@ -0,0 +1,28 @@ +package com.margelo.nitro.nitromaps + +/** + * `CameraPosition.Builder.tilt` throws for a non-finite pitch, which unwinds the Fabric mount + * transaction, and a non-finite zoom or heading is taken silently and leaves the camera reading + * back as `NaN`. + */ +internal fun Camera.isValid(): Boolean = + isValidCoordinate(center.latitude, center.longitude) && + zoom.isDrawableAsFloatOrAbsent() && + heading.isDrawableAsFloatOrAbsent() && + pitch.isFiniteOrAbsent() && + altitude.isFiniteOrAbsent() + +/** + * `CameraPosition` holds zoom and bearing as `Float`, so the value the SDK receives is the + * converted one: a `Double` past `Float.MAX_VALUE` - `Double.MAX_VALUE` among them - becomes + * `Infinity`, which the builder takes without complaint and then normalizes into a `NaN` bearing. + * Checking after the conversion is what makes this guard match what the map is handed. + */ +private fun Double?.isDrawableAsFloatOrAbsent(): Boolean = this == null || toFloat().isFinite() + +/** + * Pitch and altitude stay `Double`: pitch is coerced into the drawable range before it is narrowed, + * and altitude never reaches `CameraPosition` at all. An absent value is filled in from the camera + * the map already has, so only a supplied one is checked. + */ +private fun Double?.isFiniteOrAbsent(): Boolean = this == null || isFinite() 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 bc1e31e..fa588a7 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 @@ -639,6 +639,14 @@ class GoogleMapProviderAdapter( 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. + 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) diff --git a/package/android/src/test/java/com/margelo/nitro/nitromaps/CameraValidityTest.kt b/package/android/src/test/java/com/margelo/nitro/nitromaps/CameraValidityTest.kt new file mode 100644 index 0000000..de9a3e8 --- /dev/null +++ b/package/android/src/test/java/com/margelo/nitro/nitromaps/CameraValidityTest.kt @@ -0,0 +1,103 @@ +package com.margelo.nitro.nitromaps + +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue +import org.junit.Test + +class CameraValidityTest { + @Test + fun acceptsACameraTheSdkCanRepresent() { + assertTrue(camera().isValid()) + assertTrue(camera(latitude = -90.0, longitude = 180.0).isValid()) + } + + @Test + fun acceptsACameraThatSuppliesNoFramingValues() { + assertTrue(camera(zoom = null, heading = null, pitch = null, altitude = null).isValid()) + } + + @Test + fun rejectsANonFiniteCenter() { + assertFalse(camera(latitude = Double.NaN, longitude = Double.NaN).isValid()) + assertFalse(camera(longitude = Double.POSITIVE_INFINITY).isValid()) + } + + @Test + fun rejectsACenterOutsideTheWorld() { + assertFalse(camera(latitude = 1000.0).isValid()) + assertFalse(camera(longitude = -180.0001).isValid()) + } + + @Test + fun rejectsANonFiniteFramingValue() { + assertFalse(camera(zoom = Double.NaN).isValid()) + assertFalse(camera(heading = Double.POSITIVE_INFINITY).isValid()) + assertFalse(camera(pitch = Double.NaN).isValid()) + assertFalse(camera(altitude = Double.NEGATIVE_INFINITY).isValid()) + } + + /** A pitch past the drawable range is clamped rather than rejected, so the camera still applies. */ + @Test + fun acceptsAPitchOutsideTheDrawableRange() { + assertTrue(camera(pitch = 120.0).isValid()) + assertTrue(camera(pitch = -10.0).isValid()) + } + + /** + * `CameraPosition.Builder.tilt` throws `IllegalArgumentException` for anything outside 0..90, so + * the clamp is what keeps a valid camera with an unsupported pitch from taking the mount + * transaction down. + */ + @Test + fun clampsAPitchTheSdkWouldRefuse() { + assertEquals(90.0f, camera(pitch = 120.0).toCameraPosition().tilt, 0.0f) + assertEquals(0.0f, camera(pitch = -10.0).toCameraPosition().tilt, 0.0f) + assertEquals(45.0f, camera(pitch = 45.0).toCameraPosition().tilt, 0.0f) + } + + /** + * Zoom and bearing are narrowed to `Float` on the way into `CameraPosition`, so a `Double` past + * `Float.MAX_VALUE` is not something the map can be handed, however finite it is as a `Double`. + */ + @Test + fun rejectsAFramingValueThatOverflowsAFloat() { + assertFalse(camera(zoom = Double.MAX_VALUE).isValid()) + assertFalse(camera(heading = -Double.MAX_VALUE).isValid()) + } + + /** + * What the guard above prevents, pinned against the real SDK: the builder takes the overflowed + * value without complaint, and its `% 360` normalization turns an infinite bearing into `NaN`. + */ + @Test + fun anOverflowingFramingValueWouldReachTheSdkAsInfinity() { + val position = camera(zoom = Double.MAX_VALUE, heading = Double.MAX_VALUE).toCameraPosition() + + assertEquals(Float.POSITIVE_INFINITY, position.zoom, 0.0f) + assertTrue(position.bearing.isNaN()) + } + + /** Pitch is coerced into the drawable range before it is narrowed, so an absurd one still draws. */ + @Test + fun clampsAPitchThatOverflowsAFloat() { + assertTrue(camera(pitch = Double.MAX_VALUE).isValid()) + assertEquals(90.0f, camera(pitch = Double.MAX_VALUE).toCameraPosition().tilt, 0.0f) + } + + private fun camera( + latitude: Double = 52.23, + longitude: Double = 21.01, + zoom: Double? = 12.0, + heading: Double? = 90.0, + pitch: Double? = 45.0, + altitude: Double? = 1000.0, + ): Camera = + Camera( + center = Coordinate(latitude = latitude, longitude = longitude), + zoom = zoom, + heading = heading, + pitch = pitch, + altitude = altitude, + ) +} diff --git a/package/ios/AppleMapProviderAdapter.swift b/package/ios/AppleMapProviderAdapter.swift index c7b792b..ac0b127 100644 --- a/package/ios/AppleMapProviderAdapter.swift +++ b/package/ios/AppleMapProviderAdapter.swift @@ -271,6 +271,15 @@ final class AppleMapProviderAdapter: MapProviderAdapter { } func updateMapCamera(_ camera: Camera, animated: Bool, duration: Double = 0) { + // `setCamera` raises an Objective-C NSException - `Invalid camera + // centerCoordinate` - from `-[MKMapCamera _validate]` for a center MapKit + // cannot place, and Swift cannot catch that. The framing values do not + // raise, but a non-finite one collapses the altitude or leaves + // `view.region` reading back as `NaN`. + guard camera.isValid else { + return + } + let mapCamera = camera.toMKMapCamera() guard !view.camera.approximatelyEquals(mapCamera) else { return diff --git a/package/ios/Camera+Validity.swift b/package/ios/Camera+Validity.swift new file mode 100644 index 0000000..1ca276d --- /dev/null +++ b/package/ios/Camera+Validity.swift @@ -0,0 +1,12 @@ +extension Camera { + /// Whether the camera can be handed to `MKMapView.camera` without MapKit raising. + var isValid: Bool { + center.isValid + && CameraFraming.isDrawable( + zoom: zoom, + heading: heading, + pitch: pitch, + altitude: altitude + ) + } +} diff --git a/package/ios/Geometry/CameraFraming.swift b/package/ios/Geometry/CameraFraming.swift new file mode 100644 index 0000000..af48414 --- /dev/null +++ b/package/ios/Geometry/CameraFraming.swift @@ -0,0 +1,44 @@ +import Foundation + +/// The framing half of a camera - zoom, heading, pitch and altitude - which +/// `CLLocationCoordinate2DIsValid` says nothing about. +enum CameraFraming { + /// Every value the caller supplied has to be a real number. `MKMapCamera` + /// does not reject a `NaN` one: it collapses the altitude to the minimum the + /// map allows, or leaves `MKMapView.region` reading back as `NaN`. On Android + /// a non-finite tilt throws out of `CameraPosition` instead. + static func isDrawable( + zoom: Double?, + heading: Double?, + pitch: Double?, + altitude: Double? + ) -> Bool { + isRealAsFloatOrAbsent(zoom) + && isRealOrAbsent(heading) + && isRealOrAbsent(pitch) + && isRealOrAbsent(altitude) + } + + /// `GMSCameraPosition` holds zoom as a `Float`, and `CameraPosition` does the + /// same on Android, so the value the SDK receives is the narrowed one: a + /// `Double` past `Float.greatestFiniteMagnitude` arrives as `infinity`. Zoom + /// is the only value narrowed that way - heading, pitch and altitude stay + /// `Double` through `CLLocationDirection` and `MKMapCamera`. + private static func isRealAsFloatOrAbsent(_ value: Double?) -> Bool { + guard let value else { + return true + } + + return value.isFinite && Float(value).isFinite + } + + /// An omitted value is filled in from the camera the map already has, so only + /// a supplied one has to be checked. + private static func isRealOrAbsent(_ value: Double?) -> Bool { + guard let value else { + return true + } + + return value.isFinite + } +} diff --git a/package/ios/GoogleMapProviderAdapter.swift b/package/ios/GoogleMapProviderAdapter.swift index 898f071..b33a967 100644 --- a/package/ios/GoogleMapProviderAdapter.swift +++ b/package/ios/GoogleMapProviderAdapter.swift @@ -32,8 +32,10 @@ final class GoogleMapProviderAdapter: NSObject, MapProviderAdapter { } lazy var view: GMSMapView = { - let camera = - self.camera?.toGMSCameraPosition() + // The map is created from the stored prop directly, so it is checked here + // too: `updateMapCamera` never runs for the camera the map starts with. + let initialCamera = + self.camera.flatMap { $0.isValid ? $0.toGMSCameraPosition() : nil } ?? GMSCameraPosition(latitude: 0, longitude: 0, zoom: 10) let mapView: GMSMapView if let googleMapId = _googleMapId?.trimmingCharacters(in: .whitespacesAndNewlines), @@ -42,10 +44,10 @@ final class GoogleMapProviderAdapter: NSObject, MapProviderAdapter { mapView = GMSMapView( frame: .zero, mapID: GMSMapID(identifier: googleMapId), - camera: camera + camera: initialCamera ) } else { - mapView = GMSMapView(frame: .zero, camera: camera) + mapView = GMSMapView(frame: .zero, camera: initialCamera) } mapView.delegate = self @@ -320,6 +322,10 @@ final class GoogleMapProviderAdapter: NSObject, MapProviderAdapter { } private func updateMapCamera(_ camera: Camera, animated: Bool, duration: Double? = nil) { + guard camera.isValid else { + return + } + let target = camera.toGMSCameraPosition(current: view.camera) guard !view.camera.approximatelyEquals(target) else { return diff --git a/package/ios/Tests/Geometry/CameraFramingTests.swift b/package/ios/Tests/Geometry/CameraFramingTests.swift new file mode 100644 index 0000000..28b0804 --- /dev/null +++ b/package/ios/Tests/Geometry/CameraFramingTests.swift @@ -0,0 +1,49 @@ +import Testing + +@testable import NitroMapsGeometry + +@Test +func acceptsFramingValuesTheMapCanUse() { + #expect(CameraFraming.isDrawable(zoom: 12, heading: 90, pitch: 45, altitude: 1000)) + #expect(CameraFraming.isDrawable(zoom: 0, heading: 0, pitch: 0, altitude: 0)) +} + +@Test +func acceptsAbsentFramingValues() { + #expect(CameraFraming.isDrawable(zoom: nil, heading: nil, pitch: nil, altitude: nil)) + #expect(CameraFraming.isDrawable(zoom: 12, heading: nil, pitch: nil, altitude: nil)) +} + +@Test +func rejectsANonFiniteFramingValue() { + #expect(!CameraFraming.isDrawable(zoom: .nan, heading: nil, pitch: nil, altitude: nil)) + #expect(!CameraFraming.isDrawable(zoom: nil, heading: .infinity, pitch: nil, altitude: nil)) + #expect(!CameraFraming.isDrawable(zoom: nil, heading: nil, pitch: .nan, altitude: nil)) + #expect(!CameraFraming.isDrawable(zoom: nil, heading: nil, pitch: nil, altitude: -.infinity)) +} + +/// A pitch past the range the SDKs draw is pulled back to it rather than +/// rejected, so the camera still reaches the map. +@Test +func acceptsAPitchOutsideTheDrawableRange() { + #expect(CameraFraming.isDrawable(zoom: nil, heading: nil, pitch: 120, altitude: nil)) + #expect(CameraFraming.isDrawable(zoom: nil, heading: nil, pitch: -10, altitude: nil)) +} + +/// Zoom is narrowed to a `Float` on its way into both SDKs, so a `Double` too +/// large for one is not a zoom the map can be handed, however finite it is. +@Test +func rejectsAZoomThatOverflowsAFloat() { + #expect(!CameraFraming.isDrawable(zoom: .greatestFiniteMagnitude, heading: nil, pitch: nil, altitude: nil)) + #expect(!CameraFraming.isDrawable(zoom: -.greatestFiniteMagnitude, heading: nil, pitch: nil, altitude: nil)) + #expect(CameraFraming.isDrawable(zoom: 3.4e38, heading: nil, pitch: nil, altitude: nil)) +} + +/// Heading, pitch and altitude stay `Double` all the way to `MKMapCamera` and +/// `CLLocationDirection`, so a large one is still a value the map can take. +@Test +func keepsALargeHeadingPitchOrAltitude() { + #expect(CameraFraming.isDrawable(zoom: nil, heading: .greatestFiniteMagnitude, pitch: nil, altitude: nil)) + #expect(CameraFraming.isDrawable(zoom: nil, heading: nil, pitch: .greatestFiniteMagnitude, altitude: nil)) + #expect(CameraFraming.isDrawable(zoom: nil, heading: nil, pitch: nil, altitude: .greatestFiniteMagnitude)) +} diff --git a/package/src/camera/__tests__/isValidCamera.test.ts b/package/src/camera/__tests__/isValidCamera.test.ts new file mode 100644 index 0000000..d63348b --- /dev/null +++ b/package/src/camera/__tests__/isValidCamera.test.ts @@ -0,0 +1,99 @@ +import { describe, expect, test } from 'bun:test'; +import { isValidCamera } from '../isValidCamera'; + +const validCamera = { + center: { latitude: 52.23, longitude: 21.01 }, + zoom: 12, + heading: 90, + pitch: 45, + altitude: 1000, +}; + +describe('isValidCamera', () => { + test('accepts a camera both SDKs can represent', () => { + expect(isValidCamera(validCamera)).toBe(true); + expect(isValidCamera({ center: { latitude: -90, longitude: 180 } })).toBe( + true, + ); + }); + + test('accepts a camera that supplies no framing values', () => { + expect(isValidCamera({ center: { latitude: 0, longitude: 0 } })).toBe(true); + }); + + test('rejects a missing camera', () => { + expect(isValidCamera(undefined)).toBe(false); + }); + + test('rejects a non-finite center', () => { + expect( + isValidCamera({ + ...validCamera, + center: { latitude: Number.NaN, longitude: Number.NaN }, + }), + ).toBe(false); + expect( + isValidCamera({ + ...validCamera, + center: { latitude: 0, longitude: Number.POSITIVE_INFINITY }, + }), + ).toBe(false); + }); + + test('rejects a center outside the world', () => { + expect( + isValidCamera({ + ...validCamera, + center: { latitude: 1000, longitude: 0 }, + }), + ).toBe(false); + expect( + isValidCamera({ + ...validCamera, + center: { latitude: 0, longitude: 181 }, + }), + ).toBe(false); + }); + + test('rejects a non-finite framing value', () => { + expect(isValidCamera({ ...validCamera, zoom: Number.NaN })).toBe(false); + expect( + isValidCamera({ ...validCamera, heading: Number.POSITIVE_INFINITY }), + ).toBe(false); + expect(isValidCamera({ ...validCamera, pitch: Number.NaN })).toBe(false); + expect( + isValidCamera({ ...validCamera, altitude: Number.NEGATIVE_INFINITY }), + ).toBe(false); + }); + + // Android throws on a tilt past 90 and MapKit flattens it; both are handled + // natively by clamping, so the camera itself stays usable. + test('accepts a pitch outside the range the SDKs draw', () => { + expect(isValidCamera({ ...validCamera, pitch: 120 })).toBe(true); + expect(isValidCamera({ ...validCamera, pitch: -10 })).toBe(true); + }); + + // Zoom, and bearing on Android, are narrowed to a 32-bit float before the SDK + // sees them, so a number past that range arrives as `Infinity` however finite + // it is here. + test('rejects a framing value that overflows a 32-bit float', () => { + expect(isValidCamera({ ...validCamera, zoom: Number.MAX_VALUE })).toBe( + false, + ); + expect(isValidCamera({ ...validCamera, heading: -Number.MAX_VALUE })).toBe( + false, + ); + expect(isValidCamera({ ...validCamera, zoom: 3.4e38 })).toBe(true); + }); + + // Pitch and altitude stay 64-bit on both platforms - pitch is clamped before + // it is narrowed, and altitude never reaches the Android camera at all. + test('accepts a large pitch or altitude', () => { + expect(isValidCamera({ ...validCamera, pitch: Number.MAX_VALUE })).toBe( + true, + ); + expect(isValidCamera({ ...validCamera, altitude: Number.MAX_VALUE })).toBe( + true, + ); + }); +}); diff --git a/package/src/camera/__tests__/resolveCameraProp.test.ts b/package/src/camera/__tests__/resolveCameraProp.test.ts new file mode 100644 index 0000000..86e69c4 --- /dev/null +++ b/package/src/camera/__tests__/resolveCameraProp.test.ts @@ -0,0 +1,90 @@ +import { afterEach, beforeEach, describe, expect, spyOn, test } from 'bun:test'; +import { resolveCameraProp } from '../resolveCameraProp'; + +const warnSpy = spyOn(console, 'warn'); +const previousDev = (globalThis as { __DEV__?: boolean }).__DEV__; + +const validCamera = { + center: { latitude: 52.23, longitude: 21.01 }, + zoom: 12, +}; + +function restoreDevFlag(): void { + const globalDev = globalThis as { __DEV__?: boolean }; + if (previousDev === undefined) { + delete globalDev.__DEV__; + return; + } + + globalDev.__DEV__ = previousDev; +} + +beforeEach(() => { + warnSpy.mockClear(); +}); + +afterEach(() => { + warnSpy.mockClear(); + restoreDevFlag(); +}); + +describe('resolveCameraProp', () => { + test('passes a valid camera through unchanged', () => { + (globalThis as { __DEV__?: boolean }).__DEV__ = true; + + expect(resolveCameraProp(validCamera, undefined)).toBe(validCamera); + expect(warnSpy).not.toHaveBeenCalled(); + }); + + test('leaves a missing camera alone without warning', () => { + (globalThis as { __DEV__?: boolean }).__DEV__ = true; + + expect(resolveCameraProp(undefined, undefined)).toBeUndefined(); + expect(warnSpy).not.toHaveBeenCalled(); + }); + + test('holds the last accepted camera when the prop is unset', () => { + (globalThis as { __DEV__?: boolean }).__DEV__ = true; + + // `camera={following ? camera : undefined}` on a mounted view: unsetting it + // reaches the native view as `null`, which throws in the struct converter. + expect(resolveCameraProp(undefined, validCamera)).toBe(validCamera); + expect(warnSpy).not.toHaveBeenCalled(); + }); + + test('drops an invalid camera and warns in development', () => { + (globalThis as { __DEV__?: boolean }).__DEV__ = true; + + expect( + resolveCameraProp( + { center: { latitude: Number.NaN, longitude: Number.NaN } }, + undefined, + ), + ).toBeUndefined(); + + expect(warnSpy).toHaveBeenCalledTimes(1); + expect(warnSpy.mock.calls[0]?.[0]).toContain('camera ignored'); + }); + + test('holds the last accepted camera instead of unsetting the prop', () => { + (globalThis as { __DEV__?: boolean }).__DEV__ = true; + + // Unsetting it would reach the native view as `null`, which the generated + // struct converter rejects before any native guard runs. + expect( + resolveCameraProp({ ...validCamera, pitch: Number.NaN }, validCamera), + ).toBe(validCamera); + }); + + test('drops an invalid camera silently outside development', () => { + (globalThis as { __DEV__?: boolean }).__DEV__ = false; + + expect( + resolveCameraProp( + { center: { latitude: 1000, longitude: 0 } }, + undefined, + ), + ).toBeUndefined(); + expect(warnSpy).not.toHaveBeenCalled(); + }); +}); diff --git a/package/src/camera/isValidCamera.ts b/package/src/camera/isValidCamera.ts new file mode 100644 index 0000000..9b57d73 --- /dev/null +++ b/package/src/camera/isValidCamera.ts @@ -0,0 +1,39 @@ +import type { Camera } from '../types/camera'; +import { isValidCoordinate } from '../utils/validateGeometry'; + +// An omitted value is filled in from the camera the map already has, so only a +// supplied one has to be a real number. +function isFiniteOrAbsent(value: number | undefined): boolean { + return value == null || Number.isFinite(value); +} + +// Both SDKs keep zoom as a 32-bit float, and `CameraPosition` keeps bearing as +// one too, so the value they receive is the narrowed one, and anything past +// this range arrives as `Infinity`. The native guards check the narrowed value +// itself; this one is what turns it into a warning, and rejects the last ulp +// either way rather than depending on how the runtime rounds it. +const LARGEST_FLOAT_32 = 3.4028234663852886e38; + +function isDrawableAsFloatOrAbsent(value: number | undefined): boolean { + return ( + value == null || + (Number.isFinite(value) && Math.abs(value) <= LARGEST_FLOAT_32) + ); +} + +/** + * A camera reaches `MKMapCamera` and `CameraPosition` unchanged. MapKit raises + * an uncatchable `NSException` for a center it cannot place, and Google Maps + * throws for a non-finite pitch; the remaining non-finite values are taken + * silently and leave the camera reading back as `NaN`. + */ +export function isValidCamera(value: Camera | undefined): boolean { + return ( + value != null && + isValidCoordinate(value.center) && + isDrawableAsFloatOrAbsent(value.zoom) && + isDrawableAsFloatOrAbsent(value.heading) && + isFiniteOrAbsent(value.pitch) && + isFiniteOrAbsent(value.altitude) + ); +} diff --git a/package/src/camera/resolveCameraProp.ts b/package/src/camera/resolveCameraProp.ts new file mode 100644 index 0000000..8b4b4a8 --- /dev/null +++ b/package/src/camera/resolveCameraProp.ts @@ -0,0 +1,31 @@ +import type { Camera } from '../types/camera'; +import { isValidCamera } from './isValidCamera'; +import { warnCamera } from './warnCamera'; + +// Once the view has accepted a camera the prop must never go back to +// `undefined`, whether it was unset or is unusable - the invariant +// `resolveRegionProp` documents: React rewrites a removed prop to `null`, the +// optional JSI converter only short-circuits on `undefined`, and the generated +// struct converter then calls `asObject` on a null and throws, before any +// native guard runs. Holding the last accepted camera leaves the prop +// untouched instead, which is also what the map should show. Before anything +// has been accepted there is nothing to hold, and `undefined` at mount is +// safe: React omits the key entirely. +export function resolveCameraProp( + camera: Camera | undefined, + lastAccepted: Camera | undefined, +): Camera | undefined { + if (camera == null) { + return lastAccepted; + } + + if (isValidCamera(camera)) { + return camera; + } + + warnCamera( + 'camera ignored: invalid center coordinate, or a zoom, heading, pitch or altitude the map cannot use', + ); + + return lastAccepted; +} diff --git a/package/src/camera/useValidCamera.ts b/package/src/camera/useValidCamera.ts new file mode 100644 index 0000000..3196b46 --- /dev/null +++ b/package/src/camera/useValidCamera.ts @@ -0,0 +1,25 @@ +import { useLayoutEffect, useMemo, useRef } from 'react'; +import type { Camera } from '../types/camera'; +import { resolveCameraProp } from './resolveCameraProp'; + +/** + * Holds the last camera the native view accepted, so an unset or invalid one + * never makes the prop transition back to `undefined`. + * + * The counterpart of `useValidRegion`, and memoized the same way: on camera + * identity, so a stable object is checked - and warned about - once rather than + * on every render, with the ref written after commit. + */ +export function useValidCamera(camera: Camera | undefined): Camera | undefined { + const lastAccepted = useRef(undefined); + const accepted = useMemo( + () => resolveCameraProp(camera, lastAccepted.current), + [camera], + ); + + useLayoutEffect(() => { + lastAccepted.current = accepted; + }, [accepted]); + + return accepted; +} diff --git a/package/src/camera/warnCamera.ts b/package/src/camera/warnCamera.ts new file mode 100644 index 0000000..0fc99da --- /dev/null +++ b/package/src/camera/warnCamera.ts @@ -0,0 +1,3 @@ +import { createWarn } from '../utils/warn'; + +export const warnCamera = createWarn('Camera'); diff --git a/package/src/components/MapView.tsx b/package/src/components/MapView.tsx index 040a046..7c6f9af 100644 --- a/package/src/components/MapView.tsx +++ b/package/src/components/MapView.tsx @@ -6,6 +6,7 @@ import { 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'; @@ -134,6 +135,7 @@ export function MapView({ enteringAnimationsEqual, ); const validRegion = useValidRegion(region); + const validCamera = useValidCamera(camera); const hasMarkerPress = onMarkerPressProp != null || hasCollectedMarkerPress; @@ -285,7 +287,7 @@ export function MapView({ googleMapId={googleMapId} mapType={mapType} region={validRegion} - camera={camera} + camera={validCamera} scrollEnabled={scrollEnabled} zoomEnabled={zoomEnabled} rotateEnabled={rotateEnabled}