diff --git a/code/CHANGELOG.md b/code/CHANGELOG.md index e50d95aa8..561c43ffa 100644 --- a/code/CHANGELOG.md +++ b/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 diff --git a/code/packages/sdk/src/nodes/__tests__/node.events.test.ts b/code/packages/sdk/src/nodes/__tests__/node.events.test.ts index 9f3341849..829979caa 100644 --- a/code/packages/sdk/src/nodes/__tests__/node.events.test.ts +++ b/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); diff --git a/code/packages/sdk/src/nodes/node.ts b/code/packages/sdk/src/nodes/node.ts index 26ee8b4ce..24700cafc 100644 --- a/code/packages/sdk/src/nodes/node.ts +++ b/code/packages/sdk/src/nodes/node.ts @@ -42,11 +42,44 @@ import { DEFAULT_THROTTLE_MS } from '@/constants'; // 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 void> + Map >(); +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; + 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 => { @@ -1211,13 +1244,13 @@ export abstract class WeaveNode implements WeaveNodeBase { 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(event, callback); + const instance = this.instance; + listeners.set(event, { + remove: () => instance.removeEventListener(event, callback), + }); + instance.addEventListener(event, callback); } /** @@ -1234,15 +1267,7 @@ export abstract class WeaveNode implements WeaveNodeBase { : [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); } }