diff --git a/src/renderer/core/layout/store/layoutStore.test.ts b/src/renderer/core/layout/store/layoutStore.test.ts index ff7ee1fa06..eb4a15aa51 100644 --- a/src/renderer/core/layout/store/layoutStore.test.ts +++ b/src/renderer/core/layout/store/layoutStore.test.ts @@ -1,9 +1,14 @@ import { beforeEach, describe, expect, it, vi } from 'vitest' import { LiteGraph } from '@/lib/litegraph/src/litegraph' +import { getSlotKey } from '@/renderer/core/layout/slots/slotIdentifier' import { layoutStore } from '@/renderer/core/layout/store/layoutStore' import { LayoutSource } from '@/renderer/core/layout/types' -import type { LayoutChange, NodeLayout } from '@/renderer/core/layout/types' +import type { + LayoutChange, + NodeLayout, + SlotLayout +} from '@/renderer/core/layout/types' describe('layoutStore CRDT operations', () => { beforeEach(() => { @@ -406,4 +411,49 @@ describe('layoutStore CRDT operations', () => { LiteGraph.NODE_TITLE_HEIGHT = originalTitleHeight } }) + + it.for([ + { type: 'input' as const, isInput: true }, + { type: 'output' as const, isInput: false } + ])( + 'should preserve $type slot layouts when deleting a node', + ({ type, isInput }) => { + const nodeId = 'slot-persist-node' + const layout = createTestNode(nodeId) + + layoutStore.applyOperation({ + type: 'createNode', + entity: 'node', + nodeId, + layout, + timestamp: Date.now(), + source: LayoutSource.External, + actor: 'test' + }) + + const slotKey = getSlotKey(nodeId, 0, isInput) + const slotLayout: SlotLayout = { + nodeId, + index: 0, + type, + position: { x: 110, y: 120 }, + bounds: { x: 105, y: 115, width: 10, height: 10 } + } + layoutStore.batchUpdateSlotLayouts([{ key: slotKey, layout: slotLayout }]) + expect(layoutStore.getSlotLayout(slotKey)).toEqual(slotLayout) + + layoutStore.applyOperation({ + type: 'deleteNode', + entity: 'node', + nodeId, + previousLayout: layout, + timestamp: Date.now(), + source: LayoutSource.External, + actor: 'test' + }) + + // Slot layout must survive so Vue-patched components can still drag links + expect(layoutStore.getSlotLayout(slotKey)).toEqual(slotLayout) + } + ) }) diff --git a/src/renderer/core/layout/store/layoutStore.ts b/src/renderer/core/layout/store/layoutStore.ts index ed928bb00b..d5bab8031e 100644 --- a/src/renderer/core/layout/store/layoutStore.ts +++ b/src/renderer/core/layout/store/layoutStore.ts @@ -151,6 +151,10 @@ class LayoutStoreImpl implements LayoutStore { return this._pendingSlotSync } + get hasSlotLayouts(): boolean { + return this.slotLayouts.size > 0 + } + setPendingSlotSync(value: boolean): void { this._pendingSlotSync = value } @@ -513,23 +517,6 @@ class LayoutStoreImpl implements LayoutStore { } } - /** - * Delete all slot layouts for a node - */ - deleteNodeSlotLayouts(nodeId: NodeId): void { - const keysToDelete: string[] = [] - for (const [key, layout] of this.slotLayouts) { - if (layout.nodeId === nodeId) { - keysToDelete.push(key) - } - } - for (const key of keysToDelete) { - this.slotLayouts.delete(key) - // Remove from spatial index - this.slotSpatialIndex.remove(key) - } - } - /** * Clear all slot layouts and their spatial index (O(1) operations) * Used when switching rendering modes (Vue ↔ LiteGraph) @@ -1117,13 +1104,10 @@ class LayoutStoreImpl implements LayoutStore { // During undo/redo, Vue components may still hold references to the old ref. // If we delete the trigger, Vue won't be notified when the node is re-created. // The trigger will be called in finalizeOperation to notify Vue of the change. - + // We also intentionally do NOT delete slot layouts here for the same reason, + // and cleanup is handled by onUnmounted in useSlotElementTracking. // Remove from spatial index this.spatialIndex.remove(operation.nodeId) - - // Clean up associated slot layouts - this.deleteNodeSlotLayouts(operation.nodeId) - // Clean up associated links const linksToDelete = this.findLinksConnectedToNode(operation.nodeId) diff --git a/src/renderer/core/layout/types.ts b/src/renderer/core/layout/types.ts index 4332059f5e..2ca796e79b 100644 --- a/src/renderer/core/layout/types.ts +++ b/src/renderer/core/layout/types.ts @@ -302,7 +302,6 @@ export interface LayoutStore { deleteLinkLayout(linkId: LinkId): void deleteLinkSegmentLayout(linkId: LinkId, rerouteId: RerouteId | null): void deleteSlotLayout(key: string): void - deleteNodeSlotLayouts(nodeId: NodeId): void deleteRerouteLayout(rerouteId: RerouteId): void clearAllSlotLayouts(): void diff --git a/src/renderer/extensions/vueNodes/composables/useSlotElementTracking.test.ts b/src/renderer/extensions/vueNodes/composables/useSlotElementTracking.test.ts new file mode 100644 index 0000000000..4872c8f35d --- /dev/null +++ b/src/renderer/extensions/vueNodes/composables/useSlotElementTracking.test.ts @@ -0,0 +1,134 @@ +import { mount } from '@vue/test-utils' +import { createTestingPinia } from '@pinia/testing' +import { setActivePinia } from 'pinia' +import { beforeEach, describe, expect, it, vi } from 'vitest' +import { defineComponent, nextTick, ref } from 'vue' + +import { getSlotKey } from '@/renderer/core/layout/slots/slotIdentifier' +import { layoutStore } from '@/renderer/core/layout/store/layoutStore' +import { LayoutSource } from '@/renderer/core/layout/types' +import type { SlotLayout } from '@/renderer/core/layout/types' +import { useNodeSlotRegistryStore } from '@/renderer/extensions/vueNodes/stores/nodeSlotRegistryStore' + +import { + flushScheduledSlotLayoutSync, + useSlotElementTracking +} from './useSlotElementTracking' + +const mockGraph = vi.hoisted(() => ({ _nodes: [] as unknown[] })) + +vi.mock('@/scripts/app', () => ({ + app: { canvas: { graph: mockGraph, setDirty: vi.fn() } } +})) + +vi.mock('@/composables/element/useCanvasPositionConversion', () => ({ + useSharedCanvasPositionConversion: () => ({ + clientPosToCanvasPos: ([x, y]: [number, number]) => [x, y] + }) +})) + +const NODE_ID = 'test-node' +const SLOT_INDEX = 0 + +function createWrapperComponent(type: 'input' | 'output') { + return defineComponent({ + setup() { + const el = ref(null) + useSlotElementTracking({ + nodeId: NODE_ID, + index: SLOT_INDEX, + type, + element: el + }) + return { el } + }, + // No template ref — el starts null so the immediate watcher doesn't fire + // before the stop handle is assigned + template: '
' + }) +} + +/** + * Mount the wrapper, set the element ref, and wait for slot registration. + */ +async function mountAndRegisterSlot(type: 'input' | 'output') { + const wrapper = mount(createWrapperComponent(type)) + wrapper.vm.el = document.createElement('div') + await nextTick() + flushScheduledSlotLayoutSync() + return wrapper +} + +describe('useSlotElementTracking', () => { + beforeEach(() => { + setActivePinia(createTestingPinia({ stubActions: false })) + layoutStore.initializeFromLiteGraph([]) + layoutStore.applyOperation({ + type: 'createNode', + entity: 'node', + nodeId: NODE_ID, + layout: { + id: NODE_ID, + position: { x: 0, y: 0 }, + size: { width: 200, height: 100 }, + zIndex: 0, + visible: true, + bounds: { x: 0, y: 0, width: 200, height: 100 } + }, + timestamp: Date.now(), + source: LayoutSource.External, + actor: 'test' + }) + mockGraph._nodes = [{ id: 1 }] + }) + + it.each([ + { type: 'input' as const, isInput: true }, + { type: 'output' as const, isInput: false } + ])('cleans up $type slot layout on unmount', async ({ type, isInput }) => { + const wrapper = await mountAndRegisterSlot(type) + + const slotKey = getSlotKey(NODE_ID, SLOT_INDEX, isInput) + const registryStore = useNodeSlotRegistryStore() + expect(registryStore.getNode(NODE_ID)?.slots.has(slotKey)).toBe(true) + expect(layoutStore.getSlotLayout(slotKey)).not.toBeNull() + + wrapper.unmount() + + expect(layoutStore.getSlotLayout(slotKey)).toBeNull() + expect(registryStore.getNode(NODE_ID)).toBeUndefined() + }) + + it('clears pendingSlotSync when slot layouts already exist', () => { + // Seed a slot layout (simulates slot layouts persisting through undo/redo) + const slotKey = getSlotKey(NODE_ID, SLOT_INDEX, true) + const slotLayout: SlotLayout = { + nodeId: NODE_ID, + index: 0, + type: 'input', + position: { x: 0, y: 0 }, + bounds: { x: 0, y: 0, width: 10, height: 10 } + } + layoutStore.batchUpdateSlotLayouts([{ key: slotKey, layout: slotLayout }]) + + // Simulate what app.ts onConfigure does: set pending, then flush + layoutStore.setPendingSlotSync(true) + expect(layoutStore.pendingSlotSync).toBe(true) + + // No slots were scheduled (undo/redo — onMounted didn't fire), + // but slot layouts already exist. Flush should clear the flag. + flushScheduledSlotLayoutSync() + + expect(layoutStore.pendingSlotSync).toBe(false) + }) + + it('keeps pendingSlotSync when graph has nodes but no slot layouts', () => { + // No slot layouts exist (simulates initial mount before Vue registers slots) + layoutStore.setPendingSlotSync(true) + + flushScheduledSlotLayoutSync() + + // Should remain pending — waiting for Vue components to mount + expect(layoutStore.pendingSlotSync).toBe(true) + }) +}) diff --git a/src/renderer/extensions/vueNodes/composables/useSlotElementTracking.ts b/src/renderer/extensions/vueNodes/composables/useSlotElementTracking.ts index 2259870d7f..aa840527c1 100644 --- a/src/renderer/extensions/vueNodes/composables/useSlotElementTracking.ts +++ b/src/renderer/extensions/vueNodes/composables/useSlotElementTracking.ts @@ -37,12 +37,13 @@ export function flushScheduledSlotLayoutSync() { // No pending nodes - check if we should wait for Vue components to mount const graph = app.canvas?.graph const hasNodes = graph && graph._nodes && graph._nodes.length > 0 - if (hasNodes) { - // Graph has nodes but Vue hasn't mounted them yet - keep flag set - // so late mounts can re-assert it via scheduleSlotLayoutSync() + if (hasNodes && !layoutStore.hasSlotLayouts) { + // Graph has nodes but no slot layouts yet - Vue hasn't mounted. + // Keep flag set so late mounts can re-assert via scheduleSlotLayoutSync() return } - // No nodes in graph - safe to clear the flag (no Vue components will mount) + // Either no nodes (nothing to wait for) or slot layouts already exist + // (undo/redo preserved them). Clear the flag so links can render. layoutStore.setPendingSlotSync(false) app.canvas?.setDirty(true, true) return