fix(markers): fix dom markers memory leaks#273
Conversation
| _create(newMarkers, oldMarkers); | ||
| }); | ||
| scope.$on('$destroy', function () { | ||
| _destroy(leafletScope.markers, {}, leafletMarkers, map, layers); |
There was a problem hiding this comment.
I'm wondering if we should call deleteMarker on each leaflet marker when the directive is destroyed. Thus, we are sure that everything is reallyclean up
There was a problem hiding this comment.
Normally this would make sense. But the markers are on the same scope as the map. So deleting the markers should already happen since the map is being removed.
Line 137 in c30e5d1
There was a problem hiding this comment.
Yeap, but if there is some cleaning to be done before removing the marker, then we might need to call the deleteMarker explicitly...
There was a problem hiding this comment.
Thanks for merging it. I still have a question: in this line, we only delete the markers that are not anymore in the scope (if I understand correctly what _seeWhatWeAlreadyHave does). But if we destroy the directives, shouldn't we delete all the leaflet markers?
|
LGTM |
2 participants
I've added the dom markers feature a few months ago, and by playing with it again, I found out that there were memory leaks due to this feature. This PR solves them.
Demo of the feature: http://codepen.io/ValentinH/full/XKkEEd/