From 9f3e36328d0cc9e2892214779bec8bf1f98c5adf Mon Sep 17 00:00:00 2001 From: Jakub Kasprzyk Date: Tue, 25 Aug 2026 00:47:07 +0200 Subject: [PATCH 1/4] fix: stop re-serializing overlay props on unrelated re-renders Nitro diffs view props by reference identity, but MapView rebuilt every overlay array, callback envelope and entering-animation descriptor on each render. A parent state change with unchanged map data marked up to 19 props dirty, paying a full JSI conversion of the descriptor arrays on the JS thread plus a native re-apply of every one of them. Compare descriptors structurally and hand back the previous value when nothing changed, memoize the callback(...) envelopes on the handler they wrap, and create the hybridRef envelope once per mount. An unchanged re-render now reaches native with zero dirty props. normalizeMarkerDescriptors no longer tries to preserve array identity; that job now belongs to the stabilizer, which does it correctly for every field. Descriptors are treated as immutable: mutating an object already handed to MapView is not picked up, because the comparison sees the same object on both sides. --- package/src/components/MapView.tsx | 143 ++++-- package/src/hooks/useNitroCallback.ts | 16 + package/src/hooks/useStableValue.ts | 24 + .../__tests__/descriptorEquality.test.ts | 426 ++++++++++++++++++ .../normalizeMarkerDescriptors.test.ts | 65 +-- package/src/overlays/descriptorEquality.ts | 222 +++++++++ .../overlays/normalizeMarkerDescriptors.ts | 44 +- .../utils/__tests__/enteringAnimation.test.ts | 46 ++ 8 files changed, 874 insertions(+), 112 deletions(-) create mode 100644 package/src/hooks/useNitroCallback.ts create mode 100644 package/src/hooks/useStableValue.ts create mode 100644 package/src/overlays/__tests__/descriptorEquality.test.ts create mode 100644 package/src/overlays/descriptorEquality.ts create mode 100644 package/src/utils/__tests__/enteringAnimation.test.ts diff --git a/package/src/components/MapView.tsx b/package/src/components/MapView.tsx index 122eadb..132b08e 100644 --- a/package/src/components/MapView.tsx +++ b/package/src/components/MapView.tsx @@ -1,11 +1,26 @@ -import { useCallback, useImperativeHandle, useMemo, useRef, type Ref, type RefObject } from 'react'; -import { callback } from 'react-native-nitro-modules'; +import { + useCallback, + useImperativeHandle, + useMemo, + useRef, + type Ref, + type RefObject, +} from 'react'; import { useCollectedOverlays } from '../hooks/useCollectedOverlays'; +import { useNitroCallback } from '../hooks/useNitroCallback'; +import { useStableValue } from '../hooks/useStableValue'; import { NativeMapView } from '../native/MapViewNative'; import type { MapView as NativeMapViewHybrid, NativePoiPressEvent, } from '../native/specs/MapView.nitro'; +import { + circleListsEqual, + enteringAnimationsEqual, + markerListsEqual, + polygonListsEqual, + polylineListsEqual, +} from '../overlays/descriptorEquality'; import { OverlayType, overlayCallbackKey } from '../overlays/overlayType'; import { normalizeMarkerDescriptors } from '../overlays/normalizeMarkerDescriptors'; import { resolveMapProvider } from '../providers'; @@ -82,18 +97,39 @@ export function MapView({ hasCirclePress, } = useCollectedOverlays(children); const normalizedBulkMarkers = useMemo( - () => (markersProp != null ? normalizeMarkerDescriptors(markersProp) : null), + () => + markersProp != null ? normalizeMarkerDescriptors(markersProp) : null, [markersProp], ); - const markers = - normalizedBulkMarkers != null ? normalizedBulkMarkers : collectedMarkers; - const polylines = - polylinesProp != null ? polylinesProp : collectedPolylines; - const polygons = - polygonsProp != null ? polygonsProp : collectedPolygons; - const circles = - circlesProp != null ? circlesProp : collectedCircles; + // Everything below is rebuilt whenever `children`, a bulk prop or an animation + // prop changes identity - which for inline JSX is every render. Nitro would + // re-serialize each one across JSI, so hand back the previous value when + // nothing actually changed. + const markers = useStableValue( + normalizedBulkMarkers ?? collectedMarkers, + markerListsEqual, + ); + const polylines = useStableValue( + polylinesProp ?? collectedPolylines, + polylineListsEqual, + ); + const polygons = useStableValue( + polygonsProp ?? collectedPolygons, + polygonListsEqual, + ); + const circles = useStableValue( + circlesProp ?? collectedCircles, + circleListsEqual, + ); + const markerEntering = useStableValue( + normalizeEnteringAnimation(markerEnteringAnimation), + enteringAnimationsEqual, + ); + const clusterEntering = useStableValue( + normalizeEnteringAnimation(clusterEnteringAnimation), + enteringAnimationsEqual, + ); const hasMarkerPress = onMarkerPressProp != null || hasCollectedMarkerPress; @@ -109,6 +145,10 @@ export function MapView({ | ((event: PoiPressEvent) => void) | undefined; + const handleHybridRef = useCallback((nativeRef: NativeMapViewHybrid) => { + hybridRef.current = nativeRef; + }, []); + const handleMarkerPress = useCallback( (id: string) => { callbackRegistry.current.get(overlayCallbackKey(OverlayType.Marker, id))?.onPress?.(); @@ -176,6 +216,37 @@ export function MapView({ [onPoiPressCallback], ); + // Nitro's `callback(...)` envelope is a fresh object per call, so each of + // these is memoized on the handler it wraps. An inline arrow passed by the + // caller still changes identity every render - that part is theirs to hoist. + const hybridRefCallback = useNitroCallback(handleHybridRef); + const onRegionChangeCallback = useNitroCallback(onRegionChange); + const onRegionChangeCompleteCallback = useNitroCallback( + onRegionChangeComplete, + ); + const onMapReadyCallback = useNitroCallback(onMapReady); + const onPressCallback = useNitroCallback(onPress); + const onPoiPressNativeCallback = useNitroCallback( + onPoiPress == null ? undefined : handlePoiPress, + ); + const onLongPressCallback = useNitroCallback(onLongPress); + const onClusterPressCallback = useNitroCallback(onClusterPress); + const onMarkerPressCallback = useNitroCallback( + hasMarkerPress ? handleMarkerPress : undefined, + ); + const onMarkerDragEndCallback = useNitroCallback( + hasMarkerDragEnd ? handleMarkerDragEnd : undefined, + ); + const onPolylinePressCallback = useNitroCallback( + hasPolylinePressHandler ? handlePolylinePress : undefined, + ); + const onPolygonPressCallback = useNitroCallback( + hasPolygonPressHandler ? handlePolygonPress : undefined, + ); + const onCirclePressCallback = useNitroCallback( + hasCirclePressHandler ? handleCirclePress : undefined, + ); + useImperativeHandle( ref, () => ({ @@ -201,9 +272,7 @@ export function MapView({ { - hybridRef.current = nativeRef; - })} + hybridRef={hybridRefCallback} provider={resolvedProvider} googleMapId={googleMapId} mapType={mapType} @@ -220,42 +289,24 @@ export function MapView({ customMapStyle={customMapStyle} clusteringEnabled={clusteringEnabled} mapPadding={mapPadding} - markerEnteringAnimation={normalizeEnteringAnimation(markerEnteringAnimation)} - clusterEnteringAnimation={normalizeEnteringAnimation(clusterEnteringAnimation)} + markerEnteringAnimation={markerEntering} + clusterEnteringAnimation={clusterEntering} markers={markers} polylines={polylines} polygons={polygons} circles={circles} - onRegionChange={ - onRegionChange == null ? undefined : callback(onRegionChange) - } - onRegionChangeComplete={ - onRegionChangeComplete == null - ? undefined - : callback(onRegionChangeComplete) - } - onMapReady={onMapReady == null ? undefined : callback(onMapReady)} - onPress={onPress == null ? undefined : callback(onPress)} - onPoiPress={onPoiPress == null ? undefined : callback(handlePoiPress)} - onLongPress={onLongPress == null ? undefined : callback(onLongPress)} - onClusterPress={ - onClusterPress == null ? undefined : callback(onClusterPress) - } - onMarkerPress={ - hasMarkerPress ? callback(handleMarkerPress) : undefined - } - onMarkerDragEnd={ - hasMarkerDragEnd ? callback(handleMarkerDragEnd) : undefined - } - onPolylinePress={ - hasPolylinePressHandler ? callback(handlePolylinePress) : undefined - } - onPolygonPress={ - hasPolygonPressHandler ? callback(handlePolygonPress) : undefined - } - onCirclePress={ - hasCirclePressHandler ? callback(handleCirclePress) : undefined - } + onRegionChange={onRegionChangeCallback} + onRegionChangeComplete={onRegionChangeCompleteCallback} + onMapReady={onMapReadyCallback} + onPress={onPressCallback} + onPoiPress={onPoiPressNativeCallback} + onLongPress={onLongPressCallback} + onClusterPress={onClusterPressCallback} + onMarkerPress={onMarkerPressCallback} + onMarkerDragEnd={onMarkerDragEndCallback} + onPolylinePress={onPolylinePressCallback} + onPolygonPress={onPolygonPressCallback} + onCirclePress={onCirclePressCallback} /> ); } diff --git a/package/src/hooks/useNitroCallback.ts b/package/src/hooks/useNitroCallback.ts new file mode 100644 index 0000000..0565f2d --- /dev/null +++ b/package/src/hooks/useNitroCallback.ts @@ -0,0 +1,16 @@ +import { useMemo } from 'react'; +import { callback } from 'react-native-nitro-modules'; + +/** + * Wraps {@linkcode handler} in Nitro's `{ f }` callback envelope, reusing the + * same envelope for as long as the handler itself is stable. + * + * `callback(...)` allocates a new object on every call and Nitro diffs view + * props by reference identity, so wrapping inline in JSX marks every event prop + * dirty on each render - re-converting the function across JSI and re-applying + * it to the native view. Passing `undefined` through is intentional: `callback` + * returns it unchanged, which is how an unset handler is expressed. + */ +export function useNitroCallback(handler: Handler) { + return useMemo(() => callback(handler), [handler]); +} diff --git a/package/src/hooks/useStableValue.ts b/package/src/hooks/useStableValue.ts new file mode 100644 index 0000000..ea5bdf1 --- /dev/null +++ b/package/src/hooks/useStableValue.ts @@ -0,0 +1,24 @@ +import { useRef } from 'react'; + +/** + * Keeps the previously returned value when {@linkcode next} is structurally + * equal to it, as judged by {@linkcode isEqual}. + * + * Nitro compares view props by reference identity, so a value rebuilt from + * unchanged data would otherwise be converted across JSI and re-applied to the + * native view on every render of the component holding the `MapView`. For an + * overlay array that means walking it into a `std::vector`, bridging it into a + * native array and reconciling it against the map - all far more expensive than + * the comparison done here. + */ +export function useStableValue( + next: Value, + isEqual: (left: Value, right: Value) => boolean, +): Value { + const previous = useRef(next); + const stable = isEqual(previous.current, next) ? previous.current : next; + + previous.current = stable; + + return stable; +} diff --git a/package/src/overlays/__tests__/descriptorEquality.test.ts b/package/src/overlays/__tests__/descriptorEquality.test.ts new file mode 100644 index 0000000..938c7c6 --- /dev/null +++ b/package/src/overlays/__tests__/descriptorEquality.test.ts @@ -0,0 +1,426 @@ +import { describe, expect, test } from 'bun:test'; +import type { + CircleDescriptor, + MarkerDescriptor, + PolygonDescriptor, + PolylineDescriptor, +} from '../../native/specs/overlays'; +import { + circleDescriptorsEqual, + circleListsEqual, + descriptorListsEqual, + enteringAnimationsEqual, + markerDescriptorsEqual, + markerListsEqual, + polygonDescriptorsEqual, + polygonListsEqual, + polylineDescriptorsEqual, + polylineListsEqual, +} from '../descriptorEquality'; + +/** + * These comparators decide whether the overlay arrays keep their identity, and + * Nitro skips a prop whose identity is unchanged. A field the comparator misses + * is therefore a map update that never reaches the native side, so every field + * of every descriptor gets a case here. + * + * Each case returns a new descriptor rather than mutating one, so a case only + * changes the field it names. + */ +type Change = [string, (base: Descriptor) => Descriptor]; + +function describeFieldCoverage( + name: string, + base: Descriptor, + descriptorsEqual: (left: Descriptor, right: Descriptor) => boolean, + changes: Array>, +) { + describe(name, () => { + test('treats a structural clone as equal', () => { + expect(descriptorsEqual(base, structuredClone(base))).toBe(true); + }); + + test('treats the same reference as equal', () => { + expect(descriptorsEqual(base, base)).toBe(true); + }); + + for (const [field, change] of changes) { + test(`detects a change to ${field}`, () => { + expect(descriptorsEqual(base, change(base))).toBe(false); + }); + } + }); +} + +const baseMarker: MarkerDescriptor = { + id: 'marker-1', + coordinate: { latitude: 52.2297, longitude: 21.0122 }, + title: 'Warsaw', + subtitle: 'Capital', + draggable: true, + clusterable: true, + image: { uri: 'asset:/pin.png', width: 32, height: 48, scale: 2 }, + anchor: { x: 0.5, y: 1 }, + centerOffset: { x: 1, y: -2 }, + rotation: 45, + flat: true, + opacity: 0.9, + enteringAnimation: { + kind: 'fade', + duration: 200, + delay: 50, + reduceMotion: 'system', + }, +}; + +describeFieldCoverage( + 'markerDescriptorsEqual', + baseMarker, + markerDescriptorsEqual, + [ + ['id', (d) => ({ ...d, id: 'marker-2' })], + [ + 'coordinate.latitude', + (d) => ({ ...d, coordinate: { ...d.coordinate, latitude: 0 } }), + ], + [ + 'coordinate.longitude', + (d) => ({ ...d, coordinate: { ...d.coordinate, longitude: 0 } }), + ], + ['title', (d) => ({ ...d, title: 'Krakow' })], + ['a cleared title', (d) => ({ ...d, title: undefined })], + ['subtitle', (d) => ({ ...d, subtitle: 'Other' })], + ['draggable', (d) => ({ ...d, draggable: false })], + ['clusterable', (d) => ({ ...d, clusterable: false })], + [ + 'image.uri', + (d) => ({ ...d, image: { ...d.image, uri: 'asset:/other.png' } }), + ], + [ + 'image.width', + (d) => ({ + ...d, + image: { ...d.image, uri: 'asset:/pin.png', width: 33 }, + }), + ], + [ + 'image.height', + (d) => ({ + ...d, + image: { ...d.image, uri: 'asset:/pin.png', height: 49 }, + }), + ], + [ + 'image.scale', + (d) => ({ ...d, image: { ...d.image, uri: 'asset:/pin.png', scale: 3 } }), + ], + ['a cleared image', (d) => ({ ...d, image: undefined })], + ['anchor.x', (d) => ({ ...d, anchor: { x: 0, y: 1 } })], + ['anchor.y', (d) => ({ ...d, anchor: { x: 0.5, y: 0 } })], + ['a cleared anchor', (d) => ({ ...d, anchor: undefined })], + ['centerOffset.x', (d) => ({ ...d, centerOffset: { x: 9, y: -2 } })], + ['centerOffset.y', (d) => ({ ...d, centerOffset: { x: 1, y: 9 } })], + ['a cleared centerOffset', (d) => ({ ...d, centerOffset: undefined })], + ['rotation', (d) => ({ ...d, rotation: 90 })], + ['flat', (d) => ({ ...d, flat: false })], + ['opacity', (d) => ({ ...d, opacity: 0.5 })], + [ + 'enteringAnimation.kind', + (d) => ({ + ...d, + enteringAnimation: { ...d.enteringAnimation, kind: 'fade-scale' }, + }), + ], + [ + 'enteringAnimation.duration', + (d) => ({ + ...d, + enteringAnimation: { + ...d.enteringAnimation, + kind: 'fade', + duration: 400, + }, + }), + ], + [ + 'enteringAnimation.delay', + (d) => ({ + ...d, + enteringAnimation: { ...d.enteringAnimation, kind: 'fade', delay: 0 }, + }), + ], + [ + 'enteringAnimation.reduceMotion', + (d) => ({ + ...d, + enteringAnimation: { + ...d.enteringAnimation, + kind: 'fade', + reduceMotion: 'never', + }, + }), + ], + [ + 'a cleared enteringAnimation', + (d) => ({ ...d, enteringAnimation: undefined }), + ], + ], +); + +const basePolyline: PolylineDescriptor = { + id: 'polyline-1', + coordinates: [ + { latitude: 52.2297, longitude: 21.0122 }, + { latitude: 52.237, longitude: 21.017 }, + ], + strokeColor: '#FF0000', + strokeWidth: 3, + tappable: true, +}; + +describeFieldCoverage( + 'polylineDescriptorsEqual', + basePolyline, + polylineDescriptorsEqual, + [ + ['id', (d) => ({ ...d, id: 'polyline-2' })], + [ + 'a coordinate', + (d) => ({ + ...d, + coordinates: [d.coordinates[0], { latitude: 53, longitude: 21.017 }], + }), + ], + [ + 'the coordinate count', + (d) => ({ ...d, coordinates: d.coordinates.slice(0, -1) }), + ], + [ + 'the coordinate order', + (d) => ({ ...d, coordinates: [...d.coordinates].reverse() }), + ], + ['strokeColor', (d) => ({ ...d, strokeColor: '#00FF00' })], + ['strokeWidth', (d) => ({ ...d, strokeWidth: 4 })], + ['tappable', (d) => ({ ...d, tappable: false })], + ], +); + +const basePolygon: PolygonDescriptor = { + id: 'polygon-1', + coordinates: [ + { latitude: 52.2297, longitude: 21.0122 }, + { latitude: 52.237, longitude: 21.017 }, + { latitude: 52.24, longitude: 21.03 }, + ], + fillColor: '#0000FF80', + strokeColor: '#0000FF', + strokeWidth: 2, + tappable: true, +}; + +describeFieldCoverage( + 'polygonDescriptorsEqual', + basePolygon, + polygonDescriptorsEqual, + [ + ['id', (d) => ({ ...d, id: 'polygon-2' })], + [ + 'a coordinate', + (d) => ({ + ...d, + coordinates: [ + { latitude: 52.2297, longitude: 22 }, + ...d.coordinates.slice(1), + ], + }), + ], + [ + 'the coordinate count', + (d) => ({ ...d, coordinates: d.coordinates.slice(0, -1) }), + ], + ['fillColor', (d) => ({ ...d, fillColor: '#00FF0080' })], + ['strokeColor', (d) => ({ ...d, strokeColor: '#00FF00' })], + ['strokeWidth', (d) => ({ ...d, strokeWidth: 5 })], + ['tappable', (d) => ({ ...d, tappable: false })], + ], +); + +const baseCircle: CircleDescriptor = { + id: 'circle-1', + center: { latitude: 52.2297, longitude: 21.0122 }, + radius: 500, + fillColor: '#0000FF80', + strokeColor: '#0000FF', + strokeWidth: 2, + tappable: true, +}; + +describeFieldCoverage( + 'circleDescriptorsEqual', + baseCircle, + circleDescriptorsEqual, + [ + ['id', (d) => ({ ...d, id: 'circle-2' })], + [ + 'center.latitude', + (d) => ({ ...d, center: { ...d.center, latitude: 53 } }), + ], + [ + 'center.longitude', + (d) => ({ ...d, center: { ...d.center, longitude: 22 } }), + ], + ['radius', (d) => ({ ...d, radius: 600 })], + ['fillColor', (d) => ({ ...d, fillColor: '#00FF0080' })], + ['strokeColor', (d) => ({ ...d, strokeColor: '#00FF00' })], + ['strokeWidth', (d) => ({ ...d, strokeWidth: 5 })], + ['tappable', (d) => ({ ...d, tappable: false })], + ], +); + +describe('shared nested objects', () => { + // Descriptors alias the objects handed in by the caller, so two distinct + // descriptors can share one coordinate object. Mutating that object in place + // is deliberately invisible here: the map is driven by immutable data, and + // callers must build a new object instead. Documented in the README. + test('does not see a coordinate mutated in place', () => { + const shared = { latitude: 1, longitude: 2 }; + const left: MarkerDescriptor = { id: 'm1', coordinate: shared }; + const right: MarkerDescriptor = { id: 'm1', coordinate: shared }; + + shared.latitude = 99; + + expect(markerDescriptorsEqual(left, right)).toBe(true); + }); + + test('sees a coordinate replaced with a new object', () => { + const left: MarkerDescriptor = { + id: 'm1', + coordinate: { latitude: 1, longitude: 2 }, + }; + const right: MarkerDescriptor = { + id: 'm1', + coordinate: { latitude: 99, longitude: 2 }, + }; + + expect(markerDescriptorsEqual(left, right)).toBe(false); + }); +}); + +describe('descriptorListsEqual', () => { + const list = [baseMarker, { ...baseMarker, id: 'marker-2' }]; + + test('short-circuits on the same reference', () => { + expect(descriptorListsEqual(list, list, markerDescriptorsEqual)).toBe(true); + }); + + test('accepts a structurally equal list', () => { + expect( + descriptorListsEqual(list, structuredClone(list), markerDescriptorsEqual), + ).toBe(true); + }); + + test('rejects a shorter list', () => { + expect( + descriptorListsEqual(list, [baseMarker], markerDescriptorsEqual), + ).toBe(false); + }); + + test('rejects a longer list', () => { + expect( + descriptorListsEqual(list, [...list, baseMarker], markerDescriptorsEqual), + ).toBe(false); + }); + + test('rejects a reordered list', () => { + expect( + descriptorListsEqual(list, [...list].reverse(), markerDescriptorsEqual), + ).toBe(false); + }); + + test('rejects a list with one changed descriptor', () => { + const changed = [list[0], { ...list[1], title: 'Different' }]; + + expect(descriptorListsEqual(list, changed, markerDescriptorsEqual)).toBe( + false, + ); + }); + + test('accepts two empty lists', () => { + expect(descriptorListsEqual([], [], markerDescriptorsEqual)).toBe(true); + }); +}); + +describe('the per-overlay list comparators', () => { + // Four near-identical wrappers - these pin that each one reaches its own item + // comparator, which a copy-paste slip would otherwise hide. + test('markerListsEqual compares markers', () => { + expect(markerListsEqual([baseMarker], [structuredClone(baseMarker)])).toBe( + true, + ); + expect( + markerListsEqual([baseMarker], [{ ...baseMarker, opacity: 0.1 }]), + ).toBe(false); + }); + + test('polylineListsEqual compares polylines', () => { + expect( + polylineListsEqual([basePolyline], [structuredClone(basePolyline)]), + ).toBe(true); + expect( + polylineListsEqual([basePolyline], [{ ...basePolyline, strokeWidth: 9 }]), + ).toBe(false); + }); + + test('polygonListsEqual compares polygons', () => { + expect( + polygonListsEqual([basePolygon], [structuredClone(basePolygon)]), + ).toBe(true); + expect( + polygonListsEqual( + [basePolygon], + [{ ...basePolygon, fillColor: '#123456' }], + ), + ).toBe(false); + }); + + test('circleListsEqual compares circles', () => { + expect(circleListsEqual([baseCircle], [structuredClone(baseCircle)])).toBe( + true, + ); + expect(circleListsEqual([baseCircle], [{ ...baseCircle, radius: 1 }])).toBe( + false, + ); + }); +}); + +describe('enteringAnimationsEqual', () => { + test('treats two unset animations as equal', () => { + expect(enteringAnimationsEqual(undefined, undefined)).toBe(true); + }); + + test('treats a set and an unset animation as different', () => { + expect(enteringAnimationsEqual({ kind: 'fade' }, undefined)).toBe(false); + expect(enteringAnimationsEqual(undefined, { kind: 'fade' })).toBe(false); + }); + + test('compares every field', () => { + const base = { + kind: 'fade', + duration: 200, + delay: 25, + reduceMotion: 'never', + } as const; + + expect(enteringAnimationsEqual(base, { ...base })).toBe(true); + expect(enteringAnimationsEqual(base, { ...base, kind: 'fade-scale' })).toBe( + false, + ); + expect(enteringAnimationsEqual(base, { ...base, duration: 300 })).toBe( + false, + ); + expect(enteringAnimationsEqual(base, { ...base, delay: 0 })).toBe(false); + expect( + enteringAnimationsEqual(base, { ...base, reduceMotion: 'system' }), + ).toBe(false); + }); +}); diff --git a/package/src/overlays/__tests__/normalizeMarkerDescriptors.test.ts b/package/src/overlays/__tests__/normalizeMarkerDescriptors.test.ts index 8177763..62bf6bc 100644 --- a/package/src/overlays/__tests__/normalizeMarkerDescriptors.test.ts +++ b/package/src/overlays/__tests__/normalizeMarkerDescriptors.test.ts @@ -2,7 +2,10 @@ import { beforeEach, describe, expect, mock, test } from 'bun:test'; import type { MarkerDescriptor } from '../../types/overlays'; const resolveAssetSourceMock = mock( - (source: number | { uri: string; width?: number; height?: number; scale?: number }) => { + ( + source: + number | { uri: string; width?: number; height?: number; scale?: number }, + ) => { if (typeof source === 'number') { return { uri: `asset:/require-${source}.png`, @@ -20,8 +23,10 @@ mock.module('../assetSourceResolver', () => ({ resolveAssetSource: resolveAssetSourceMock, })); -const { clearResolvedMarkerImageCacheForTests } = await import('../resolveMarkerImage'); -const { normalizeMarkerDescriptors } = await import('../normalizeMarkerDescriptors'); +const { clearResolvedMarkerImageCacheForTests } = + await import('../resolveMarkerImage'); +const { normalizeMarkerDescriptors } = + await import('../normalizeMarkerDescriptors'); const baseDescriptor = { id: 'marker-1', @@ -37,29 +42,17 @@ describe('normalizeMarkerDescriptors', () => { clearResolvedMarkerImageCacheForTests(); }); - test('returns the same array reference when descriptors are unchanged', () => { - const descriptors = [baseDescriptor]; - - expect(normalizeMarkerDescriptors(descriptors)).toBe(descriptors); - }); - - test('returns the same array reference when images are already resolved MarkerImage objects', () => { - const image = { - uri: 'asset:/pin.png', - width: 32, - height: 32, - scale: 2, - }; - const descriptors = [{ ...baseDescriptor, image }]; - - expect(normalizeMarkerDescriptors(descriptors)).toBe(descriptors); + test('carries a descriptor without an image across unchanged', () => { + expect(normalizeMarkerDescriptors([baseDescriptor])).toEqual([ + { ...baseDescriptor, image: undefined }, + ]); }); - test('returns a new array when a require() image is resolved', () => { - const descriptors = [{ ...baseDescriptor, image: 42 as never }]; - const normalized = normalizeMarkerDescriptors(descriptors); + test('resolves a require() image into a MarkerImage', () => { + const normalized = normalizeMarkerDescriptors([ + { ...baseDescriptor, image: 42 as never }, + ]); - expect(normalized).not.toBe(descriptors); expect(normalized[0]?.image).toEqual({ uri: 'asset:/require-42.png', width: 32, @@ -68,15 +61,27 @@ describe('normalizeMarkerDescriptors', () => { }); }); - test('stabilizes after the first require() resolution', () => { + test('carries an already resolved MarkerImage across', () => { + const image = { uri: 'asset:/pin.png', width: 32, height: 32, scale: 2 }; + + expect( + normalizeMarkerDescriptors([{ ...baseDescriptor, image }])[0]?.image, + ).toEqual(image); + }); + + test('resolves each require() source only once', () => { const descriptors = [{ ...baseDescriptor, image: 7 as never }]; - const first = normalizeMarkerDescriptors(descriptors); - // The serialized shape is not a public descriptor (`enteringAnimation` is a - // descriptor object there, a union here), but feeding the output back in is - // exactly what this test asserts is stable, so cast it back. - const second = normalizeMarkerDescriptors(first as MarkerDescriptor[]); + normalizeMarkerDescriptors(descriptors); + normalizeMarkerDescriptors(descriptors); - expect(second).toBe(first); expect(resolveAssetSourceMock).toHaveBeenCalledTimes(1); }); + + test('normalizes a per-marker entering animation', () => { + const normalized = normalizeMarkerDescriptors([ + { ...baseDescriptor, enteringAnimation: false }, + ]); + + expect(normalized[0]?.enteringAnimation).toEqual({ kind: 'none' }); + }); }); diff --git a/package/src/overlays/descriptorEquality.ts b/package/src/overlays/descriptorEquality.ts new file mode 100644 index 0000000..5972e26 --- /dev/null +++ b/package/src/overlays/descriptorEquality.ts @@ -0,0 +1,222 @@ +import type { Coordinate } from '../types/coordinate'; +import type { + CircleDescriptor, + MarkerAnchor, + MarkerDescriptor, + MarkerImage, + MarkerPoint, + OverlayEnteringAnimationDescriptor, + PolygonDescriptor, + PolylineDescriptor, +} from '../native/specs/overlays'; + +/** + * Structural comparisons for the descriptors handed to the native map view. + * + * Nitro diffs view props by reference identity, so a render that rebuilds an + * equivalent array would re-serialize every descriptor across JSI. These + * comparators let the collector hand back the previous array instead. + * + * Every field of a descriptor must be compared here - a field left out is a map + * update that silently never reaches the native side. + */ + +function coordinatesEqual(left: Coordinate, right: Coordinate): boolean { + return ( + left === right || + (left.latitude === right.latitude && left.longitude === right.longitude) + ); +} + +function coordinateListsEqual( + left: Coordinate[], + right: Coordinate[], +): boolean { + if (left === right) { + return true; + } + + if (left.length !== right.length) { + return false; + } + + for (let index = 0; index < left.length; index += 1) { + if (!coordinatesEqual(left[index], right[index])) { + return false; + } + } + + return true; +} + +function pointsEqual( + left: MarkerAnchor | MarkerPoint | undefined, + right: MarkerAnchor | MarkerPoint | undefined, +): boolean { + if (left === right) { + return true; + } + + if (left == null || right == null) { + return false; + } + + return left.x === right.x && left.y === right.y; +} + +function markerImagesEqual( + left: MarkerImage | undefined, + right: MarkerImage | undefined, +): boolean { + if (left === right) { + return true; + } + + if (left == null || right == null) { + return false; + } + + return ( + left.uri === right.uri && + left.width === right.width && + left.height === right.height && + left.scale === right.scale + ); +} + +export function enteringAnimationsEqual( + left: OverlayEnteringAnimationDescriptor | undefined, + right: OverlayEnteringAnimationDescriptor | undefined, +): boolean { + if (left === right) { + return true; + } + + if (left == null || right == null) { + return false; + } + + return ( + left.kind === right.kind && + left.duration === right.duration && + left.delay === right.delay && + left.reduceMotion === right.reduceMotion + ); +} + +export function markerDescriptorsEqual( + left: MarkerDescriptor, + right: MarkerDescriptor, +): boolean { + return ( + left === right || + (left.id === right.id && + coordinatesEqual(left.coordinate, right.coordinate) && + left.title === right.title && + left.subtitle === right.subtitle && + left.draggable === right.draggable && + left.clusterable === right.clusterable && + markerImagesEqual(left.image, right.image) && + pointsEqual(left.anchor, right.anchor) && + pointsEqual(left.centerOffset, right.centerOffset) && + left.rotation === right.rotation && + left.flat === right.flat && + left.opacity === right.opacity && + enteringAnimationsEqual(left.enteringAnimation, right.enteringAnimation)) + ); +} + +export function polylineDescriptorsEqual( + left: PolylineDescriptor, + right: PolylineDescriptor, +): boolean { + return ( + left === right || + (left.id === right.id && + left.strokeColor === right.strokeColor && + left.strokeWidth === right.strokeWidth && + left.tappable === right.tappable && + coordinateListsEqual(left.coordinates, right.coordinates)) + ); +} + +export function polygonDescriptorsEqual( + left: PolygonDescriptor, + right: PolygonDescriptor, +): boolean { + return ( + left === right || + (left.id === right.id && + left.fillColor === right.fillColor && + left.strokeColor === right.strokeColor && + left.strokeWidth === right.strokeWidth && + left.tappable === right.tappable && + coordinateListsEqual(left.coordinates, right.coordinates)) + ); +} + +export function circleDescriptorsEqual( + left: CircleDescriptor, + right: CircleDescriptor, +): boolean { + return ( + left === right || + (left.id === right.id && + left.radius === right.radius && + left.fillColor === right.fillColor && + left.strokeColor === right.strokeColor && + left.strokeWidth === right.strokeWidth && + left.tappable === right.tappable && + coordinatesEqual(left.center, right.center)) + ); +} + +export function descriptorListsEqual( + left: Descriptor[], + right: Descriptor[], + descriptorsEqual: (left: Descriptor, right: Descriptor) => boolean, +): boolean { + if (left === right) { + return true; + } + + if (left.length !== right.length) { + return false; + } + + for (let index = 0; index < left.length; index += 1) { + if (!descriptorsEqual(left[index], right[index])) { + return false; + } + } + + return true; +} + +export function markerListsEqual( + left: MarkerDescriptor[], + right: MarkerDescriptor[], +): boolean { + return descriptorListsEqual(left, right, markerDescriptorsEqual); +} + +export function polylineListsEqual( + left: PolylineDescriptor[], + right: PolylineDescriptor[], +): boolean { + return descriptorListsEqual(left, right, polylineDescriptorsEqual); +} + +export function polygonListsEqual( + left: PolygonDescriptor[], + right: PolygonDescriptor[], +): boolean { + return descriptorListsEqual(left, right, polygonDescriptorsEqual); +} + +export function circleListsEqual( + left: CircleDescriptor[], + right: CircleDescriptor[], +): boolean { + return descriptorListsEqual(left, right, circleDescriptorsEqual); +} diff --git a/package/src/overlays/normalizeMarkerDescriptors.ts b/package/src/overlays/normalizeMarkerDescriptors.ts index ad78867..c0ba6a9 100644 --- a/package/src/overlays/normalizeMarkerDescriptors.ts +++ b/package/src/overlays/normalizeMarkerDescriptors.ts @@ -24,43 +24,15 @@ function normalizeDescriptor(descriptor: PublicMarkerDescriptor): MarkerDescript }; } -function descriptorsEqual( - left: MarkerDescriptor, - right: MarkerDescriptor, -): boolean { - return ( - left.id === right.id && - left.coordinate.latitude === right.coordinate.latitude && - left.coordinate.longitude === right.coordinate.longitude && - left.title === right.title && - left.subtitle === right.subtitle && - left.draggable === right.draggable && - left.clusterable === right.clusterable && - left.image === right.image && - left.markerColor === right.markerColor && - left.anchor === right.anchor && - left.centerOffset === right.centerOffset && - left.rotation === right.rotation && - left.flat === right.flat && - left.opacity === right.opacity && - left.zIndex === right.zIndex && - left.enteringAnimation === right.enteringAnimation - ); -} - +/** + * Widens the public marker descriptors into the shape the native view expects, + * mainly by resolving `require()` image sources. + * + * Reference identity of the result does not matter here: `MapView` stabilizes + * the array structurally before it reaches the native prop. + */ export function normalizeMarkerDescriptors( descriptors: PublicMarkerDescriptor[], ): MarkerDescriptor[] { - let changed = false; - const next = descriptors.map((descriptor) => { - const normalized = normalizeDescriptor(descriptor); - if (descriptorsEqual(normalized, descriptor as MarkerDescriptor)) { - return descriptor as MarkerDescriptor; - } - - changed = true; - return normalized; - }); - - return changed ? next : (descriptors as MarkerDescriptor[]); + return descriptors.map(normalizeDescriptor); } diff --git a/package/src/utils/__tests__/enteringAnimation.test.ts b/package/src/utils/__tests__/enteringAnimation.test.ts new file mode 100644 index 0000000..8bfbaf7 --- /dev/null +++ b/package/src/utils/__tests__/enteringAnimation.test.ts @@ -0,0 +1,46 @@ +import { describe, expect, test } from 'bun:test'; +import { normalizeEnteringAnimation } from '../enteringAnimation'; + +/** + * Reference identity is deliberately not asserted here: `MapView` stabilizes the + * descriptor structurally before it reaches the native prop, so this function + * only has to get the mapping right. + */ +describe('normalizeEnteringAnimation', () => { + test('passes undefined through', () => { + expect(normalizeEnteringAnimation(undefined)).toBeUndefined(); + }); + + test('maps false onto the "none" kind', () => { + expect(normalizeEnteringAnimation(false)).toEqual({ kind: 'none' }); + }); + + test('maps "system" onto the "system" kind', () => { + expect(normalizeEnteringAnimation('system')).toEqual({ kind: 'system' }); + }); + + test('maps each preset onto its descriptor kind', () => { + expect(normalizeEnteringAnimation({ preset: 'fade' })).toMatchObject({ + kind: 'fade', + }); + expect(normalizeEnteringAnimation({ preset: 'fade-scale' })).toMatchObject({ + kind: 'fade-scale', + }); + }); + + test('carries the timing fields across', () => { + expect( + normalizeEnteringAnimation({ + preset: 'fade', + duration: 200, + delay: 25, + reduceMotion: 'never', + }), + ).toEqual({ + kind: 'fade', + duration: 200, + delay: 25, + reduceMotion: 'never', + }); + }); +}); From dff9d3e010b01706b97e2eedc17e270e15e6b07b Mon Sep 17 00:00:00 2001 From: Jakub Kasprzyk Date: Tue, 25 Aug 2026 00:47:07 +0200 Subject: [PATCH 2/4] docs: document MapView re-render behavior Describe what MapView memoizes on the caller's behalf, what is left for the caller to hoist, and the immutability requirement the structural comparison implies. --- README.md | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/README.md b/README.md index 2026089..37dfef3 100644 --- a/README.md +++ b/README.md @@ -35,6 +35,7 @@ Built with [Nitro Modules](https://nitro.margelo.com/) for high-performance nati - [GeoJSON overlays](#geojson-overlays) - [Google Maps setup](#google-maps-setup) - [Marker entering animations](#marker-entering-animations) +- [Re-renders](#re-renders) - [Capability matrix](#capability-matrix) - [Public API](#public-api) - [Example app](#example-app) @@ -511,6 +512,32 @@ Explicit configs use milliseconds. `duration` defaults to `180`, `delay` default On Google Maps providers, marker and cluster entering animations can reduce UI-thread frame rate when a large viewport refresh adds many markers at once. The provider caps animated markers per refresh and may show the remaining markers immediately to preserve map gesture performance. For very large marker sets, prefer clustering, shorter durations, or `markerEnteringAnimation={false}` / `clusterEnteringAnimation={false}` when smooth gestures are more important than entrance motion. +## Re-renders + +Nitro compares view props by reference identity, so a prop rebuilt from unchanged data would still be re-serialized across JSI and re-applied to the native map. `MapView` guards against that on your behalf: + +- Overlay arrays - whether they come from `` children or the bulk `markers` / `polylines` / `polygons` / `circles` props - are compared field by field. Passing a freshly built array with identical content costs one comparison and nothing else. +- `markerEnteringAnimation` and `clusterEnteringAnimation` are compared the same way, so an inline `{ preset: 'fade' }` object is fine. +- Event handlers are wrapped once per handler identity rather than once per render, and the internal `hybridRef` wrapper is created once per mount. + +A re-render of the component holding the `MapView` therefore reaches native with no dirty props at all when nothing actually changed. What is left is yours to control: an arrow function created inline in JSX (`onPress={() => …}`) is a new handler on every render, so wrap it in `useCallback` if the surrounding component re-renders often. + +Descriptors and the objects inside them are treated as immutable. Mutating a coordinate or a descriptor you already handed to `MapView` is **not** picked up, because the comparison sees the same object on both sides - build a new object instead: + +```tsx +// Not picked up - the same coordinate object is mutated in place. +marker.coordinate.latitude = 52.5; + +// Picked up. +setMarkers((current) => + current.map((m) => + m.id === 'm1' + ? { ...m, coordinate: { ...m.coordinate, latitude: 52.5 } } + : m, + ), +); +``` + ## Capability matrix | Capability | `apple` iOS | `google` iOS | `google` Android | From 39866a159fb0bd8b2689851331bb55e39892849f Mon Sep 17 00:00:00 2001 From: Jakub Kasprzyk Date: Tue, 25 Aug 2026 00:54:33 +0200 Subject: [PATCH 3/4] fix: write the stabilizer ref after commit instead of during render React can discard a render, and a ref written during one would leave useStableValue remembering a value the native view never received. Move the write into a layout effect so it only records committed renders. The effect is a no-op on the renders this hook exists for: an unchanged value leaves `stable` - and with it the dependency - untouched, so nothing re-runs. Also drop the hooks barrel additions. Nothing imports that barrel, and MapView reaches the hooks by path like it already did for useCollectedOverlays. --- package/src/hooks/useStableValue.ts | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/package/src/hooks/useStableValue.ts b/package/src/hooks/useStableValue.ts index ea5bdf1..18fa57c 100644 --- a/package/src/hooks/useStableValue.ts +++ b/package/src/hooks/useStableValue.ts @@ -1,4 +1,4 @@ -import { useRef } from 'react'; +import { useLayoutEffect, useRef } from 'react'; /** * Keeps the previously returned value when {@linkcode next} is structurally @@ -10,6 +10,12 @@ import { useRef } from 'react'; * overlay array that means walking it into a `std::vector`, bridging it into a * native array and reconciling it against the map - all far more expensive than * the comparison done here. + * + * The ref is written after commit rather than during render: React may discard + * a render, and a value remembered from one that never committed is a value the + * native view never received. The effect is a no-op on the renders this hook + * exists for, because an unchanged value leaves `stable` - and so the + * dependency - untouched. */ export function useStableValue( next: Value, @@ -18,7 +24,9 @@ export function useStableValue( const previous = useRef(next); const stable = isEqual(previous.current, next) ? previous.current : next; - previous.current = stable; + useLayoutEffect(() => { + previous.current = stable; + }, [stable]); return stable; } From 328a997ae7f5ab303abbef80c9bb1e0c940e7f0b Mon Sep 17 00:00:00 2001 From: Jakub Kasprzyk Date: Fri, 11 Sep 2026 13:39:30 +0200 Subject: [PATCH 4/4] fix: compare markerColor and zIndex when stabilizing markers --- .../__tests__/descriptorEquality.test.ts | 39 +++++++++++-------- package/src/overlays/descriptorEquality.ts | 2 + 2 files changed, 25 insertions(+), 16 deletions(-) diff --git a/package/src/overlays/__tests__/descriptorEquality.test.ts b/package/src/overlays/__tests__/descriptorEquality.test.ts index 938c7c6..158a61e 100644 --- a/package/src/overlays/__tests__/descriptorEquality.test.ts +++ b/package/src/overlays/__tests__/descriptorEquality.test.ts @@ -2,6 +2,7 @@ import { describe, expect, test } from 'bun:test'; import type { CircleDescriptor, MarkerDescriptor, + OverlayEnteringAnimationDescriptor, PolygonDescriptor, PolylineDescriptor, } from '../../native/specs/overlays'; @@ -52,6 +53,13 @@ function describeFieldCoverage( }); } +const baseEnteringAnimation = { + kind: 'fade', + duration: 200, + delay: 50, + reduceMotion: 'system', +} as const satisfies OverlayEnteringAnimationDescriptor; + const baseMarker: MarkerDescriptor = { id: 'marker-1', coordinate: { latitude: 52.2297, longitude: 21.0122 }, @@ -60,17 +68,14 @@ const baseMarker: MarkerDescriptor = { draggable: true, clusterable: true, image: { uri: 'asset:/pin.png', width: 32, height: 48, scale: 2 }, + markerColor: '#FF9500', anchor: { x: 0.5, y: 1 }, centerOffset: { x: 1, y: -2 }, rotation: 45, flat: true, opacity: 0.9, - enteringAnimation: { - kind: 'fade', - duration: 200, - delay: 50, - reduceMotion: 'system', - }, + zIndex: 3, + enteringAnimation: baseEnteringAnimation, }; describeFieldCoverage( @@ -115,6 +120,8 @@ describeFieldCoverage( (d) => ({ ...d, image: { ...d.image, uri: 'asset:/pin.png', scale: 3 } }), ], ['a cleared image', (d) => ({ ...d, image: undefined })], + ['markerColor', (d) => ({ ...d, markerColor: '#007AFF' })], + ['a cleared markerColor', (d) => ({ ...d, markerColor: undefined })], ['anchor.x', (d) => ({ ...d, anchor: { x: 0, y: 1 } })], ['anchor.y', (d) => ({ ...d, anchor: { x: 0.5, y: 0 } })], ['a cleared anchor', (d) => ({ ...d, anchor: undefined })], @@ -124,29 +131,30 @@ describeFieldCoverage( ['rotation', (d) => ({ ...d, rotation: 90 })], ['flat', (d) => ({ ...d, flat: false })], ['opacity', (d) => ({ ...d, opacity: 0.5 })], + ['zIndex', (d) => ({ ...d, zIndex: 9 })], + ['a cleared zIndex', (d) => ({ ...d, zIndex: undefined })], [ 'enteringAnimation.kind', (d) => ({ ...d, - enteringAnimation: { ...d.enteringAnimation, kind: 'fade-scale' }, + enteringAnimation: { + ...baseEnteringAnimation, + kind: 'fade-scale' as const, + }, }), ], [ 'enteringAnimation.duration', (d) => ({ ...d, - enteringAnimation: { - ...d.enteringAnimation, - kind: 'fade', - duration: 400, - }, + enteringAnimation: { ...baseEnteringAnimation, duration: 400 }, }), ], [ 'enteringAnimation.delay', (d) => ({ ...d, - enteringAnimation: { ...d.enteringAnimation, kind: 'fade', delay: 0 }, + enteringAnimation: { ...baseEnteringAnimation, delay: 0 }, }), ], [ @@ -154,9 +162,8 @@ describeFieldCoverage( (d) => ({ ...d, enteringAnimation: { - ...d.enteringAnimation, - kind: 'fade', - reduceMotion: 'never', + ...baseEnteringAnimation, + reduceMotion: 'never' as const, }, }), ], diff --git a/package/src/overlays/descriptorEquality.ts b/package/src/overlays/descriptorEquality.ts index 5972e26..71d7961 100644 --- a/package/src/overlays/descriptorEquality.ts +++ b/package/src/overlays/descriptorEquality.ts @@ -117,11 +117,13 @@ export function markerDescriptorsEqual( left.draggable === right.draggable && left.clusterable === right.clusterable && markerImagesEqual(left.image, right.image) && + left.markerColor === right.markerColor && pointsEqual(left.anchor, right.anchor) && pointsEqual(left.centerOffset, right.centerOffset) && left.rotation === right.rotation && left.flat === right.flat && left.opacity === right.opacity && + left.zIndex === right.zIndex && enteringAnimationsEqual(left.enteringAnimation, right.enteringAnimation)) ); }