Spotlight cleanup (#5015)

* fix spotlight cleanup

* remove modal navigation from router transactions

* cache gathered paths

* e2e updates

* update selection test

* lint, add comments

* freeze tensorflow dep

* add destroy items

---------

Co-authored-by: Br2850 <bdbristol@gmail.com>

app/packages/spotlight/src/createScrollReader.ts

@@ -15,14 +15,17 @@ export default function createScrollReader(
   let timeout: ReturnType<typeof setTimeout>;
   let zooming = false;
 
-  element.addEventListener("scroll", () => {
+  const scroll = () => {
     scrolling = true;
-  });
+  };
+  element.addEventListener("scroll", scroll);
 
-  element.addEventListener("scrollend", () => {
+  const scrollEnd = () => {
     scrolling = false;
     requestAnimationFrame(() => render(zooming, true));
-  });
+  };
+
+  element.addEventListener("scrollend", scrollEnd);
 
   const updateScrollStatus = () => {
     const threshold = getScrollSpeedThreshold();
@@ -68,7 +71,9 @@ export default function createScrollReader(
   return {
     destroy: () => {
       destroyed = true;
+      element.removeEventListener("scroll", scroll);
+      element.removeEventListener("scrollend", scrollEnd);
     },
-    zooming: () => zooming
+    zooming: () => zooming,
   };
 }

app/packages/spotlight/src/index.ts

@@ -18,7 +18,7 @@ import {
   SCROLLBAR_WIDTH,
   TWO,
   ZERO,
-  ZOOMING_COEFFICIENT
+  ZOOMING_COEFFICIENT,
 } from "./constants";
 import createScrollReader from "./createScrollReader";
 import { Load, RowChange } from "./events";
@@ -108,8 +108,8 @@ export default class Spotlight<K, V> extends EventTarget {
       return;
     }
 
-    this.#backward?.remove();
-    this.#forward?.remove();
+    this.#backward?.destroy();
+    this.#forward?.destroy();
     this.#element?.classList.remove(styles.spotlightLoaded);
     this.#element?.remove();
     this.#scrollReader?.destroy();
@@ -204,7 +204,7 @@ export default class Spotlight<K, V> extends EventTarget {
               const backward = this.#forward;
               this.#forward = section;
               this.#forward.attach(this.#element);
-              this.#backward.remove();
+              this.#backward.destroy();
               this.#backward = backward;
               offset = before - this.#containerHeight + this.#config.spacing;
             }
@@ -272,7 +272,7 @@ export default class Spotlight<K, V> extends EventTarget {
               this.#backward = result.section;
               this.#backward.attach(this.#element);
 
-              this.#forward.remove();
+              this.#forward.destroy();
               this.#forward = forward;
             }
 

app/packages/spotlight/src/row.ts

@@ -13,6 +13,7 @@ export default class Row<K, V> {
   #from: number;
   #hidden: boolean;
 
+  readonly #aborter: AbortController = new AbortController();
   readonly #config: SpotlightConfig<K, V>;
   readonly #dangle?: boolean;
   readonly #container: HTMLDivElement = create(DIV);
@@ -47,7 +48,7 @@ export default class Row<K, V> {
       element.style.top = pixels(ZERO);
 
       if (config.onItemClick) {
-        element.addEventListener("click", (event) => {
+        const handler = (event) => {
           if (event.metaKey || event.shiftKey) {
             return;
           }
@@ -59,18 +60,13 @@ export default class Row<K, V> {
             item,
             iter,
           });
+        };
+
+        element.addEventListener("click", handler, {
+          signal: this.#aborter.signal,
         });
-        element.addEventListener("contextmenu", (event) => {
-          if (event.metaKey || event.shiftKey) {
-            return;
-          }
-          event.preventDefault();
-          focus(item.id);
-          config.onItemClick({
-            event,
-            item,
-            iter,
-          });
+        element.addEventListener("contextmenu", handler, {
+          signal: this.#aborter.signal,
         });
       }
 
@@ -123,6 +119,11 @@ export default class Row<K, V> {
     return this.#row[this.#row.length - ONE].item.id;
   }
 
+  destroy() {
+    this.#destroyItems();
+    this.#aborter.abort();
+  }
+
   has(item: string) {
     for (const i of this.#row) {
       if (i.item.id.description === item) {
@@ -138,6 +139,7 @@ export default class Row<K, V> {
     }
 
     this.#container.remove();
+    this.#destroyItems();
   }
 
   show(
@@ -225,4 +227,24 @@ export default class Row<K, V> {
     const set = new Set(this.#row.map(({ item }) => item.aspectRatio));
     return set.size === ONE ? this.#row[ZERO].item.aspectRatio : null;
   }
+
+  #destroyItems() {
+    const destroy = this.#config.destroy;
+    if (!destroy) {
+      return;
+    }
+
+    const errors = [];
+    for (const item of this.#row) {
+      try {
+        destroy(item.item.id);
+      } catch (e) {
+        errors.push(e);
+      }
+    }
+
+    if (errors.length > 0) {
+      console.error("Errors occurred during row destruction:", errors);
+    }
+  }
 }

app/packages/spotlight/src/section.ts

@@ -108,6 +108,12 @@ export default class Section<K, V> {
       : element.appendChild(this.#section);
   }
 
+  destroy() {
+    this.#section.remove();
+    for (const row of this.#rows) row.destroy();
+    this.#rows = [];
+  }
+
   find(item: string): Row<K, V> | null {
     for (const row of this.#rows) {
       if (row.has(item)) {
@@ -118,11 +124,6 @@ export default class Section<K, V> {
     return null;
   }
 
-  remove() {
-    this.#section.remove();
-    this.#rows = [];
-  }
-
   render({
     config,
     target,

app/packages/spotlight/src/types.ts

@@ -56,8 +56,9 @@ export type Request<K, V> = (key: K) => Promise<{
 }>;
 
 export interface SpotlightConfig<K, V> {
-  get: Get<K, V>;
   at?: At;
+  destroy?: (id: ID) => void;
+  get: Get<K, V>;
   key: K;
   offset?: number;
   onItemClick?: ItemClick<K, V>;