Duplicate markers

[HarelM]

• I'm reporting a bug, not asking for help
• I'm sure this is a Leaflet.MarkerCluster code issue, not an issue with my own code nor with the framework I'm using (Cordova, Ionic, Angular, React…)
• I've searched through the issues to make sure it's not yet reported

---

How to reproduce

Not entirely sure, has to do with timing so it's not straight forward.
I think the case that I'm experiencing has the following characteristics:
I'm changing the base layer of my map which adds and removes a tile layer.
While doing do I change the markers inside the cluster.
I have seen a similar stack trace when trying to add a cluster layer when there's no base layer map so the zoom is not defined or something, but I'm not sure...
Image of this issue can be seen here:
IsraelHikingMap/Site#582

I do get the following error message.

ERROR Error: Uncaught (in promise): TypeError: Cannot read property '_zoom' of undefined
TypeError: Cannot read property '_zoom' of undefined
    at NewClass._addLayer (leaflet.markercluster.js:6)
    at NewClass.addLayer (leaflet.markercluster.js:6)
    at categories.layer.ts:135
    at ZoneDelegate.webpackJsonp.../../../../zone.js/dist/zone.js.ZoneDelegate.invoke (zone.js:392)
    at Object.onInvoke (core.js:4629)
    at ZoneDelegate.webpackJsonp.../../../../zone.js/dist/zone.js.ZoneDelegate.invoke (zone.js:391)
    at Zone.webpackJsonp.../../../../zone.js/dist/zone.js.Zone.run (zone.js:142)
    at zone.js:873
    at ZoneDelegate.webpackJsonp.../../../../zone.js/dist/zone.js.ZoneDelegate.invokeTask (zone.js:425)
    at Object.onInvokeTask (core.js:4620)
    at NewClass._addLayer (leaflet.markercluster.js:6)
    at NewClass.addLayer (leaflet.markercluster.js:6)
    at categories.layer.ts:135
    at ZoneDelegate.webpackJsonp.../../../../zone.js/dist/zone.js.ZoneDelegate.invoke (zone.js:392)
    at Object.onInvoke (core.js:4629)
    at ZoneDelegate.webpackJsonp.../../../../zone.js/dist/zone.js.ZoneDelegate.invoke (zone.js:391)
    at Zone.webpackJsonp.../../../../zone.js/dist/zone.js.Zone.run (zone.js:142)
    at zone.js:873
    at ZoneDelegate.webpackJsonp.../../../../zone.js/dist/zone.js.ZoneDelegate.invokeTask (zone.js:425)
    at Object.onInvokeTask (core.js:4620)
    at resolvePromise (zone.js:824)
    at zone.js:876
    at ZoneDelegate.webpackJsonp.../../../../zone.js/dist/zone.js.ZoneDelegate.invokeTask (zone.js:425)
    at Object.onInvokeTask (core.js:4620)
    at ZoneDelegate.webpackJsonp.../../../../zone.js/dist/zone.js.ZoneDelegate.invokeTask (zone.js:424)
    at Zone.webpackJsonp.../../../../zone.js/dist/zone.js.Zone.runTask (zone.js:192)
    at drainMicroTaskQueue (zone.js:602)
    at ZoneTask.webpackJsonp.../../../../zone.js/dist/zone.js.ZoneTask.invokeTask [as invoke] (zone.js:503)
    at invokeTask (zone.js:1540)
    at XMLHttpRequest.globalZoneAwareCallback (zone.js:1566)

What behaviour I'm expecting and which behaviour I'm seeing

Number of element in a cluster should no grow when changing zoom level, but every time I change the zoom level more makers are added to the cluster. this only happens once I enter a "corrupted" state - meaning after an error message like the above is generated.

Minimal example reproducing the issue

Couldn't create on...
I'd be happy to help out but I'm not sure how...

Versions:

leaflet 1.2.0
leaflet.markercluster 1.2.0

[danzel]

Weird. Check the leafletmarkercluster code and see if we read maxZoom / minZoom from map all the time, or if we cache it internally.
If we read it all the time, maybe when you remove/add the baselayer the value changes and that means that the clusters are wrong if they are edited in that time?

[HarelM]

@danzel Can you direct me to the relevant file that might cause this?
Is it MarkerClusterGroup.js?
BTW, my code is here in case you want to take a look:
https://github.com/IsraelHikingMap/Site/blob/master/IsraelHiking.Web/sources/application/services/layers/base-poi-marker.layer.ts
and here is the relevant derived class:
https://github.com/IsraelHikingMap/Site/blob/master/IsraelHiking.Web/sources/application/services/layers/categories.layer.ts

[danzel]

Hey sorry I'm not gonna look through your code. Make a minimal reproduction.

In leaflet.markercluster, look for uses of um... map.getMinZoom / getMaxZoom.
Maybe these are changing when you are changing the base layer, or maybe there is a moment where there is no base layer and we try use them?
Try log those in your app (hack the markercluster code to log them whenever they are called) and see if it logs anything weird.

[HarelM]

I don't think this is the case I'm seeing on my site but the following plunker gets stuck when uncommenting the removeLayer() call:
https://plnkr.co/edit/0hplCMyWXLI8Q9L7qcdn?p=preview

[HarelM]

I have switched away from the minified version to the src version.
The code is failing at the following line inside _addLayer with error:
TypeError: Cannot read property '_zoom' of undefined which means somehow parent is undefined:

for (z = zoom - 1; z > parent._zoom; z--) {
    lastParent = new this._markerCluster(this, z, lastParent);
    gridClusters[z].addObject(lastParent, this._map.project(closest.getLatLng(), z));
 }
 parent._addChild(lastParent);

I have played around with my site to try and reproduce it and it doesn't seem to be related to removing or adding a layer to the map.

Looking at the code the following is interesting:

if (parent) {
    this._removeLayer(closest, false);
}

but later the for loop tried the following:

for (z = zoom - 1; z > parent._zoom; z--) {
    lastParent = new this._markerCluster(this, z, lastParent);
    gridClusters[z].addObject(lastParent, this._map.project(closest.getLatLng(), z));
}

So I'm guessing this method should return if parent is null?
I'm not sure I fully understand what the code is doing in order to try and solve this...
Any help would be appreciated.

[HarelM]

I tried to add the following code around the for-loop in order to avoid this pitfall:

//First create any new intermediate parent clusters that don't exist
if (parent) {
    var lastParent = newCluster;
    for (z = zoom - 1; z > parent._zoom; z--) {
        lastParent = new this._markerCluster(this, z, lastParent);
        gridClusters[z].addObject(lastParent, this._map.project(closest.getLatLng(), z));
    }
    parent._addChild(lastParent);
}
//Remove closest from this zoom level and any above that it is in, replace with newCluster
this._removeFromGridUnclustered(closest, zoom);

and it help a bit but then it failed in a different place:

seems like visibleLayer.__parent is null :-/

I can probably continue adding safe-guards around the usage of _zoom but I'm guessing this is not the right approach...

In any case I'll be happy to test any changes you have in mind since I can't create a minimal reproduction but I can reproduce it easily now...

[danzel]

That error comes up every now and then.
IIRC it happens when you try remove a marker that isn't in the cluster, or add a marker twice or some other weird thing.
I might be wrong, but maybe try searching other issues to see if that comes up, it has come up before and I believe it was an app issue.

[HarelM]

@danzel I can't help if you're not helping me...
The fact is that there's a an error raised in the code which causes an issue on my site
I have looked at the code and tried to suggest what might cause it but I didn't see any valuable response from your side.
This is your repository and your code and you can't expect me to look in issues and understand what people are complaining about - this requires familiarly with the project and the code which I just don't have.
Please take the time to investigate your code and suggest what might cause this issue and how to solve it or what other info will help to better understand the issue.
If there is a problem with how I use this library I expect to receive a meaningful error message and not TypeError: Cannot read property '_zoom' of undefined

[danzel]

I appreciate that you want this issue resolved. I don't have time to look through your application to figure out how it is interacting with L.MCG to cause this issue however. If you can create a stand alone simplified test that reproduces this error then I can take a look.

This issue is almost certainly your app interacting with L.MCG in an incorrect way, something that we don't always give good error messages for (usually due to performance concerns).

If you search the github issues for __parent _zoom you'll find other people reporting issues with this stack trace, the stack trace isn't enough to figure out the error however. It used to be due to moving markers (when we didn't support it), this should work correctly with current versions however.

I may have written this code, but that doesn't mean I need to support it, I'm doing you a favor by replying on here at all, I'm not getting paid to do this.

[HarelM]

I think I managed to reproduce it here:
https://plnkr.co/edit/zDTjlHntbx0hQlUW3piJ?p=preview
I think it's something with the combination of removeLayer and clearLayers

I would appreciate your help even though you are not getting paid (I'm not getting paid for this either... ;-)).

[danzel]

Thanks!
Will take a look, there is some special case handling for removing markers while we aren't on the map, so possible related to that.

[HarelM]

Great! I have tried some things and when using the following code instead of clearLayers() which as far as I can understand is doing the same the issue does not happen, so I tend to think I have a workaround for now...

this.markers.eachLayer((m) => {
    this.markers.removeLayer(m);
});

[danzel]

Added a commit, try build latest markercluster code and try that.
npm run-script prepublish to build.

[ghybs]

Hi @HarelM,

For the sake of testing, here is your Plunker updated with @danzel's latest commit:
https://plnkr.co/edit/d5tiy3jPY99oBlKxRDvH?p=preview

It correctly prevents the error in the provided case.

[HarelM]

@danzel @ghybs Thanks for the quick response and fix!
Let me know when a new version is available.
I would suggest adding a test to this case to make sure this issue does not repeat in the future.

[danzel]

Does this seem to fix it in your app as well?

[HarelM]

The workaround I wrote earlier seems to overcome the issue and the reproductions came from logging I did on my app so if the reproduction is fixed I'm guessing my app will be fixed too but since I'm using angular-cli it hard for me to test code that does not exist in npm. so I prefer a pre-release in npm if possible...

[michaelsoriano]

Hi @HarelM - Can you post your workaround for this issue? Just to be clear, I am seeing just duplicate callouts per group - upon zooming in. The group + coordinates seem okay - it's just the callouts:

Just reference, I'm adding layer + binding popup per group:

markers.addLayer(L.marker([response[i].Lat__c,response[i].Long__c])) .addTo(map) .bindPopup(response[i].Name);

Thanks.

[HarelM]

See above, the problem I had was related to clearLayers method, so I cleared every marker individually.

this.markers.eachLayer((m) => {
    this.markers.removeLayer(m);
});

[danzel]

Fix (to be released shortly) in 1.4.0

[chrislandau]

I'm still seeing this issue. And I know even less about why this would be happening at a code level. But the problem occurred for me when placing a comment and then zooming in/out while the comment bubble was open. For every zoom level (maybe every tile load) I was getting another pin. Still investigating a solution, but if there is an obvious one, I'd love to hear it! Thanks!

[HarelM]

I've moved away from leaflet, sorry... Good luck though :-)