Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
PeteMac3D opened this issue Sep 19, 2023 · 7 comments
Open

Comments

@PeteMac3D
Copy link

PeteMac3D commented Sep 19, 2023

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
Copy link

welcome bot commented Sep 19, 2023

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 PeteMac3D changed the title Usageage in React 18: Destroying a component with DistortableImage crashes app Use in React 18: Destroying a component with DistortableImage crashes app Sep 19, 2023
@PeteMac3D
Copy link
Author

visual of the problem for context
distortableImage_issue_91

@PeteMac3D
Copy link
Author

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
Copy link

jpoep commented Sep 20, 2023

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
Copy link
Author

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

@Preefix
Copy link

Preefix commented Oct 12, 2023

Any updates on this issue?

@yearsalary
Copy link

yearsalary commented Feb 5, 2024

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;
  },
);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants