Issue with Spiderfy unspiderfyAll: Breaking Errors When Toggling Layers #26

dejandiklic · 2024-08-07T06:50:41Z

When attempting to clean up spiderfy with unspiderfyAll, after toggling one layer, I expected it to work as intended. Instead, if I toggle on my layer, apply spiderfy to it, then toggle it off and on again, I encounter breaking errors from the library.

Steps to reproduce;

Initialize spiderfy on a Mapbox GL map.
Toggle on desired layer
Apply spiderfy to a clustered layer.
Toggle off desider layer.
Call unspiderfyAll
Toggle on desider layer.
Click on cluster.
Toggle off desider layer.

Source and Layer Information in use:

Source type: cluster
Layer type: symbol

Expected Behavior:
Everything works as expected. I can toggle the layer on and off as much as I want without issues.

Actual Behavior:
The app crashes

I logged the layers and sources after each toggle, and it seems that the map gets the correct state. I think the problem lies somewhere in the library, as the layers seem to duplicate. Specifically, on step 6, I get the following error:

Logs:
After step 1:

After step 2:

After step 5:

After step 6:

After step 7:

Versions
"@nazka/map-gl-js-spiderfy": "^1.2.3"
"mapbox-gl": "^3.4.0"

Relevant code:

`

const spiderfy = useMemo(() => {
	return new Spiderfy(map, {
		spiderLeavesPaint: {
			'text-color': 'black',
		},
		spiderLeavesLayout: fieldPerformerUnClusterLayoutConfig,
	});
}, [map]);

useEffect(() => {
	if (!map) return;
	if (fieldPerformer) {
		dispatch(fetchMapFieldPerformerAsync()).unwrap().then((geojson) => {
			map.addSource('fieldPerformer-source', fieldPerformersSource(geojson as GeoJSON));
			map.addLayer(FieldPerformerUnClusteredLayer);
			map.addLayer(FieldPerformerClusteredLayer);
			spiderfy.applyTo('fieldPerformer-layer-clustered');
		});
	} else {
		spiderfy.unspiderfyAll();
		removeClusteredLayerAndSource(map, 'fieldPerformer');
	}
}, [fieldPerformer, style]);

`

The text was updated successfully, but these errors were encountered:

simondriesen · 2024-08-20T13:31:29Z

I'm not able to reproduce your issue. I will need a working example for me to look into it more.

There should not even be a need to use spiderfy.unspiderfyAll(); when removing a spiderified layer. All should be cleaned automatically.

Here you have my test file for reference (just a map with a toggle button):

<html>
  <head>
    <title>Test</title>
    <link rel="stylesheet" href="https://api.mapbox.com/mapbox-gl-js/v3.6.0/mapbox-gl.css">
    <style>
      body { margin: 0; padding: 0; }
      #map { position: absolute; top: 0; bottom: 0; width: 100%; }
    </style>
  </head>
  <body>
    <div id="map"></div>
    <script type="module">
      import mapboxgl from "https://cdn.skypack.dev/[email protected]";
      import Spiderfy from "https://cdn.skypack.dev/@nazka/map-gl-js-spiderfy";

      mapboxgl.accessToken = ""; // Your Mapbox API token

      const map = new mapboxgl.Map({
        container: 'map',
        center: [4.7126, 50.8781],
        zoom: 11.5,
      });
      window.map = map;

      map.on('load', () => {
        map.addSource('markers', {
          'type': 'geojson',
          'data': 'https://raw.githubusercontent.com/nazka/map-gl-js-spiderfy/dev/demo/json/sample-data.json',
          'cluster': true, // must be true
        }); // public toilets in Leuven sample data

        map.loadImage(
        'https://raw.githubusercontent.com/nazka/map-gl-js-spiderfy/dev/demo/img/circle-yellow.png',
        (error, image) => {
          if (error) throw error;
          map.addImage('cluster', image);

          map.addLayer({
            'id': 'markers',
            'type': 'symbol',
            'source': 'markers',
            'layout': {
              'icon-image': 'cluster',
              'icon-allow-overlap': true
            }
          });
          
          map.addLayer({
          "id": "counters",
          "type": "symbol",
          "source": "markers",
          "layout": {
            "text-field": ["get", "point_count"],
            "text-size": 12,
            "text-allow-overlap": true
          }
        })
          
          const spiderfy = new Spiderfy(map, {
            onLeafClick: f => console.log(map.getStyle().layers, map.getStyle().sources),
            minZoomLevel: 12,
            zoomIncrement: 2,
          });
          window.spiderfy = spiderfy;
          spiderfy.applyTo('markers');
        });
      });
    </script>
    
    <!-- Toggle logic -->
    <button onclick="toggleLayer()" style="position: absolute;">toggle layer</button>
    <script>
      function toggleLayer() {
        const map = window.map;
        const spiderfy = window.spiderfy;
        if (map.getLayer('markers')) {
          map.removeLayer('markers');
          map.removeLayer('counters');
          map.removeSource('markers');
          spiderfy.unspiderfyAll();
          console.log(map.getStyle().layers, map.getStyle().sources)
        } else {
          map.addSource('markers', {
            type: 'geojson',
            data: 'https://raw.githubusercontent.com/nazka/map-gl-js-spiderfy/dev/demo/json/sample-data.json',
            cluster: true,
          });
          map.addLayer({
            id: 'markers',
            type: 'symbol',
            source: 'markers',
            layout: {
              'icon-image': 'cluster',
              'icon-allow-overlap': true,
            },
          });
          map.addLayer({
            id: 'counters',
            type: 'symbol',
            source: 'markers',
            layout: {
              'text-field': ['get', 'point_count'],
              'text-size': 12,
              'text-allow-overlap': true,
            },
          });
          console.log(map.getStyle().layers, map.getStyle().sources)
        }
      }
    </script>
  </body>
</html>

dejandiklic · 2024-09-02T15:49:32Z

Sorry for the late reply. Following the same logic you used in the example, I was able to fix my issue, which I would now call a misunderstanding of usage rather than a bug. I initially thought I needed to "remove" spiderify and apply it again when removing layers. My intended flow was:

Add layer
Apply spiderify to it
Remove layer
Remove spiderify

This would represent a layer toggle. However, upon closer inspection, I realized you only need to add spiderify once and use unspiderfyAll to clean up the map. The correct flow should be:

Add layer
Apply spiderify to it
Remove layer
unspiderfyAll
Add layer
Remove layer
unspiderfyAll

I've created a sandbox that demonstrates the usage I initially thought would work:
https://codesandbox.io/p/sandbox/keen-merkle-24flrv?file=%2Fsrc%2FApp.js%3A9%2C1

It might be worth considering adding a destroy or a removeLayer method that takes the same layer passed to the applyTo function. This way, we wouldn't need to keep track of whether spiderify has already been applied.

Thank you for your time and assistance in helping me understand this better. Your insights were very helpful!

simondriesen added the bug Something isn't working label Aug 20, 2024

simondriesen added enhancement New feature or request and removed bug Something isn't working labels Sep 3, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Issue with Spiderfy unspiderfyAll: Breaking Errors When Toggling Layers #26

Issue with Spiderfy unspiderfyAll: Breaking Errors When Toggling Layers #26

dejandiklic commented Aug 7, 2024

simondriesen commented Aug 20, 2024

dejandiklic commented Sep 2, 2024

Issue with Spiderfy unspiderfyAll: Breaking Errors When Toggling Layers #26

Issue with Spiderfy unspiderfyAll: Breaking Errors When Toggling Layers #26

Comments

dejandiklic commented Aug 7, 2024

simondriesen commented Aug 20, 2024

dejandiklic commented Sep 2, 2024