enketo-core icon indicating copy to clipboard operation
enketo-core copied to clipboard

Store a reference of the Leaflet map object on the map container

Open jdugh opened this issue 2 years ago • 2 comments

Would it be possible to store the reference of the Leaflet map object on the map container like described on this post : https://github.com/Leaflet/Leaflet/issues/6298#issuecomment-415124583

L.Map.addInitHook(function () {
  this.getContainer()._leaflet_map = this;
});

This trick seems to have a side effect (memory leaks) "only" if we remove the map (and on older browsers). Except mistake, enketo never remove a map ?

With this reference, we could for example easily override the behavior of all the map in the form.

If this approache is accepted, I don't know very well where this code must be placed. It need to be execute before all Leaflet map initializing initialize()

Do you think it's a bad idea or not ?

jdugh avatar Dec 10 '21 17:12 jdugh

Can you say more about your use case? Does it specifically depend on the Leaflet object being attached to a DOM node, or would generally having access to the object be suitable? I'm generally cautious about attaching arbitrary data to the DOM (and would like go generally move away from that in Enketo), but I could imagine exposing, for example, a WeakMap<HTMLElement, LeafletMap> (and perhaps an inverted WeakMap if desired).

eyelidlessness avatar Mar 07 '22 19:03 eyelidlessness

Hi @eyelidlessness , (Sory for the reponse delay). My 'common' use case is to get the this.map leaflet object of a geo* widget to add it some features like :

  • Add a new Layer
  • Add new interraction
  • ...

Like this widget - which adding a geojson layer to the map. (it's not the prettiest widget - but for example)

I have review my code, and finally I think I have to extend directly the GeoPicker widget instead of 'standard' Widget, like I did in this widget - I don't know why I didn't think of it before.

So,

  • Put the leaflet map object in the DOM node is certainly a bad idea - as you explained
  • Put it on a WeakMap can be interesting - for the dev community - but for my use case I think it's not necessary

jdugh avatar Jun 16 '22 03:06 jdugh