GuillaumeAmat/leaflet-overpass-layer

ZoomIndicator is missing onRemove

mbrucher opened this issue · 3 comments

I'm trying to wrap with (great) layer with react like shown on SO: https://stackoverflow.com/questions/56199253/upgrade-an-overpass-layer-code-to-use-react-leaflet-v2
When doing so and adding the layer witht he zoom control, I get an exception during unmount (for instance if I change page, close the leaflet object...) saying that it's trying to call onRemove from leaflet call to remove:

a.Control.prototype.onRemove is undefined

The stack looks like:

onRemove MinZoomIndicator.js:67
remove leaflet-src.js:4512
fire leaflet-src.js:591
remove leaflet-src.js:3574
React 16
unstable_runWithPriority scheduler.development.js:255
React 7
dispatchInteractiveEvent 

Disabling the indicator removes the error, but not sure why Control would require an inherited class to implement this?

Hi @mbrucher, great to hear! :)

I don't have much time to dig into it but I think you could take a look here:
https://github.com/GuillaumeAmat/leaflet-overpass-layer/blob/develop/src/MinZoomIndicator.js#L84

For whatever reason, L.Control seems to have a different API than expected. Perhaps onRemove is a deprecated/removed API?

If you call the right method here, it should be good or at least make a progress in the resolution.

In recent versions of Leaflet, L.Control has a remove method, which calls onRemove while cleaning up. See Control.js
The entire purpose on onRemove() is to let extended Controls like this one clean up after itself, so there is no need to call any method on the parent class.
I've removed the method call in #35 and cleanup works now as intended.

Thanks @fodor0205 👌