Update map-feature.disconnectedCallback to clean up when it's the last

Peter Rushforth · prushforth · commit 9ffba2ee5721 · 2025-09-17T12:00:38.000-04:00
feature in a MapFeatureLayer being disconnected, make it remove the
&lt;svg&gt; (_container) element created by the renderer.  Because Renderer
inherits from Layer, we can run its remove() method to do this.

Because `disabled` is an observed attribute on &lt;map-linK&gt;, purge the
shadowRoot contents when the link is disabled.

Add test for &lt;svg&gt; count when panning and zooming the map to prevent
regressions hopefully.
diff --git a/ToDo b/ToDo
@@ -107,7 +107,7 @@ especially for <map-feature>, that the _container is left with empty <svg> eleme
 after each move or zoom, and these build up.  Figure out how to make them go
 away after the last <map-feature> is removed.  This is something to do with
 the parameterization of the MapFeatureLayer (via options), and the associated
-renderer possibly.  Anyway, fix it and add a test.
+renderer possibly.  Anyway, fix it and add a test.  DONE
 
 I updated index.html to access the GeoServer canada provinces layer which includes
 a <map-link rel="features"> that can be used to debug.  Will have to figure out
diff --git a/src/map-feature.js b/src/map-feature.js
@@ -225,6 +225,11 @@ export class HTMLFeatureElement extends HTMLElement {
       this.removeFeature(this._featureLayer);
       // If this was the last feature in the layer, clean up the layer
       if (this._featureLayer.getLayers().length === 0) {
+        if (this._featureLayer.options.renderer) {
+          // since this is the last reference to the MapFeatureLayer, we need to also
+          // manually remove the shared renderer
+          this._featureLayer.options.renderer.remove();
+        }
         this._featureLayer.remove();
         this._featureLayer = null;
         delete this._featureLayer;
@@ -324,6 +329,9 @@ export class HTMLFeatureElement extends HTMLElement {
           this._featureLayer = new MapFeatureLayer(null, {
             // pass the vector layer a renderer of its own, otherwise leaflet
             // puts everything into the overlayPane
+            // with this feature creating its own MapFeatureLayer for each
+            // sub-sequence of features, it means that there may be > 1 <svg>
+            // container (one per renderer) in the pane...
             renderer: featureRenderer(),
             // pass the vector layer the container for the parent into which
             // it will append its own container for rendering into
diff --git a/src/map-link.js b/src/map-link.js
@@ -343,6 +343,7 @@ export class HTMLLinkElement extends HTMLElement {
         ) {
           this.parentExtent._extentLayer.removeLayer(this._templatedLayer);
           delete this._templatedLayer;
+          this.shadowRoot.innerHTML = '';
           this.getLayerEl()._validateDisabled();
         }
         break;
@@ -559,8 +560,7 @@ export class HTMLLinkElement extends HTMLElement {
         zIndex: this.zIndex,
         pane: this.parentExtent._extentLayer.getContainer(),
         linkEl: this,
-        projection: this.mapEl._map.options.projection,
-        renderer: this.mapEl._map.options.renderer
+        projection: this.mapEl._map.options.projection
       }).addTo(this.parentExtent._extentLayer);
     } else if (this.rel === 'query') {
       if (!this.shadowRoot) {
diff --git a/test/e2e/layers/renderer.html b/test/e2e/layers/renderer.html
@@ -0,0 +1,23 @@
+<!DOCTYPE html>
+<html lang="en">
+  <head>
+    <title>renderer.html</title>
+    <meta charset="UTF-8">
+    <script type="module" src="mapml.js"></script>
+  </head>
+  <body>
+    <mapml-viewer data-testid="map" style="width: 500px;height: 500px;" projection="CBMTILE" zoom="0" lat="45.5052040" lon="-75.2202344" controls>
+      <map-layer data-testid="test-layer" checked >
+       <map-extent data-test-id="test-extent" units="CBMTILE" checked="checked" >
+          <map-meta name="extent" content="top-left-easting=-6093972, top-left-northing=5350661, bottom-right-easting=5150842, bottom-right-northing=-5894153"></map-meta>
+          <map-input name="z" type="zoom" min="0" max="18" value="0" ></map-input>
+          <map-input name="xmin" type="location" units="gcrs" axis="longitude" position="top-left" ></map-input>
+          <map-input name="ymin" type="location" units="gcrs" axis="latitude" position="bottom-right" ></map-input>
+          <map-input name="xmax" type="location" units="gcrs" axis="longitude" position="bottom-right"  ></map-input>
+          <map-input name="ymax" type="location" units="gcrs" axis="latitude" position="top-left"  ></map-input>
+          <map-link data-testid="test-link" rel="features" tref="data/us_pop_density.mapml?{xmin}{ymin}{xmax}{ymax}{z}" ></map-link>
+        </map-extent>
+      </map-layer>
+    </mapml-viewer>
+  </body>
+</html>
diff --git a/test/e2e/layers/templatedFeaturesCleanup.test.js b/test/e2e/layers/templatedFeaturesCleanup.test.js
@@ -0,0 +1,38 @@
+import { test, expect, chromium } from '@playwright/test';
+
+test.describe('Templated features cleanup test' , () => {
+  let page;
+  let context;
+  test.beforeAll(async function () {
+    context = await chromium.launchPersistentContext('');
+    page =
+      context.pages().find((page) => page.url() === 'about:blank') ||
+      (await context.newPage());
+    await page.goto('renderer.html');
+  });
+
+  test.afterAll(async function () {
+    await context.close();
+  });
+
+  test('Templated <map-link rel="features"> renderer cleans up after itself', async () => {
+    await page.waitForTimeout(1000);
+    const link = page.getByTestId('test-link');
+    let svgCount = await link.evaluate((l) => l._templatedLayer._container.querySelectorAll('svg').length);
+    const viewer = page.getByTestId('map');
+    await page.getByLabel('Zoom in').click();
+    await page.getByLabel('Zoom in').click();
+    await page.getByLabel('Zoom in').click();
+    await page.getByLabel('Zoom in').click();
+    await page.waitForTimeout(1000);
+    svgCount = await link.evaluate((l) => l._templatedLayer._container.querySelectorAll('svg').length);
+    expect(svgCount).toEqual(1);
+    await page.getByLabel('Interactive map').press('ArrowLeft');
+    await page.getByLabel('Interactive map').press('ArrowLeft');
+    await page.getByLabel('Interactive map').press('ArrowLeft');
+    await page.getByLabel('Reload').click();
+    await page.waitForTimeout(1000);
+    svgCount = await link.evaluate((l) => l._templatedLayer._container.querySelectorAll('svg').length);
+    expect(svgCount).toEqual(1);
+  });
+});
