GeolocateControl create control button twice when mounted/unmounted/remounted

[lhapaipai]

First, thank you for this amazing Library. I love it and use it more and more

maplibre-gl-js version: 4.3.2 browser: all

Description

due to the asynchronous behavior (permission request for geolocation) of this control we can find ourselves in certain cases with the button appearing twice (and also duplication of listeners)

Steps to Trigger Behavior

import {GeolocateControl, Map} from "maplibre-gl";
import "maplibre-gl/dist/maplibre-gl.css";

const map = new Map({
  container: "root",
  style: "https://demotiles.maplibre.org/style.json"
})

const geolocateControl = new GeolocateControl({});

map.addControl(geolocateControl);
map.removeControl(geolocateControl);
map.addControl(geolocateControl);

Link to Demonstration

JSBin

Expected Behavior

Only one GeolocateControl is created

Actual Behavior

Solution

from the source code

export class GeolocateControl extends Evented implements IControl {

    /** {@inheritDoc IControl.onAdd} */
    onAdd(map: Map) {
        this._map = map;
        this._container = DOM.create('div', 'maplibregl-ctrl maplibregl-ctrl-group');
        checkGeolocationSupport().then((supported) => this._setupUI(supported));
        return this._container;
    }

    onRemove() {

        DOM.remove(this._container);
        this._map.off('zoom', this._onZoom);
        this._map = undefined;
+       this._setup = false;

    }

    _setupUI = (supported: boolean) => {
+       // this method is called asynchronously during onAdd
+       // the method could be called from previous onAdd call
+       if (this._setup) {
+          return;
+       }
        // this method is called asynchronously during onAdd
        // the control could have been removed before reaching here
        if (!this._map) {
            return;
        }

        this._geolocateButton = DOM.create('button', 'maplibregl-ctrl-geolocate', this._container);
        this._setup = true;

        // ...
    };
}

the UI is constructed asynchronously. the _setupUI could be called from previous onAdd call. This kind of issue was found when using MapLibre with React and its StrictMode which goes up goes up the components https://react.dev/reference/react/StrictMode#fixing-bugs-found-by-re-running-effects-in-development

some libraries have made hacks to get around this issue react-map-gl source code

[HarelM]

Thanks for taking the time to open this issue! Looking at the code, I see a few issues.

1. There isn't a need to do the UI setup in a async callback. I believe a better solution is to construct all the UI and have it disabled, after that check for support and update only that part of the UI (title and disable properties).
2. Track user location map start move is never unregistered.
3. The setup variable is not really needed, you can simply check if map is defined.

Feel free to open a PR to resolve these issues.

[lhapaipai]

Thank you @HarelM, Your recommendations seem very fair to me.