publiclab / Leaflet.DistortableImage

A Leaflet extension to distort or "rubber sheet" images
https://publiclab.github.io/Leaflet.DistortableImage/examples/
BSD 2-Clause "Simplified" License
265 stars 284 forks source link

Use in React 18: Destroying a component with DistortableImage crashes app #1391

Open PeteMac3D opened 9 months ago

PeteMac3D commented 9 months ago

Scope: React v.18.2.0, vite v.4.4.5, React-Leaflet: v4.2.1, Leaflet.DistortableImage: v0.21.9

Description: Not sure if this is a bug, my usage, or simply DistortableImage isn't set up for use in React? Destroying a component results in a crash of the app, specifically if I included the action L.RotateAction or L.LockAction (pass in as an array or used as default) When I have these eliminated everything works as expected.

Think the problem lays with the unbinding of the event handlers and the loss of context relating to 'this' , initialized by React during diffing stage ?

See console error(s) image

offending line of code in editHandle.js

 _unbindListeners: function() {
    this.off({
      contextmenu: L.DomEvent.stop,
      dragstart: this._onHandleDragStart,
      drag: this._onHandleDrag,
      dragend: this._onHandleDragEnd,
    }, this);

    this._handled._map.off('zoomend', this.updateHandle, this);
    this._handled.off('update', this.updateHandle, this);
  }

useEffect in compoment

useEffect(() => {
    map.whenReady(function () {
      img = L.distortableImageOverlay('/100mlayout.png', {
        actions: [
          L.RotateAction,
          L.OpacityAction,
          L.BorderAction,
          L.ExportAction,
          // L.LockAction,
          L.RestoreAction,
          L.StackAction,
          L.DeleteAction,
        ]

        corners: calulateBounds(map.getCenter()),
      }).addTo(map);
    });

    // return () => map.remove();
  }, []);

Parent component

 return (
    <MapContainer
      className="project__map"
      center={center}
      zoom={zoom}
      // ref={map}
      whenReady={() => {
        setIsReady(true);
      }}
    >
      <ReactLeafletGoogleLayer
        apiKey="******************************************"
        type={'satellite'}
      />
      {isReady && <MapOverlay setIsReady={setIsReady} />}
      <MapSearch
        apiKey="******************************************"
        setIsReady={setIsReady}
      />

    </MapContainer>
  );

As a side note I tried to remove the action in useEffect (using the useeffect clean up function) but cannot locate the method 'removeTool' I dont see it on any of the instances prototypes ?

welcome[bot] commented 9 months ago

Thanks for opening your first issue here! This space is protected by our Code of Conduct - and we're here to help. Please follow the issue template to help us help you 👍🎉😄 If you have screenshots or a gif to share demonstrating the issue, that's really helpful! 📸 You can make a gif too! Do join our Gitter channel for some brainstorming discussions.

PeteMac3D commented 9 months ago

visual of the problem for context distortableImage_issue_91

PeteMac3D commented 9 months ago

Fixed the problem for anyone who find a problem implementing DistortableImage into React /React leaflet - think it relates to issue #1381 in a way with

map.remove()

crashing the app at the start of my error tree MapContainer had a clean up function in a useEffect

  useEffect(()=>{
        return ()=>{
context?.map.remove()};}, [context ]);

context being from @react-leaflet/core

I used #1381 solution getting reference to the map via context and it works as expected

   context?.map.eachLayer(layer => {
                // @ts-ignore
                if (layer.editing) {
                  // @ts-ignore
                  const layers = layer.editing.currentHandle?._layers ?? {};
                  // @ts-ignore
                  Object.values(layers).forEach(layer => layer.remove());
                  // @ts-ignore
                  layer.editing.currentHandle = null;
                }
                layer.remove();
              });
              context?.map.remove();
jpoep commented 9 months ago

I believe it's a similar issue to one we experienced when unmounting the whole map: #1381

The whole remove operation errors because removing each individual layer fails if it's a DistortableImage layer with certain handles like you described. I'm not sure what causes it but we did find a hacky workaround.

I suggest you keep track of L.DistortableImageLayer in your hook and unmount it like this:

/* eslint-disable @typescript-eslint/ban-ts-comment */
// @ ts-ignore
 if (layer.editing) {
    // @ts-ignore
    const layers = layer.editing.currentHandle?._layers ?? {};
    // @ts-ignore
    Object.values(layers).forEach(layer => layer.remove());
    // @ts-ignore
    layer.editing.currentHandle = null;
  }
  layer.remove();
/* eslint-enable @typescript-eslint/ban-ts-comment */

I'm not entirely sure this will work for your in your case but we also had problems when removing individual layers like I assume you are trying to do. When attempting to remove the entire map after this, our app crashed just like yours does.

The error we were getting was slightly different but also occurred in _unbindListeners of editHandle.js, so maybe give it a shot!

Edit: beat me to it 😄

PeteMac3D commented 9 months ago

@jpoep a simultaneous response :D Thanks for you post -massive help

Preefix commented 8 months ago

Any updates on this issue?

yearsalary commented 5 months ago

I believe it's a similar issue to one we experienced when unmounting the whole map: #1381

The whole remove operation errors because removing each individual layer fails if it's a DistortableImage layer with certain handles like you described. I'm not sure what causes it but we did find a hacky workaround.

I suggest you keep track of L.DistortableImageLayer in your hook and unmount it like this:

/* eslint-disable @typescript-eslint/ban-ts-comment */
// @ ts-ignore
 if (layer.editing) {
    // @ts-ignore
    const layers = layer.editing.currentHandle?._layers ?? {};
    // @ts-ignore
    Object.values(layers).forEach(layer => layer.remove());
    // @ts-ignore
    layer.editing.currentHandle = null;
  }
  layer.remove();
/* eslint-enable @typescript-eslint/ban-ts-comment */

I'm not entirely sure this will work for your in your case but we also had problems when removing individual layers like I assume you are trying to do. When attempting to remove the entire map after this, our app crashed just like yours does.

The error we were getting was slightly different but also occurred in _unbindListeners of editHandle.js, so maybe give it a shot!

Edit: beat me to it 😄

thank you for this solution. finally, i solved this issue but i have confused when call this logic in hooks, for like me, i remain my code ~ i`m using unload event hooks

import {useMapEvent} from 'react-leaflet';
import L from 'leaflet';
import 'leaflet-toolbar';
import 'leaflet-distortableimage';
import 'leaflet-toolbar/dist/leaflet.toolbar.css';
import 'leaflet-distortableimage/dist/leaflet.distortableimage.css';

import {forwardRef, useEffect, useImperativeHandle, useState} from 'react';

import {Path} from '../types';

interface IDistortableImageOverlayProps {
  editable: boolean;
  mapCenter: [number, number];
  imageUrl: string;
  corners?: Path[];
}

export default forwardRef(
  (
    {editable, mapCenter, imageUrl, corners}: IDistortableImageOverlayProps,
    ref,
  ) => {
    const map = useMapEvent('unload', () => {
      map.eachLayer((layer: any) => {
        // @ts-ignore
        if (layer.editing) {
          // @ts-ignore
          const layers = layer.editing.currentHandle?._layers ?? {};
          // @ts-ignore
          Object.values(layers).forEach((layer) => layer.remove());
          // @ts-ignore
          layer.editing.currentHandle = null;
        }
        layer.remove();
      });
    });
    const [layer, setLayer] = useState<any>(null);

    const getCorners = () => {
      return layer ? layer.getCorners() : [];
    };

    useImperativeHandle(ref, () => ({
      getCorners,
    }));

    useEffect(() => {
      if (!layer) {
        map.whenReady(() => {
          //@ts-ignore
          const newLayer = L.distortableImageOverlay(imageUrl, {
            corners,
            editable,
            zIndex: 401,
            actions: [
              // @ts-ignore
              L.DragAction,
              // @ts-ignore
              L.ScaleAction,
              // @ts-ignore
              L.DistortAction,
              // @ts-ignore
              L.RotateAction,
              // @ts-ignore
              L.FreeRotateAction,
              // @ts-ignore
              L.BorderAction,
            ],
          });
          setLayer(newLayer);
        });
      }
    }, []);

    useEffect(() => {
      if (map && layer) {
        layer.addTo(map);
        layer.setOpacity(0.7);
      }
    }, [layer]);

    return null;
  },
);