Issue with unmounting/removing routingmachine while routing in progress

[kancur]

Hi there!

I ran into an issue when removing the leaflet-routing-machine from map, but the routing is currently in progress, I receive this error:

Uncaught TypeError: Cannot read properties of null (reading 'removeLayer')
    at NewClass._clearLines (leaflet-routing-machine.js:16186:1)
    at NewClass.<anonymous> (leaflet-routing-machine.js:16163:1)
    at NewClass.<anonymous> (leaflet-routing-machine.js:18004:1)
    at XMLHttpRequest.loaded (leaflet-routing-machine.js:46:1)

Here's an example. Press x key on a keyboard to mount/unmount the RoutingMachine. To slow down the server response, throttle your internet speed in developer console to slow (3g) and quickly mount and unmount the router.

Codesandbox: https://codesandbox.io/s/rlv3-routing-machine-forked-eev2bx?file=/src/Map.js

Am I doing anything wrong, or is there any way to prevent this error?

(While the codesandbox example uses the createControlComponent function, locally I'm adding the RoutingMachine to map in a useEffect like this --> map.addControl(routingControl) and cleaning it up in a return statement like this ---> map.removeControl(routingControl), however, the issue is the same)

Another example:

A routesfound event listener gets called after unmounting the component (after calling map.removeControl(routingControl), or routingControl.remove())

routingControl.on('routesfound', (e) => {
      console.log('routes found')
})

Is that an expected behavior? Thank you!

[curtisy1]

I'm not very familiar with react-leaflet but I'm guessing this is some weird case where the control gets removed during re-render and loses access to the map reference it had before the callback succeeded (possibly because the map gets re-rendered as well?). Funnily enough, this also happens if you spam the x key fast enough to unmount before the request completes.

This is most definitely a bug. I'll see if I can spin up an example project tomorrow to see what the exact issue is.

As for the routesfound event, I guess you could say it's expected? Since the callback happens after an asynchronous XHR request, it has nothing to do with the control per se and just continues executing. I guess checking for a running XHR and cancelling it instead would be possible as well but this might lead to even harder to debug race conditions (such as the request finishing before we can cancel it) so if it's not a problem, I'd prefer to leave the callback behavior as is

[curtisy1]

Okay, so what is happening here is that the underlying L.Control, which L.Routing.Control uses, resets the map reference on removal. Since the request might not necessarily be ready before the map is removed, the callback executes after the map is being reset.

A quick mitigation would be to move the routing logic from the plugin's responsibility to your code by setting autoRoute: false during initialization and then calling routingControl.route(); manually (that's all autoRoute does anyway). You could then add more fault-resistant handling by only removing the control once the routing is done (or after some timeout passed). Something like this comes to mind

  const instance = useRef(null);
  const [routingDone, setRoutingDone] = useState(false);

  useEffect(() => {
    if (props.active) {
      const control = L.Routing.control({
        waypoints: [
          L.latLng(48.14843383355716, 17.067866637833937),
          L.latLng(48.17660790221943, 17.13068946952261)
        ],
        lineOptions: {
          styles: [{ color: "#6FA1EC", weight: 4 }]
        },
        show: false,
        addWaypoints: false,
        routeWhileDragging: true,
        draggableWaypoints: true,
        fitSelectedRoutes: true,
        showAlternatives: false,
        autoRoute: false,
      });

      control.addTo(props.map);

      setRoutingDone(false);
      control.on('routesfound', (e) => {
        setRoutingDone(true);
      });
      control.route();

      instance.current = control;
    } else {
      if (routingDone) {
        props.map.removeControl(instance.current);
        instance.current = null;
      }
    }
  }, [props.active]);

while this might still lead to some rare race conditions, it could probably be tweaked to avoid them entirely.

If having some kind of delay in removing the control is out of the question, you could hide the itinerary and the routes instead of removing the control

[kancur]

Thank you for your response and good mitigation ideas! I understand it better now. 👍

[curtisy1]

You're very welcome! If you need further help, feel free to ask me anytime