fix: stop re-serializing overlay props on unrelated re-renders

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 `<Marker />` 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                           |

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({
     <NativeMapView
       key={`${resolvedProvider}:${googleMapId ?? ''}`}
       style={style}
-      hybridRef={callback((nativeRef) => {
-        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}
     />
   );
 }

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: Handler) {
+  return useMemo(() => callback(handler), [handler]);
+}

package/src/hooks/useStableValue.ts

@@ -0,0 +1,32 @@
+import { useLayoutEffect, 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.
+ *
+ * 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<Value>(
+  next: Value,
+  isEqual: (left: Value, right: Value) => boolean,
+): Value {
+  const previous = useRef(next);
+  const stable = isEqual(previous.current, next) ? previous.current : next;
+
+  useLayoutEffect(() => {
+    previous.current = stable;
+  }, [stable]);
+
+  return stable;
+}