Skip to content
Closed
63 changes: 63 additions & 0 deletions debug-popup-follow.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
<!doctype html>
<html>
<head>
<meta charset="utf-8" />
<title>Popup follow marker - Issue #5655</title>
<link rel="stylesheet" href="https://unpkg.com/maplibre-gl@latest/dist/maplibre-gl.css">
<style>
html, body, #map { height: 100%; margin: 0; }
body { display: flex; font-family: Arial, sans-serif; }
#map { flex: 1; }
#log { width: 300px; background: #f0f0f0; padding: 10px; overflow-y: auto; font-size: 12px; }
#log p { margin: 4px 0; color: #333; }
</style>
</head>
<body>
<div id="map"></div>
<div id="log">
<h3>Drag the marker</h3>
<p>Watch if popup stays attached...</p>
<div id="events"></div>
</div>

<script src="https://unpkg.com/maplibre-gl@latest/dist/maplibre-gl.js"></script>
<script>
const map = new maplibregl.Map({
container: 'map',
style: 'https://demotiles.maplibre.org/style.json',
center: [13.404954, 52.520008],
zoom: 12,
pitch: 40
});

const marker = new maplibregl.Marker({draggable: true})
.setLngLat([13.404954, 52.520008])
.addTo(map);

const popup = new maplibregl.Popup({closeButton: false, closeOnClick: false})
.setHTML('<div style="padding: 10px;"><strong>Hello! 👋</strong><br>Keep me attached!</div>')
.setMaxWidth('200px');

marker.setPopup(popup).togglePopup();

const eventsDiv = document.getElementById('events');
function log(msg) {
const p = document.createElement('p');
p.textContent = new Date().toLocaleTimeString() + ' - ' + msg;
eventsDiv.appendChild(p);
eventsDiv.scrollTop = eventsDiv.scrollHeight;
}

marker.on('dragstart', () => log('🔴 dragstart'));
marker.on('drag', () => {
const pos = marker.getLngLat();
log(`📍 drag [${pos.lng.toFixed(4)}, ${pos.lat.toFixed(4)}]`);
});
marker.on('dragend', () => log('🟢 dragend'));

// Extra: try map interactions
map.on('move', () => log('🗺️ map moved'));
map.on('zoomend', () => log('🔍 zoom changed'));
</script>
</body>
</html>
127 changes: 107 additions & 20 deletions src/source/geojson_source.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,18 @@ const wrapDispatcher = (dispatcher) => {
} as Dispatcher;
};

// Helper to extract messages from spy calls
const getMessages = (spy: any) => spy.mock.calls.map((a: any[]) => a[0]);

// Filter to loadData ('LD') messages only
const getLoadData = (spy: any) => getMessages(spy).filter(m => m?.type === MessageType.loadData);

// Filter to messages with actual data (dataDiff or data field), excluding pure options updates
const getDataMessages = (spy: any) => getLoadData(spy).filter(m => m?.data?.dataDiff || m?.data?.data);

// Filter to setData messages only (have .data field)
const getSetDataMessages = (spy: any) => getLoadData(spy).filter(m => m?.data?.data);

const mockDispatcher = wrapDispatcher({
sendAsync() { return Promise.resolve({}); }
});
Expand Down Expand Up @@ -162,15 +174,41 @@ describe('GeoJSONSource.setData', () => {
});

test('only marks source as loaded when there are no pending loads', async () => {
const source = createSource();
const setDataPromise = source.once('data');
source.setData({} as GeoJSON.GeoJSON);
source.setData({} as GeoJSON.GeoJSON);
await setDataPromise;
expect(source.loaded()).toBeFalsy();
const setDataPromise2 = source.once('data');
await setDataPromise2;
expect(source.loaded()).toBeTruthy();
const calls: any[] = [];
let delayLoadData = false;
const pendingResolvers: Array<() => void> = [];

const mockDispatcher = wrapDispatcher({
sendAsync(message: any) {
calls.push(message);
if (message?.type === MessageType.loadData && delayLoadData) {
return new Promise<void>(resolve => pendingResolvers.push(resolve));
}
return Promise.resolve({});
}
});

const source = new GeoJSONSource('id', {
type: 'geojson',
data: {type: 'FeatureCollection', features: []} as GeoJSON.GeoJSON
}, mockDispatcher, undefined);

// initial load completes → loaded() true
await source.load();
expect(source.loaded()).toBe(true);

// ab hier Worker-Updates verzögern
delayLoadData = true;

// Options-Änderung triggert loadData → sollte loaded() direkt auf false setzen
source.setClusterOptions({clusterRadius: 80});
expect(source.loaded()).toBe(false);

// Worker „fertig"
pendingResolvers.splice(0).forEach(r => r());
await new Promise(r => setTimeout(r, 0));

expect(source.loaded()).toBe(true);
});

test('marks source as not loaded before firing "dataloading" event', async () => {
Expand Down Expand Up @@ -566,13 +604,15 @@ describe('GeoJSONSource.updateData', () => {
await waitForEvent(source, 'data', (e: MapSourceDataEvent) => e.sourceDataType === 'metadata');
await waitForEvent(source, 'data', (e: MapSourceDataEvent) => e.sourceDataType === 'metadata');

expect(spy).toHaveBeenCalledTimes(2);
expect(spy.mock.calls[0][0].data.data).toEqual(JSON.stringify(data1));
expect(spy.mock.calls[1][0].data.dataDiff).toEqual({
remove: ['1', '4'],
add: [{id: '2', type: 'Feature', properties: {}, geometry: {type: 'LineString', coordinates: []}}, {id: '5', type: 'Feature', properties: {}, geometry: {type: 'LineString', coordinates: []}}],
update: [{id: '3', addOrUpdateProperties: [], newGeometry: {type: 'Point', coordinates: []}}, {id: '6', addOrUpdateProperties: [], newGeometry: {type: 'LineString', coordinates: []}}]
});
// Filter to messages with actual data (dataDiff or data fields), excluding options-only updates
const dataMessages = getDataMessages(spy);

// First call: setData(data1) - has .data.data field
// Following calls: updateData(...) - have .data.dataDiff field
expect(dataMessages.length).toBeGreaterThanOrEqual(2);
expect(dataMessages[0].data.data).toEqual(JSON.stringify(data1));
expect(dataMessages[1].data.dataDiff).toEqual(update1);
expect(dataMessages[2].data.dataDiff).toEqual(update2);
});

test('is overwritten by a subsequent call to setData when data is loading', async () => {
Expand Down Expand Up @@ -608,9 +648,12 @@ describe('GeoJSONSource.updateData', () => {
await waitForEvent(source, 'data', (e: MapSourceDataEvent) => e.sourceDataType === 'metadata');
await waitForEvent(source, 'data', (e: MapSourceDataEvent) => e.sourceDataType === 'metadata');

expect(spy).toHaveBeenCalledTimes(2);
expect(spy.mock.calls[0][0].data.data).toEqual(JSON.stringify(data1));
expect(spy.mock.calls[1][0].data.data).toEqual(JSON.stringify(data2));
// Filter to setData messages (data field), excluding options-only updates
const dataMessages = getSetDataMessages(spy);

expect(dataMessages.length).toBeGreaterThanOrEqual(2);
expect(dataMessages[0].data.data).toEqual(JSON.stringify(data1));
expect(dataMessages[1].data.data).toEqual(JSON.stringify(data2));
});

test('is queued after setData when data is loading', async () => {
Expand Down Expand Up @@ -831,6 +874,50 @@ describe('GeoJSONSource.load', () => {
source.load();

expect(spy).toHaveBeenCalledTimes(1);
expect(warnSpy).toHaveBeenCalledWith('No data or diff provided to GeoJSONSource id.');
expect(warnSpy).toHaveBeenCalledWith('No pending worker updates for GeoJSONSource id.');
});

test('setClusterOptions triggers worker update and toggles loaded()', async () => {
const calls: any[] = [];
const mockDispatcher = wrapDispatcher({
sendAsync(message: any) {
calls.push(message);
// Simuliere sofort erfolgreiche Antwort
return Promise.resolve({});
}
});

const source = new GeoJSONSource(
'id',
{
type: 'geojson',
data: {type: 'FeatureCollection', features: []} as GeoJSON.GeoJSON,
cluster: true,
clusterRadius: 50
},
mockDispatcher,
undefined
);

// Initiale Ladung
await source.load();
expect(source.loaded()).toBe(true);

// Clear vorherige Nachrichten
calls.length = 0;

// Änderung: Radius -> 80
source.setClusterOptions({clusterRadius: 80});

// Sollte genau ein Worker-Update ausgelöst haben
expect(calls.length).toBe(1);
expect(calls[0].type).toBe(MessageType.loadData);
// loaded() sollte während des Updates false sein (direkt nach Aufruf)
expect(source.loaded()).toBe(false);

// simulated "worker done" (sendAsync hat bereits resolved)
// kurzer Tick, damit interne Flags umspringen können
await new Promise(r => setTimeout(r, 0));
expect(source.loaded()).toBe(true);
});
});
38 changes: 23 additions & 15 deletions src/source/geojson_source.ts
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ export class GeoJSONSource extends Evented implements Source {
map: Map;
actor: Actor;
_isUpdatingWorker: boolean;
_pendingWorkerUpdate: { data?: GeoJSON.GeoJSON | string; diff?: GeoJSONSourceDiff };
_pendingWorkerUpdate: { data?: GeoJSON.GeoJSON | string; diff?: GeoJSONSourceDiff; optionsChanged?: boolean };
_collectResourceTiming: boolean;
_removed: boolean;

Expand Down Expand Up @@ -203,6 +203,10 @@ export class GeoJSONSource extends Evented implements Source {
return pixelValue * (EXTENT / this.tileSize);
}

private _hasPendingWorkerUpdate(): boolean {
return this._pendingWorkerUpdate.data !== undefined || this._pendingWorkerUpdate.diff !== undefined || this._pendingWorkerUpdate.optionsChanged;
}

private _getClusterMaxZoom(clusterMaxZoom: number): number {
const effectiveClusterMaxZoom = clusterMaxZoom ? Math.round(clusterMaxZoom) : this.maxzoom - 1;
if (!(Number.isInteger(clusterMaxZoom) || clusterMaxZoom === undefined)) {
Expand Down Expand Up @@ -308,13 +312,16 @@ export class GeoJSONSource extends Evented implements Source {
* ```
*/
setClusterOptions(options: SetClusterOptions): this {
this.workerOptions.cluster = options.cluster;
if (options) {
if (options.clusterRadius !== undefined) this.workerOptions.superclusterOptions.radius = this._pixelsToTileUnits(options.clusterRadius);
if (options.clusterMaxZoom !== undefined) {
this.workerOptions.superclusterOptions.maxZoom = this._getClusterMaxZoom(options.clusterMaxZoom);
}
if (options.cluster !== undefined) {
this.workerOptions.cluster = options.cluster;
}
if (options.clusterRadius !== undefined) {
this.workerOptions.superclusterOptions.radius = this._pixelsToTileUnits(options.clusterRadius);
}
if (options.clusterMaxZoom !== undefined) {
this.workerOptions.superclusterOptions.maxZoom = this._getClusterMaxZoom(options.clusterMaxZoom);
}
this._pendingWorkerUpdate.optionsChanged = true;
this._updateWorkerData();
return this;
}
Expand Down Expand Up @@ -380,15 +387,13 @@ export class GeoJSONSource extends Evented implements Source {
* using geojson-vt or supercluster as appropriate.
*/
async _updateWorkerData(): Promise<void> {
if (this._isUpdatingWorker) return;

const {data, diff} = this._pendingWorkerUpdate;

if (!data && !diff) {
warnOnce(`No data or diff provided to GeoJSONSource ${this.id}.`);
if (!this._hasPendingWorkerUpdate()) {
warnOnce(`No pending worker updates for GeoJSONSource ${this.id}.`);
return;
}

const {data, diff} = this._pendingWorkerUpdate;

const options: LoadGeoJSONParameters = extend({type: this.type}, this.workerOptions);
if (data) {
if (typeof data === 'string') {
Expand All @@ -405,6 +410,8 @@ export class GeoJSONSource extends Evented implements Source {
}

this._isUpdatingWorker = true;
// Reset the flag since this update is using the latest options
this._pendingWorkerUpdate.optionsChanged = undefined;
this.fire(new Event('dataloading', {dataType: 'source'}));
try {
const result = await this.actor.sendAsync({type: MessageType.loadData, data: options});
Expand Down Expand Up @@ -439,14 +446,14 @@ export class GeoJSONSource extends Evented implements Source {
this.fire(new ErrorEvent(err));
} finally {
// If there is more pending data, update worker again.
if (this._pendingWorkerUpdate.data || this._pendingWorkerUpdate.diff) {
if (this._hasPendingWorkerUpdate()) {
this._updateWorkerData();
}
}
}

loaded(): boolean {
return !this._isUpdatingWorker && this._pendingWorkerUpdate.data === undefined && this._pendingWorkerUpdate.diff === undefined;
return !this._isUpdatingWorker && !this._hasPendingWorkerUpdate();
}

async loadTile(tile: Tile): Promise<void> {
Expand Down Expand Up @@ -505,3 +512,4 @@ export class GeoJSONSource extends Evented implements Source {
return false;
}
}

Loading