fix(tui): drop the keymap listener when the renderer is destroyed

Author: TheGreatAxios

src/tui/list-modal.test.ts

@@ -3,6 +3,8 @@ import { afterEach, describe, expect, test } from "bun:test";
 import { defined } from "../../testkit/defined.js";
 import { createHarness, type Harness } from "./harness.js";
 import { runListModal, type ListModalConfig } from "./list-modal.js";
+import { createAppShell } from "./shell/index.js";
+import { pokeShellKeymapPress, shellKeymapListening } from "./shell/keymap.js";
 
 let harness: Harness | undefined;
 
@@ -30,6 +32,28 @@ async function mountModal(overrides: Partial<ListModalConfig> = {}): Promise<{
 }
 
 describe("runListModal", () => {
+  test("renderer destroy detaches the keymap before a later keypress", async () => {
+    const h = await createHarness({ width: 40, height: 12 });
+    try {
+      const shell = createAppShell(h.renderer, {
+        title: "keymap",
+        run: "idle",
+      });
+      expect(shellKeymapListening(h.renderer)).toBe(true);
+      const before = h.renderer.keyInput.listenerCount("keypress");
+      expect(before).toBeGreaterThan(0);
+      shell.dispose();
+      h.renderer.destroy();
+      expect(shellKeymapListening(h.renderer)).toBe(false);
+      expect(h.renderer.keyInput.listenerCount("keypress")).toBeLessThan(
+        before,
+      );
+      expect(() => pokeShellKeymapPress(h.renderer)).not.toThrow();
+    } finally {
+      h.destroy();
+    }
+  });
+
   test("resolves the id of the accepted row", async () => {
     const { choice, harness } = await mountModal();
     harness.pressKey("Enter");

src/tui/shell/keymap.ts

@@ -1,10 +1,20 @@
 /**
  * Registers `APP_CHORDS` on `@opentui/keymap`. Lookup belongs to the package.
  * There is no second switch from key name to handler.
+ *
+ * The default host keeps its keypress listener until the destroy event, but
+ * `isDestroyed` flips first and leftover stdin then throws inside
+ * `getLiveHost`. This host unsubscribes inside `destroy()` and ignores any
+ * keypress already queued for this turn.
  */
 import type { CliRenderer, KeyEvent, Renderable } from "@opentui/core";
-import type { Keymap } from "@opentui/keymap";
-import { createDefaultOpenTuiKeymap } from "@opentui/keymap/opentui";
+import { Keymap, type KeymapHost } from "@opentui/keymap";
+import {
+  registerDefaultKeys,
+  registerEnabledFields,
+  registerMetadataFields,
+} from "@opentui/keymap/addons";
+import { createOpenTuiKeymapHost } from "@opentui/keymap/opentui";
 
 import { bindAppHelp, setAppShortcutRows } from "../keybindings.js";
 import type { AppShell } from "./internals.js";
@@ -21,13 +31,78 @@ bindAppHelp(appHelpLines);
 type ShellKeymap = Keymap<Renderable, KeyEvent>;
 
 const keymaps = new WeakMap<CliRenderer, ShellKeymap>();
+const detached = new WeakSet<CliRenderer>();
+const pressListeners = new WeakMap<
+  CliRenderer,
+  ((event: KeyEvent) => void)[]
+>();
+
+function guardHost(renderer: CliRenderer): KeymapHost<Renderable, KeyEvent> {
+  const host = createOpenTuiKeymapHost(renderer);
+  const onKeyPress = host.onKeyPress.bind(host);
+  const onKeyRelease = host.onKeyRelease.bind(host);
+  const offs: (() => void)[] = [];
+  const ignore = (): boolean => detached.has(renderer) || renderer.isDestroyed;
+
+  const wrap = (
+    subscribe: (listener: (event: KeyEvent) => void) => () => void,
+    listener: (event: KeyEvent) => void,
+    record: boolean,
+  ): (() => void) => {
+    const wrapped = (event: KeyEvent): void => {
+      if (ignore()) return;
+      listener(event);
+    };
+    if (record) {
+      const list = pressListeners.get(renderer) ?? [];
+      list.push(wrapped);
+      pressListeners.set(renderer, list);
+    }
+    const off = subscribe(wrapped);
+    offs.push(off);
+    return off;
+  };
+
+  host.onKeyPress = (listener) => wrap(onKeyPress, listener, true);
+  host.onKeyRelease = (listener) => wrap(onKeyRelease, listener, false);
+
+  const originalDestroy = renderer.destroy.bind(renderer);
+  renderer.destroy = () => {
+    if (!detached.has(renderer)) {
+      detached.add(renderer);
+      for (const off of offs) off();
+      offs.length = 0;
+    }
+    originalDestroy();
+  };
+
+  return host;
+}
 
 function keymapFor(renderer: CliRenderer): ShellKeymap {
   const existing = keymaps.get(renderer);
   if (existing !== undefined) return existing;
-  const created = createDefaultOpenTuiKeymap(renderer);
-  keymaps.set(renderer, created);
-  return created;
+  const keymap = new Keymap(guardHost(renderer));
+  registerDefaultKeys(keymap);
+  registerEnabledFields(keymap);
+  registerMetadataFields(keymap);
+  keymaps.set(renderer, keymap);
+  return keymap;
+}
+
+/** True while this renderer's keymap is still subscribed to keypress. */
+export function shellKeymapListening(renderer: CliRenderer): boolean {
+  return keymaps.has(renderer) && !detached.has(renderer);
+}
+
+/**
+ * Re-enter the keymap press listeners. After destroy they must return
+ * without calling into a destroyed host.
+ */
+export function pokeShellKeymapPress(renderer: CliRenderer): void {
+  for (const listener of pressListeners.get(renderer) ?? []) {
+    listener({ name: "c" } as KeyEvent);
+  }
 }
 
 function helpFromKeymap(keymap: ShellKeymap): readonly string[] {