lhapaipai / maplibre-react-components

React bindings for MapLibre GL JS
https://maplibre-react-components.pentatrion.com/
Other
5 stars 1 forks source link

Possible bug: Cleanup function using 'on' instead of 'off' for event listeners #2

Closed andrejilderda closed 2 months ago

andrejilderda commented 2 months ago

Hi @lhapaipai ,

I'm looking at different map options and while scanning through the code I noticed a potential issue in the cleanup function of RLayer.tsx on this line:

      return () => {
        eventNames.forEach((eventName) => {
          // @ts-ignore
          map.on(eventName, id, onLayerEvent);
        });
      };

This appears to be adding event listeners (map.on) in the cleanup function, when it should likely be removing them (map.off). I suggest changing it to:

      return () => {
        eventNames.forEach((eventName) => {
          // @ts-ignore
          map.off(eventName, id, onLayerEvent);
        });
      };

I haven't test things out, so please correct me if I'm wrong. :)

lhapaipai commented 2 months ago

hi @andrejilderda, Thanks for your feedback, of course you are absolutely right, big mistake on my part. Have a good day