fix(dnd): isolate pending drop and registration lifecycles

packages/dnd/package.json

@@ -1,6 +1,6 @@
 {
   "name": "@wrnexus/dnd",
-  "version": "0.1.0",
+  "version": "0.1.1",
   "type": "module",
   "description": "Stable accessible drag-and-drop primitives for WRNexus.",
   "exports": {

packages/dnd/src/manager.ts

@@ -20,14 +20,19 @@ export function createDndManager(options: DndManagerOptions): DndManager {
   >();
   const manager = new DragDropManager();
   let activeIds: Array<string | number> = [];
+  let generation = 0;
+  let destroyed = false;
 
   manager.monitor.addEventListener("dragstart", (event) => {
+    if (destroyed) return;
+    generation++;
     const source = event.operation.source;
     if (!source) return;
     activeIds = [source.id];
     options.announce?.({ type: "pickedUp", ids: activeIds });
   });
   manager.monitor.addEventListener("dragover", (event) => {
+    if (destroyed) return;
     const source = event.operation.source as
       (typeof event.operation.source & { group?: string | number; index?: number }) | null;
     if (!source) return;
@@ -38,6 +43,9 @@ export function createDndManager(options: DndManagerOptions): DndManager {
     } else options.announce?.({ type: "movedOver", ids: activeIds, location: destination });
   });
   manager.monitor.addEventListener("dragend", async (event) => {
+    if (destroyed) return;
+    const endedGeneration = generation;
+    const ids = [...activeIds];
     const source = event.operation.source as
       | (typeof event.operation.source & {
           group?: string | number;
@@ -47,12 +55,12 @@ export function createDndManager(options: DndManagerOptions): DndManager {
         })
       | null;
     if (!source || event.operation.canceled) {
-      options.announce?.({ type: "cancelled", ids: activeIds });
+      options.announce?.({ type: "cancelled", ids });
       activeIds = [];
       return;
     }
     const intent: DndMoveIntent = {
-      ids: [...activeIds],
+      ids,
       from: [
         {
           group: source.initialGroup ?? source.group ?? "default",
@@ -61,17 +69,32 @@ export function createDndManager(options: DndManagerOptions): DndManager {
       ],
       to: { group: source.group ?? "default", index: source.index ?? 0 },
     };
-    const accepted = await options.onMoveIntent(intent);
+    // A pending commit must not own the next gesture's selection or lifecycle.
+    activeIds = [];
+    let accepted: boolean | void;
+    try {
+      accepted =
+        options.accept && !options.accept(ids, intent.to)
+          ? false
+          : await options.onMoveIntent(intent);
+    } catch (error) {
+      if (!destroyed && generation === endedGeneration) {
+        options.announce?.({ type: "cancelled", ids, location: intent.to });
+        options.onError?.(error, intent);
+      }
+      return;
+    }
+    if (destroyed || generation !== endedGeneration) return;
     options.announce?.({
       type: accepted === false ? "cancelled" : "dropped",
-      ids: activeIds,
+      ids,
       location: intent.to,
     });
-    activeIds = [];
   });
 
   return {
     registerSortable(registration) {
+      if (destroyed) throw new Error("Cannot register a sortable after manager destruction.");
       if (entries.has(registration.id))
         throw new Error(`Duplicate sortable id ${String(registration.id)}.`);
       const sortable = new Sortable(
@@ -95,11 +118,17 @@ export function createDndManager(options: DndManagerOptions): DndManager {
         current: registration,
       });
       return () => {
+        // Cleanup can run after refresh or be called more than once.
+        if (entries.get(registration.id)?.sortable !== sortable) return;
         sortable.destroy();
         entries.delete(registration.id);
       };
     },
     destroy() {
+      if (destroyed) return;
+      destroyed = true;
+      generation++;
+      activeIds = [];
       for (const entry of entries.values()) entry.sortable.destroy();
       entries.clear();
       manager.destroy();

packages/dnd/src/types.ts

@@ -35,6 +35,8 @@ export interface DndManagerOptions {
   announce?: (announcement: DndAnnouncement) => void;
   accept?: (ids: DndId[], destination: SortableLocation) => boolean;
   onMoveIntent(intent: DndMoveIntent): boolean | void | Promise<boolean | void>;
+  /** Reports a failed commit without leaking an unhandled event-listener rejection. */
+  onError?(error: unknown, intent: DndMoveIntent): void;
 }
 
 export interface DndManager {

packages/dnd/test/manager-lifecycle.test.ts

@@ -0,0 +1,139 @@
+import { expect, test } from "bun:test";
+import { readFileSync } from "node:fs";
+import type { DndAnnouncement, DndManager, DndManagerOptions } from "../src/types.ts";
+
+// Run the real wrapper with an isolated backend, without global module mocks.
+const source = readFileSync(new URL("../src/manager.ts", import.meta.url), "utf8");
+const compiled = new Bun.Transpiler({ loader: "ts" })
+  .transformSync(source)
+  .replace(/^import .*;\s*$/gm, "")
+  .replace("export function createDndManager", "function createDndManager");
+
+function fixture(overrides: Partial<DndManagerOptions> = {}) {
+  const listeners: Record<string, (event: any) => any> = {};
+  const announcements: DndAnnouncement[] = [];
+  const instances: Array<{ destroyed: number }> = [];
+  let managerDestructions = 0;
+  class Backend {
+    monitor = {
+      addEventListener: (name: string, callback: any) => {
+        listeners[name] = callback;
+      },
+    };
+    destroy() {
+      managerDestructions++;
+    }
+  }
+  class Sortable {
+    destroyed = 0;
+    constructor() {
+      instances.push(this);
+    }
+    destroy() {
+      this.destroyed++;
+    }
+  }
+  const create = new Function(
+    "DragDropManager",
+    "Sortable",
+    compiled + ";return createDndManager;",
+  )(Backend, Sortable);
+  const manager: DndManager = create({
+    onMoveIntent: () => true,
+    announce: (event: DndAnnouncement) => announcements.push(event),
+    ...overrides,
+  });
+  const event = (id: string) => ({
+    operation: {
+      source: { id, group: "b", index: 1, initialGroup: "a", initialIndex: 0 },
+      canceled: false,
+    },
+  });
+  return {
+    manager,
+    announcements,
+    instances,
+    start: (id: string) => listeners.dragstart!(event(id)),
+    end: (id: string) => listeners.dragend!(event(id)),
+    get managerDestructions() {
+      return managerDestructions;
+    },
+  };
+}
+
+test("late drop resolution cannot clear or announce a newer gesture", async () => {
+  let finish!: (value: boolean) => void;
+  const f = fixture({
+    onMoveIntent: (intent) =>
+      intent.ids[0] === "first"
+        ? new Promise((resolve) => {
+            finish = resolve;
+          })
+        : true,
+  });
+  f.start("first");
+  const pending = f.end("first");
+  f.start("second");
+  finish(true);
+  await pending;
+  await f.end("second");
+  expect(f.announcements.filter((event) => event.type === "dropped")).toEqual([
+    { type: "dropped", ids: ["second"], location: { group: "b", index: 1 } },
+  ]);
+});
+
+test("drop commit rejection reports cancellation and its original intent", async () => {
+  const failure = new Error("save failed");
+  const errors: unknown[] = [];
+  const f = fixture({
+    onMoveIntent: async () => {
+      throw failure;
+    },
+    onError: (error, intent) => errors.push([error, intent.ids]),
+  });
+  f.start("first");
+  await f.end("first");
+  expect(errors).toEqual([[failure, ["first"]]]);
+  expect(f.announcements.at(-1)?.type).toBe("cancelled");
+});
+
+test("drop acceptance is checked again before dispatching a commit", async () => {
+  let commits = 0;
+  const f = fixture({
+    accept: () => false,
+    onMoveIntent: () => {
+      commits++;
+    },
+  });
+  f.start("first");
+  await f.end("first");
+  expect(commits).toBe(0);
+  expect(f.announcements.at(-1)?.type).toBe("cancelled");
+});
+
+test("old cleanup cannot remove replacement registrations and destruction is terminal", async () => {
+  let finish!: (value: boolean) => void;
+  const f = fixture({
+    onMoveIntent: () =>
+      new Promise((resolve) => {
+        finish = resolve;
+      }),
+  });
+  const registration = { id: "same", group: "a", index: 0, element: {} as HTMLElement };
+  const cleanup = f.manager.registerSortable(registration);
+  cleanup();
+  f.manager.registerSortable(registration);
+  cleanup();
+  expect(() => f.manager.registerSortable(registration)).toThrow("Duplicate");
+  f.start("same");
+  const pending = f.end("same");
+  f.manager.destroy();
+  f.manager.destroy();
+  finish(true);
+  await pending;
+  cleanup();
+  expect(f.instances.map((instance) => instance.destroyed)).toEqual([1, 1]);
+  expect(f.managerDestructions).toBe(1);
+  expect(() => f.manager.registerSortable(registration)).toThrow("destruction");
+  expect(f.announcements.filter((event) => event.type === "dropped")).toEqual([]);
+});