Stop resync from rewriting sources and layers that did not change

src/circle/LongdoCircleLayer.ts

@@ -3,7 +3,7 @@ import type {
   LayerSpecification,
 } from 'maplibre-gl';
 import { type CircleEntity } from '@mapconductor/js-sdk-core';
-import { bringMarkerLayersToFront, type FeatureCollection, type PolygonFeature } from '../helpers';
+import { bringMarkerLayersToFront, setSourceFeatures, type FeatureCollection, type PolygonFeature } from '../helpers';
 import { LongdoMapViewHolder } from '../LongdoMapViewHolder';
 
 export type LongdoActualCircle = PolygonFeature & { id?: string | number };
@@ -51,7 +51,7 @@ export class LongdoCircleLayer {
     try {
       const source = this.holder.map.getSource(this.sourceId) as GeoJSONSource | undefined;
       if (!source) return false;
-      source.setData(data);
+      setSourceFeatures(source, data.features);
       return true;
     } catch {
       return false;

src/helpers.ts

@@ -42,14 +42,84 @@ export function removeSourceIfExists(map: maplibregl.Map, sourceId: string): voi
   if (map.getSource(sourceId)) map.removeSource(sourceId);
 }
 
+const MARKER_LAYER_IDS = ['mc-raster-lyr-mc-marker-tiles', 'mc-marker-layer', 'mc-marker-drag-layer'];
+
+// The layer order this module last produced, per map. Lets bringMarkerLayersToFront tell
+// "nothing moved since I arranged it" apart from "someone added a layer on top".
+const arrangedOrders = new WeakMap<object, string>();
+
+function currentLayerOrder(map: maplibregl.Map): string[] | null {
+  try {
+    const m = map as unknown as {
+      getLayersOrder?: () => string[];
+      style?: { order?: string[] };
+      getStyle?: () => { layers?: Array<{ id: string }> } | undefined;
+    };
+    if (typeof m.getLayersOrder === 'function') return m.getLayersOrder();
+    if (Array.isArray(m.style?.order)) return [...m.style!.order!];
+    const layers = m.getStyle?.()?.layers;
+    return layers ? layers.map((layer) => layer.id) : null;
+  } catch {
+    return null;
+  }
+}
+
+/**
+ * Keep the marker layers above every other layer.
+ *
+ * **Moves nothing when they already are.** `moveLayer` marks the style as changed even when
+ * the layer stays where it was, and every change fires `styledata`. The provider treats a
+ * `styledata` that arrives while sources are still loading as "style not ready", and the
+ * `idle` that follows resyncs every overlay — which called this again. The style flipped
+ * between ready and not ready about every 0.3 s, forever, with nothing on the map changing.
+ */
 export function bringMarkerLayersToFront(map: maplibregl.Map): void {
-  for (const layerId of ['mc-raster-lyr-mc-marker-tiles', 'mc-marker-layer', 'mc-marker-drag-layer']) {
+  const order = currentLayerOrder(map);
+  if (order) {
+    const present = MARKER_LAYER_IDS.filter((layerId) => order.includes(layerId));
+    if (present.length === 0) return;
+    const tail = order.slice(order.length - present.length);
+    if (tail.every((layerId, i) => layerId === present[i])) return;
+    if (arrangedOrders.get(map) === order.join('\n')) return;
+  }
+  for (const layerId of MARKER_LAYER_IDS) {
     try {
       if (map.getLayer(layerId)) map.moveLayer(layerId);
     } catch {
       continue;
     }
   }
+  const arranged = currentLayerOrder(map);
+  if (arranged) arrangedOrders.set(map, arranged.join('\n'));
+}
+
+// What each GeoJSON source was last given, keyed by the source object itself. A style swap
+// replaces the source objects, so a lost source never matches and is written again.
+const writtenFeatures = new WeakMap<object, readonly unknown[]>();
+
+/**
+ * `source.setData` that skips the write when the features are the same objects, in the same
+ * order, as the last write to this very source.
+ *
+ * Renderers build a new feature object whenever an overlay changes, so identity is enough to
+ * say "nothing changed". Skipping matters for the same reason as in
+ * [bringMarkerLayersToFront]: a redundant setData puts the source back into "loading", the
+ * next `styledata` then reads as "style not ready", and the resync on `idle` wrote it again.
+ */
+export function setSourceFeatures(
+  source: { setData(data: GeoJSONSourceData): unknown },
+  features: FeatureCollection['features'],
+): void {
+  const previous = writtenFeatures.get(source);
+  if (
+    previous &&
+    previous.length === features.length &&
+    previous.every((feature, i) => feature === features[i])
+  ) {
+    return;
+  }
+  source.setData({ type: 'FeatureCollection', features } as GeoJSONSourceData);
+  writtenFeatures.set(source, [...features]);
 }
 
 export function metersToPixels(meters: number, latitude: number): number {

src/marker/MarkerLayer.ts

@@ -4,7 +4,7 @@ import type {
   LayerSpecification,
 } from 'maplibre-gl';
 import type { MarkerEntity } from '@mapconductor/js-sdk-core';
-import { bringMarkerLayersToFront, type FeatureCollection, type PointFeature } from '../helpers';
+import { bringMarkerLayersToFront, setSourceFeatures, type FeatureCollection, type PointFeature } from '../helpers';
 import { LongdoMapViewHolder } from '../LongdoMapViewHolder';
 
 export type LongdoActualMarker = PointFeature;
@@ -93,7 +93,7 @@ export class MarkerLayer {
     try {
       const source = this.holder.map.getSource(this.sourceId) as GeoJSONSource | undefined;
       if (!source) return false;
-      source.setData(data);
+      setSourceFeatures(source, data.features);
       return true;
     } catch {
       return false;

src/polygon/LongdoPolygonLayer.ts

@@ -5,7 +5,7 @@ import type {
   LineFeature,
   PolygonFeature,
 } from '../helpers';
-import { bringMarkerLayersToFront } from '../helpers';
+import { bringMarkerLayersToFront, setSourceFeatures } from '../helpers';
 import { LongdoMapViewHolder } from '../LongdoMapViewHolder';
 
 export interface LongdoActualPolygon {
@@ -69,8 +69,8 @@ export class LongdoPolygonLayer {
       const outlineSource = this.holder.map.getSource(this.outlineSourceId) as GeoJSONSource | undefined;
       if (!fillSource || !outlineSource) return false;
 
-      fillSource.setData(fillData);
-      outlineSource.setData(outlineData);
+      setSourceFeatures(fillSource, fillData.features);
+      setSourceFeatures(outlineSource, outlineData.features);
       return true;
     } catch {
       return false;

src/polyline/LongdoPolylineLayer.ts

@@ -1,6 +1,6 @@
 import type { GeoJSONSource, LayerSpecification } from 'maplibre-gl';
 import type { PolylineEntity } from '@mapconductor/js-sdk-core';
-import { bringMarkerLayersToFront, type FeatureCollection, type LineFeature } from '../helpers';
+import { bringMarkerLayersToFront, setSourceFeatures, type FeatureCollection, type LineFeature } from '../helpers';
 import { LongdoMapViewHolder } from '../LongdoMapViewHolder';
 
 export type LongdoActualPolyline = LineFeature[];
@@ -45,7 +45,7 @@ export class LongdoPolylineLayer {
     try {
       const source = this.holder.map.getSource(this.sourceId) as GeoJSONSource | undefined;
       if (!source) return false;
-      source.setData(data);
+      setSourceFeatures(source, data.features);
       return true;
     } catch {
       return false;

test/ResyncWritesNothing.test.mjs

@@ -0,0 +1,129 @@
+import assert from 'node:assert/strict';
+import { mkdirSync, mkdtempSync, rmSync } from 'node:fs';
+import { after, test } from 'node:test';
+import { pathToFileURL } from 'node:url';
+import { resolve } from 'node:path';
+import { build } from 'tsup';
+import { createGeoPoint, createPolylineState, PolylineManager } from '@mapconductor/js-sdk-core';
+
+// The provider marks the style "not ready" on a styledata that arrives while a source is
+// still loading, and resyncs every overlay on the idle that follows. A resync that rewrote
+// unchanged sources (setData) or re-moved layers that were already on top (moveLayer) fired
+// another styledata, so the style flipped between ready and not ready every ~0.3 s forever.
+// These tests pin "a resync with nothing to do writes nothing", and that a lost style is
+// still rebuilt.
+mkdirSync(new URL('../build/', import.meta.url), { recursive: true });
+const outDir = mkdtempSync(new URL('../build/resync-tests-', import.meta.url));
+after(() => rmSync(outDir, { recursive: true, force: true }));
+await build({
+    entry: {
+        helpers: 'src/helpers.ts',
+        controller: 'src/polyline/LongdoPolylineController.ts',
+        renderer: 'src/polyline/LongdoPolylineOverlayRenderer.ts',
+        layer: 'src/polyline/LongdoPolylineLayer.ts',
+    },
+    outDir, format: ['esm'], platform: 'node', splitting: false,
+    dts: false, sourcemap: false, config: false, silent: true,
+});
+const { bringMarkerLayersToFront, setSourceFeatures } = await import(pathToFileURL(resolve(outDir, 'helpers.mjs')));
+const { LongdoPolylineController } = await import(pathToFileURL(resolve(outDir, 'controller.mjs')));
+const { LongdoPolylineOverlayRenderer } = await import(pathToFileURL(resolve(outDir, 'renderer.mjs')));
+const { LongdoPolylineLayer } = await import(pathToFileURL(resolve(outDir, 'layer.mjs')));
+
+function fakeMap(initialLayers = ['background', 'road-label']) {
+    const writes = { setData: 0, moveLayer: 0, addSource: 0, addLayer: 0 };
+    let sources = new Map();
+    let order = [...initialLayers];
+    const layers = new Map(initialLayers.map((id) => [id, { id }]));
+    const map = {
+        writes,
+        get order() { return [...order]; },
+        getLayersOrder: () => [...order],
+        style: { get order() { return [...order]; } },
+        getStyle: () => ({ layers: order.map((id) => layers.get(id)) }),
+        getSource: (id) => sources.get(id),
+        getLayer: (id) => layers.get(id),
+        addSource(id) {
+            writes.addSource++;
+            sources.set(id, { setData() { writes.setData++; } });
+        },
+        addLayer(layer, before) {
+            writes.addLayer++;
+            layers.set(layer.id, layer);
+            const at = before ? order.indexOf(before) : -1;
+            if (at >= 0) order.splice(at, 0, layer.id); else order.push(layer.id);
+        },
+        moveLayer(id, before) {
+            writes.moveLayer++;
+            order = order.filter((x) => x !== id);
+            const at = before ? order.indexOf(before) : -1;
+            if (at >= 0) order.splice(at, 0, id); else order.push(id);
+        },
+        // A style swap drops every source and layer the SDK added.
+        reloadStyle() {
+            sources = new Map();
+            order = order.filter((id) => !id.startsWith('mc-') && !id.startsWith('polyline'));
+            for (const id of [...layers.keys()]) if (!order.includes(id)) layers.delete(id);
+        },
+    };
+    return map;
+}
+
+test('bringMarkerLayersToFront moves nothing when the marker layers are already on top', () => {
+    const map = fakeMap(['background', 'mc-marker-layer', 'mc-marker-drag-layer']);
+    bringMarkerLayersToFront(map);
+    assert.equal(map.writes.moveLayer, 0);
+});
+
+test('bringMarkerLayersToFront moves the marker layers once, then leaves them alone', () => {
+    const map = fakeMap(['background', 'mc-marker-layer', 'mc-marker-drag-layer']);
+    map.addLayer({ id: 'polyline-layer' });
+    bringMarkerLayersToFront(map);
+    assert.deepEqual(map.order.slice(-2), ['mc-marker-layer', 'mc-marker-drag-layer']);
+    const moved = map.writes.moveLayer;
+    assert.ok(moved > 0);
+    bringMarkerLayersToFront(map);
+    bringMarkerLayersToFront(map);
+    assert.equal(map.writes.moveLayer, moved);
+});
+
+test('setSourceFeatures skips identical features but writes to a replaced source', () => {
+    let calls = 0;
+    const source = { setData() { calls++; } };
+    const a = { type: 'Feature' };
+    const b = { type: 'Feature' };
+    setSourceFeatures(source, [a, b]);
+    setSourceFeatures(source, [a, b]);
+    assert.equal(calls, 1);
+    setSourceFeatures(source, [b, a]);
+    assert.equal(calls, 2, 'a different order is a change');
+    setSourceFeatures(source, [b]);
+    assert.equal(calls, 3, 'a removed feature is a change');
+    const replaced = { setData() { calls++; } };
+    setSourceFeatures(replaced, [b]);
+    assert.equal(calls, 4, 'a new source object (style swap) is always written');
+});
+
+test('resync with nothing changed writes nothing; after a style swap it rebuilds', async () => {
+    const map = fakeMap(['background', 'road-label', 'mc-marker-layer', 'mc-marker-drag-layer']);
+    const holder = { map };
+    const layer = new LongdoPolylineLayer({ holder, canEditStyle: () => true });
+    const renderer = new LongdoPolylineOverlayRenderer({ layer, polylineManager: new PolylineManager(), holder });
+    const controller = new LongdoPolylineController(renderer);
+    await controller.add([createPolylineState({
+        id: 'PL',
+        points: [createGeoPoint({ latitude: 35, longitude: 139 }), createGeoPoint({ latitude: 36, longitude: 140 })],
+    })]);
+    const afterAdd = { ...map.writes };
+    assert.ok(afterAdd.setData > 0);
+
+    await controller.resync();
+    await controller.resync();
+    assert.deepEqual(map.writes, afterAdd, 'a resync with nothing to do must not touch the style');
+
+    map.reloadStyle();
+    await controller.resync();
+    assert.ok(map.getSource('polyline-source'), 'the lost source is added again');
+    assert.ok(map.writes.setData > afterAdd.setData, 'and its data written again');
+    assert.ok(map.order.includes('polyline-layer'), 'and its layer');
+});