geoman-io / leaflet-geoman

🍂🗺️ The most powerful leaflet plugin for drawing and editing geometry layers
https://geoman.io
MIT License
2.19k stars 431 forks source link

Prevent Null Pointer When Layer Has Been Removed #1479

Closed sheeley820 closed 5 months ago

sheeley820 commented 5 months ago

I am using a Leaflet plugin called Leaflet Arrowheads which allows me to add arrows to the polylines on my map. However, when attempting to rotate a polyline after arrows have been added to it I find that occasionally there is no map reference on the arrowhead, which throws an error. This JSFiddle demonstrates the issue. (The error is visible in the browser console, not the Fiddle console). I have found that adding the following to the enableRotate function above any references to this._layer._map fixes this issue, and allows the arrow to rotate with the line without throwing any errors.

if(!this._layer._map) return;

The underlying issue appears to be a race condition based on event execution. The arrowhead library fires a "remove" event, then sets the _map reference to null, however, there are times when the layer is not fully removed from the map when Geoman processes it, which results in the error. Since there are multiple operations which fail if _map is null I think it is reasonable to add a guard clause preventing that code execution. And there are no side affects which I have seen as the actual removal event fires almost immediately after the error would normally be seen.

I am willing to create a PR for this.

Falke-Design commented 5 months ago

I don't think that this would be a good approch. The question is more why is the Geoman function called, when the layer isn't on the map?

I suggest you to add pmIgnore: true to the arrow creation, then they will be not editable and the error is not thrown:

let hat = options.fill
    ? L.polygon(hatPoints, { ...hatOptions, ...localHatOptions, pmIgnore: true})
    : L.polyline(hatPoints, { ...hatOptions, ...localHatOptions, pmIgnore: true });

https://jsfiddle.net/dw50jygq/

sheeley820 commented 5 months ago

I was unaware of this property, and it appears to have accomplished what I wanted. Thanks.