[layer-maplibre, layer-leaflet] fix issues on clean-up

Fix : - multiple calls to the clean function leads to an error - clean function doesn't reset the sigma settings to the previous one
jacomyal · Oct 29, 2024 · cbb54fb · cbb54fb
1 parent 010d6f6
commit cbb54fb
Show file tree

Hide file tree

Showing 4 changed files with 141 additions and 14 deletions.
diff --git a/packages/layer-leaflet/src/index.ts b/packages/layer-leaflet/src/index.ts
@@ -2,7 +2,6 @@ import Graph from "graphology";
 import { Attributes } from "graphology-types";
 import L, { MapOptions } from "leaflet";
 import { Sigma } from "sigma";
-import { DEFAULT_SETTINGS } from "sigma/settings";
 
 import { graphToLatlng, latlngToGraph, setSigmaRatioBounds, syncMapWithSigma, syncSigmaWithMap } from "./utils";
 
@@ -22,6 +21,10 @@ export default function bindLeafletLayer(
     getNodeLatLng?: (nodeAttributes: Attributes) => { lat: number; lng: number };
   },
 ) {
+  // Keeping data for the cleanup
+  let isKilled = false;
+  const prevSigmaSettings = sigma.getSettings();
+
   // Creating map container
   const mapContainer = document.createElement("div");
   const mapContainerId = `${sigma.getContainer().id}-map`;
@@ -109,12 +112,21 @@ export default function bindLeafletLayer(
 
   // Clean up function to remove everything
   function clean() {
-    map.remove();
-    mapContainer.remove();
-    sigma.off("afterRender", fnSyncMapWithSigma);
-    sigma.off("resize", fnOnResize);
-    sigma.setSetting("stagePadding", DEFAULT_SETTINGS.stagePadding);
-    sigma.setSetting("enableCameraRotation", true);
+    if (!isKilled) {
+      isKilled = true;
+
+      map.remove();
+      mapContainer.remove();
+
+      sigma.off("afterRender", fnSyncMapWithSigma);
+      sigma.off("resize", fnOnResize);
+
+      // Reset settings
+      sigma.setSetting("stagePadding", prevSigmaSettings.stagePadding);
+      sigma.setSetting("enableCameraRotation", prevSigmaSettings.enableCameraRotation);
+      sigma.setSetting("minCameraRatio", prevSigmaSettings.minCameraRatio);
+      sigma.setSetting("maxCameraRatio", prevSigmaSettings.maxCameraRatio);
+    }
   }
 
   // When the map is ready

diff --git a/packages/layer-maplibre/src/index.ts b/packages/layer-maplibre/src/index.ts
@@ -20,6 +20,10 @@ export default function bindMaplibreLayer(
     getNodeLatLng?: (nodeAttributes: Attributes) => { lat: number; lng: number };
   },
 ) {
+  // Keeping data for the cleanup
+  let isKilled = false;
+  const prevSigmaSettings = sigma.getSettings();
+
   // Creating map container
   const mapContainer = document.createElement("div");
   const mapContainerId = `${sigma.getContainer().id}-map`;
@@ -91,13 +95,20 @@ export default function bindMaplibreLayer(
 
   // Clean up function to remove everything
   function clean() {
-    map.off("moveend", fnSyncSigmaWithMap);
-    map.remove();
-    mapContainer.remove();
-    sigma.off("afterRender", fnSyncMapWithSigma);
-    sigma.off("resize", fnOnResize);
-    sigma.setSetting("stagePadding", DEFAULT_SETTINGS.stagePadding);
-    sigma.setSetting("enableCameraRotation", true);
+    if (!isKilled) {
+      isKilled = true;
+
+      map.off("moveend", fnSyncSigmaWithMap);
+      map.remove();
+      mapContainer.remove();
+
+      sigma.off("afterRender", fnSyncMapWithSigma);
+      sigma.off("resize", fnOnResize);
+
+      // Reset settings
+      sigma.setSetting("stagePadding", prevSigmaSettings.stagePadding);
+      sigma.setSetting("enableCameraRotation", prevSigmaSettings.enableCameraRotation);
+    }
   }
 
   // Update the sigma graph for geospatial coords

diff --git a/packages/test/unit/layers/leaflet.ts b/packages/test/unit/layers/leaflet.ts
@@ -0,0 +1,52 @@
+import bindMapLayer from "@sigma/layer-leaflet";
+import Graph from "graphology";
+import { SerializedGraph } from "graphology-types";
+import Sigma from "sigma";
+import { createElement } from "sigma/utils";
+import { afterEach, beforeEach, describe, expect, test } from "vitest";
+
+const GRAPH: Pick<SerializedGraph, "nodes" | "edges"> = {
+  nodes: [
+    { key: "nantes", attributes: { x: 0, y: 0, lat: 47.2308, lng: 1.5566, size: 10, label: "Nantes" } },
+    { key: "paris", attributes: { x: 0, y: 0, lat: 48.8567, lng: 2.351, size: 10, label: "Paris" } },
+  ],
+  edges: [{ source: "nantes", target: "paris" }],
+};
+
+interface SigmaTestContext {
+  sigma: Sigma;
+}
+
+beforeEach<SigmaTestContext>(async (context) => {
+  const graph = new Graph();
+  graph.import(GRAPH);
+  const container = createElement("div", { width: "100px", height: "100px" });
+  document.body.append(container);
+  context.sigma = new Sigma(graph, container);
+});
+
+afterEach<SigmaTestContext>(async ({ sigma }) => {
+  sigma.kill();
+  sigma.getContainer().remove();
+});
+
+describe("@sigma/layer-leaflet", () => {
+  test<SigmaTestContext>("calling clean function multiple times should work", ({ sigma }) => {
+    const { clean } = bindMapLayer(sigma);
+    expect(() => {
+      clean();
+      clean();
+    }).not.to.throw();
+  });
+  test<SigmaTestContext>("clean the map should reset sigma'settings to its previous value", ({ sigma }) => {
+    const prevSettings = sigma.getSettings();
+
+    const { clean } = bindMapLayer(sigma);
+    const settings = sigma.getSettings();
+
+    clean();
+
+    expect(prevSettings).not.toEqual(settings);
+    expect(sigma.getSettings()).toEqual(prevSettings);
+  });
+});
diff --git a/packages/test/unit/layers/maplibre.ts b/packages/test/unit/layers/maplibre.ts
@@ -0,0 +1,52 @@
+import bindMapLayer from "@sigma/layer-maplibre";
+import Graph from "graphology";
+import { SerializedGraph } from "graphology-types";
+import Sigma from "sigma";
+import { createElement } from "sigma/utils";
+import { afterEach, beforeEach, describe, expect, test } from "vitest";
+
+const GRAPH: Pick<SerializedGraph, "nodes" | "edges"> = {
+  nodes: [
+    { key: "nantes", attributes: { x: 0, y: 0, lat: 47.2308, lng: 1.5566, size: 10, label: "Nantes" } },
+    { key: "paris", attributes: { x: 0, y: 0, lat: 48.8567, lng: 2.351, size: 10, label: "Paris" } },
+  ],
+  edges: [{ source: "nantes", target: "paris" }],
+};
+
+interface SigmaTestContext {
+  sigma: Sigma;
+}
+
+beforeEach<SigmaTestContext>(async (context) => {
+  const graph = new Graph();
+  graph.import(GRAPH);
+  const container = createElement("div", { width: "100px", height: "100px" });
+  document.body.append(container);
+  context.sigma = new Sigma(graph, container);
+});
+
+afterEach<SigmaTestContext>(async ({ sigma }) => {
+  sigma.kill();
+  sigma.getContainer().remove();
+});
+
+describe("@sigma/layer-maplibre", () => {
+  test<SigmaTestContext>("calling clean function multiple times should work", ({ sigma }) => {
+    const { clean } = bindMapLayer(sigma);
+    expect(() => {
+      clean();
+      clean();
+    }).not.to.throw();
+  });
+  test<SigmaTestContext>("clean the map should reset sigma'settings to its previous value", ({ sigma }) => {
+    const prevSettings = sigma.getSettings();
+
+    const { clean } = bindMapLayer(sigma);
+    const settings = sigma.getSettings();
+
+    clean();
+
+    expect(prevSettings).not.toEqual(settings);
+    expect(sigma.getSettings()).toEqual(prevSettings);
+  });
+});