Grouping and ungrouping leak the event listeners of the moved nodes' descendants

code/CHANGELOG.md

@@ -20,6 +20,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Fixed
 
+- [#1214](https://github.com/InditexTech/weavejs/issues/1214) Grouping and ungrouping leak the event listeners of the moved nodes' descendants
 - [#1229](https://github.com/InditexTech/weavejs/issues/1229) Nx caches the publish and unpublish targets, so a cache hit skips the actual publish
 - [#1227](https://github.com/InditexTech/weavejs/issues/1227) Harden the SonarCloud, release preview and OSS compliance workflows
 - [#1222](https://github.com/InditexTech/weavejs/issues/1222) Server backend setup sets the backend flag even when the native backend fails to load

code/packages/sdk/src/nodes/__tests__/node.events.test.ts

@@ -1447,6 +1447,31 @@ describe('WeaveNode instance listeners lifecycle', () => {
     expect(listenerCounts(emitter)).toEqual([0, 0]);
   });
 
+  it('removes the listeners of descendants when a container is destroyed directly', () => {
+    const { node, mock } = makeNode();
+    const emitter = attachEmitter(mock);
+    const parent = createGroup({ id: 'parent' });
+    const child = createGroup({ id: 'child' });
+    parent.add(child);
+
+    node.setupDefaultNodeEvents(parent);
+    node.setupDefaultNodeEvents(child);
+    parent.destroy();
+
+    expect(listenerCounts(emitter)).toEqual([0, 0]);
+  });
+
+  it('does not throw when a node is destroyed twice', () => {
+    const { node, mock } = makeNode();
+    attachEmitter(mock);
+    const group = createGroup();
+
+    node.setupDefaultNodeEvents(group);
+    group.destroy();
+
+    expect(() => group.destroy()).not.toThrow();
+  });
+
   it('keeps the listeners of other nodes when a node is destroyed', () => {
     const { node, mock } = makeNode();
     const emitter = attachEmitter(mock);

code/packages/sdk/src/nodes/node.ts

@@ -42,11 +42,44 @@
 
 // Weave instance listeners registered on behalf of each Konva node, so they can
 // be replaced when the node events are set up again and removed on destroy.
+type NodeInstanceListener = { remove: () => void };
 const nodeInstanceListeners = new WeakMap<
   Konva.Node,
-  Map<string, (payload: never) => void>
+  Map<string, NodeInstanceListener>
 >();
 
+const removeListenersOfKonvaNode = (node: Konva.Node): void => {
+  const listeners = nodeInstanceListeners.get(node);
+  if (!listeners) {
+    return;
+  }
+  for (const listener of listeners.values()) {
+    listener.remove();
+  }
+  nodeInstanceListeners.delete(node);
+};
+
+// Konva calls destroy() on every descendant of a destroyed container, so
+// hooking it guarantees the listeners are removed even when the nodes are
+// detached from their parent before the handler's onDestroy runs.
+const KONVA_DESTROY_PATCHED = Symbol.for('weave.konva.destroyPatched');
+const patchKonvaNodeDestroy = (): void => {
+  if (!Konva?.Node?.prototype) {
+    return;
+  }
+  const proto = Konva.Node.prototype as unknown as Record<symbol, unknown>;
+  if (proto[KONVA_DESTROY_PATCHED]) {
+    return;
+  }
+  const originalDestroy = Konva.Node.prototype.destroy;
+  Konva.Node.prototype.destroy = function (this: Konva.Node) {
+    removeListenersOfKonvaNode(this);
+    return originalDestroy.call(this);
+  };
+  Object.defineProperty(proto, KONVA_DESTROY_PATCHED, { value: true });
+};
+patchKonvaNodeDestroy();
+
 export const augmentKonvaNodeClass = (
   config?: WeaveNodeConfiguration
 ): void => {
@@ -1173,7 +1206,7 @@
     };
   }
 
   async onRegister(): Promise<void> {
     return;
   }
 
@@ -1211,13 +1244,13 @@
       nodeInstanceListeners.set(node, listeners);
     }
 
-    const previous = listeners.get(event);
-    if (previous) {
-      this.instance.removeEventListener(event, previous);
-    }
+    listeners.get(event)?.remove();
 
-    listeners.set(event, callback as (payload: never) => void);
-    this.instance.addEventListener<T>(event, callback);
+    const instance = this.instance;
+    listeners.set(event, {
+      remove: () => instance.removeEventListener(event, callback),
+    });
+    instance.addEventListener<T>(event, callback);
   }
 
   /**
@@ -1234,15 +1267,7 @@
         : [node];
 
     for (const actNode of nodes) {
-      const listeners = nodeInstanceListeners.get(actNode);
-      if (!listeners) {
-        continue;
-      }
-
-      for (const [event, callback] of listeners) {
-        this.instance.removeEventListener(event, callback);
-      }
-      nodeInstanceListeners.delete(actNode);
+      removeListenersOfKonvaNode(actNode);
     }
   }