Skip to content

Commit 138fa6a

Browse files
Resolve issues with undo with Nodes 2.0 to fix link dragging/rendering (#8808)
## Summary Resolves the following issue: 1. Enable Nodes 2.0 2. Load default workflow 3. Move any node e.g. VAE decode 4. Undo All links go invisible, input/output slots no longer function ## Changes - **What** - Fixes slot layouts being deleted during undo/redo in `handleDeleteNode`, which prevented link dragging from nodes after undo. Vue patches (not remounts) components with the same key, so `onMounted` never fires to re-register them - these were already being cleared up on unmounted - Fixes links disappearing after undo by clearing `pendingSlotSync` when slot layouts already exist (undo/redo preserved them), rather than waiting for Vue mounts that do not happen ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-8808-Resolve-issues-with-undo-with-Nodes-2-0-to-fix-link-dragging-rendering-3046d73d3650818bbb0adf0104a5792d) by [Unito](https://www.unito.io)
1 parent ce9d0ca commit 138fa6a

File tree

5 files changed

+196
-28
lines changed

5 files changed

+196
-28
lines changed

src/renderer/core/layout/store/layoutStore.test.ts

Lines changed: 51 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,14 @@
11
import { beforeEach, describe, expect, it, vi } from 'vitest'
22

33
import { LiteGraph } from '@/lib/litegraph/src/litegraph'
4+
import { getSlotKey } from '@/renderer/core/layout/slots/slotIdentifier'
45
import { layoutStore } from '@/renderer/core/layout/store/layoutStore'
56
import { LayoutSource } from '@/renderer/core/layout/types'
6-
import type { LayoutChange, NodeLayout } from '@/renderer/core/layout/types'
7+
import type {
8+
LayoutChange,
9+
NodeLayout,
10+
SlotLayout
11+
} from '@/renderer/core/layout/types'
712

813
describe('layoutStore CRDT operations', () => {
914
beforeEach(() => {
@@ -406,4 +411,49 @@ describe('layoutStore CRDT operations', () => {
406411
LiteGraph.NODE_TITLE_HEIGHT = originalTitleHeight
407412
}
408413
})
414+
415+
it.for([
416+
{ type: 'input' as const, isInput: true },
417+
{ type: 'output' as const, isInput: false }
418+
])(
419+
'should preserve $type slot layouts when deleting a node',
420+
({ type, isInput }) => {
421+
const nodeId = 'slot-persist-node'
422+
const layout = createTestNode(nodeId)
423+
424+
layoutStore.applyOperation({
425+
type: 'createNode',
426+
entity: 'node',
427+
nodeId,
428+
layout,
429+
timestamp: Date.now(),
430+
source: LayoutSource.External,
431+
actor: 'test'
432+
})
433+
434+
const slotKey = getSlotKey(nodeId, 0, isInput)
435+
const slotLayout: SlotLayout = {
436+
nodeId,
437+
index: 0,
438+
type,
439+
position: { x: 110, y: 120 },
440+
bounds: { x: 105, y: 115, width: 10, height: 10 }
441+
}
442+
layoutStore.batchUpdateSlotLayouts([{ key: slotKey, layout: slotLayout }])
443+
expect(layoutStore.getSlotLayout(slotKey)).toEqual(slotLayout)
444+
445+
layoutStore.applyOperation({
446+
type: 'deleteNode',
447+
entity: 'node',
448+
nodeId,
449+
previousLayout: layout,
450+
timestamp: Date.now(),
451+
source: LayoutSource.External,
452+
actor: 'test'
453+
})
454+
455+
// Slot layout must survive so Vue-patched components can still drag links
456+
expect(layoutStore.getSlotLayout(slotKey)).toEqual(slotLayout)
457+
}
458+
)
409459
})

src/renderer/core/layout/store/layoutStore.ts

Lines changed: 6 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -151,6 +151,10 @@ class LayoutStoreImpl implements LayoutStore {
151151
return this._pendingSlotSync
152152
}
153153

154+
get hasSlotLayouts(): boolean {
155+
return this.slotLayouts.size > 0
156+
}
157+
154158
setPendingSlotSync(value: boolean): void {
155159
this._pendingSlotSync = value
156160
}
@@ -513,23 +517,6 @@ class LayoutStoreImpl implements LayoutStore {
513517
}
514518
}
515519

516-
/**
517-
* Delete all slot layouts for a node
518-
*/
519-
deleteNodeSlotLayouts(nodeId: NodeId): void {
520-
const keysToDelete: string[] = []
521-
for (const [key, layout] of this.slotLayouts) {
522-
if (layout.nodeId === nodeId) {
523-
keysToDelete.push(key)
524-
}
525-
}
526-
for (const key of keysToDelete) {
527-
this.slotLayouts.delete(key)
528-
// Remove from spatial index
529-
this.slotSpatialIndex.remove(key)
530-
}
531-
}
532-
533520
/**
534521
* Clear all slot layouts and their spatial index (O(1) operations)
535522
* Used when switching rendering modes (Vue ↔ LiteGraph)
@@ -1117,13 +1104,10 @@ class LayoutStoreImpl implements LayoutStore {
11171104
// During undo/redo, Vue components may still hold references to the old ref.
11181105
// If we delete the trigger, Vue won't be notified when the node is re-created.
11191106
// The trigger will be called in finalizeOperation to notify Vue of the change.
1120-
1107+
// We also intentionally do NOT delete slot layouts here for the same reason,
1108+
// and cleanup is handled by onUnmounted in useSlotElementTracking.
11211109
// Remove from spatial index
11221110
this.spatialIndex.remove(operation.nodeId)
1123-
1124-
// Clean up associated slot layouts
1125-
this.deleteNodeSlotLayouts(operation.nodeId)
1126-
11271111
// Clean up associated links
11281112
const linksToDelete = this.findLinksConnectedToNode(operation.nodeId)
11291113

src/renderer/core/layout/types.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -302,7 +302,6 @@ export interface LayoutStore {
302302
deleteLinkLayout(linkId: LinkId): void
303303
deleteLinkSegmentLayout(linkId: LinkId, rerouteId: RerouteId | null): void
304304
deleteSlotLayout(key: string): void
305-
deleteNodeSlotLayouts(nodeId: NodeId): void
306305
deleteRerouteLayout(rerouteId: RerouteId): void
307306
clearAllSlotLayouts(): void
308307

Lines changed: 134 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,134 @@
1+
import { mount } from '@vue/test-utils'
2+
import { createTestingPinia } from '@pinia/testing'
3+
import { setActivePinia } from 'pinia'
4+
import { beforeEach, describe, expect, it, vi } from 'vitest'
5+
import { defineComponent, nextTick, ref } from 'vue'
6+
7+
import { getSlotKey } from '@/renderer/core/layout/slots/slotIdentifier'
8+
import { layoutStore } from '@/renderer/core/layout/store/layoutStore'
9+
import { LayoutSource } from '@/renderer/core/layout/types'
10+
import type { SlotLayout } from '@/renderer/core/layout/types'
11+
import { useNodeSlotRegistryStore } from '@/renderer/extensions/vueNodes/stores/nodeSlotRegistryStore'
12+
13+
import {
14+
flushScheduledSlotLayoutSync,
15+
useSlotElementTracking
16+
} from './useSlotElementTracking'
17+
18+
const mockGraph = vi.hoisted(() => ({ _nodes: [] as unknown[] }))
19+
20+
vi.mock('@/scripts/app', () => ({
21+
app: { canvas: { graph: mockGraph, setDirty: vi.fn() } }
22+
}))
23+
24+
vi.mock('@/composables/element/useCanvasPositionConversion', () => ({
25+
useSharedCanvasPositionConversion: () => ({
26+
clientPosToCanvasPos: ([x, y]: [number, number]) => [x, y]
27+
})
28+
}))
29+
30+
const NODE_ID = 'test-node'
31+
const SLOT_INDEX = 0
32+
33+
function createWrapperComponent(type: 'input' | 'output') {
34+
return defineComponent({
35+
setup() {
36+
const el = ref<HTMLElement | null>(null)
37+
useSlotElementTracking({
38+
nodeId: NODE_ID,
39+
index: SLOT_INDEX,
40+
type,
41+
element: el
42+
})
43+
return { el }
44+
},
45+
// No template ref — el starts null so the immediate watcher doesn't fire
46+
// before the stop handle is assigned
47+
template: '<div />'
48+
})
49+
}
50+
51+
/**
52+
* Mount the wrapper, set the element ref, and wait for slot registration.
53+
*/
54+
async function mountAndRegisterSlot(type: 'input' | 'output') {
55+
const wrapper = mount(createWrapperComponent(type))
56+
wrapper.vm.el = document.createElement('div')
57+
await nextTick()
58+
flushScheduledSlotLayoutSync()
59+
return wrapper
60+
}
61+
62+
describe('useSlotElementTracking', () => {
63+
beforeEach(() => {
64+
setActivePinia(createTestingPinia({ stubActions: false }))
65+
layoutStore.initializeFromLiteGraph([])
66+
layoutStore.applyOperation({
67+
type: 'createNode',
68+
entity: 'node',
69+
nodeId: NODE_ID,
70+
layout: {
71+
id: NODE_ID,
72+
position: { x: 0, y: 0 },
73+
size: { width: 200, height: 100 },
74+
zIndex: 0,
75+
visible: true,
76+
bounds: { x: 0, y: 0, width: 200, height: 100 }
77+
},
78+
timestamp: Date.now(),
79+
source: LayoutSource.External,
80+
actor: 'test'
81+
})
82+
mockGraph._nodes = [{ id: 1 }]
83+
})
84+
85+
it.each([
86+
{ type: 'input' as const, isInput: true },
87+
{ type: 'output' as const, isInput: false }
88+
])('cleans up $type slot layout on unmount', async ({ type, isInput }) => {
89+
const wrapper = await mountAndRegisterSlot(type)
90+
91+
const slotKey = getSlotKey(NODE_ID, SLOT_INDEX, isInput)
92+
const registryStore = useNodeSlotRegistryStore()
93+
expect(registryStore.getNode(NODE_ID)?.slots.has(slotKey)).toBe(true)
94+
expect(layoutStore.getSlotLayout(slotKey)).not.toBeNull()
95+
96+
wrapper.unmount()
97+
98+
expect(layoutStore.getSlotLayout(slotKey)).toBeNull()
99+
expect(registryStore.getNode(NODE_ID)).toBeUndefined()
100+
})
101+
102+
it('clears pendingSlotSync when slot layouts already exist', () => {
103+
// Seed a slot layout (simulates slot layouts persisting through undo/redo)
104+
const slotKey = getSlotKey(NODE_ID, SLOT_INDEX, true)
105+
const slotLayout: SlotLayout = {
106+
nodeId: NODE_ID,
107+
index: 0,
108+
type: 'input',
109+
position: { x: 0, y: 0 },
110+
bounds: { x: 0, y: 0, width: 10, height: 10 }
111+
}
112+
layoutStore.batchUpdateSlotLayouts([{ key: slotKey, layout: slotLayout }])
113+
114+
// Simulate what app.ts onConfigure does: set pending, then flush
115+
layoutStore.setPendingSlotSync(true)
116+
expect(layoutStore.pendingSlotSync).toBe(true)
117+
118+
// No slots were scheduled (undo/redo — onMounted didn't fire),
119+
// but slot layouts already exist. Flush should clear the flag.
120+
flushScheduledSlotLayoutSync()
121+
122+
expect(layoutStore.pendingSlotSync).toBe(false)
123+
})
124+
125+
it('keeps pendingSlotSync when graph has nodes but no slot layouts', () => {
126+
// No slot layouts exist (simulates initial mount before Vue registers slots)
127+
layoutStore.setPendingSlotSync(true)
128+
129+
flushScheduledSlotLayoutSync()
130+
131+
// Should remain pending — waiting for Vue components to mount
132+
expect(layoutStore.pendingSlotSync).toBe(true)
133+
})
134+
})

src/renderer/extensions/vueNodes/composables/useSlotElementTracking.ts

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -37,12 +37,13 @@ export function flushScheduledSlotLayoutSync() {
3737
// No pending nodes - check if we should wait for Vue components to mount
3838
const graph = app.canvas?.graph
3939
const hasNodes = graph && graph._nodes && graph._nodes.length > 0
40-
if (hasNodes) {
41-
// Graph has nodes but Vue hasn't mounted them yet - keep flag set
42-
// so late mounts can re-assert it via scheduleSlotLayoutSync()
40+
if (hasNodes && !layoutStore.hasSlotLayouts) {
41+
// Graph has nodes but no slot layouts yet - Vue hasn't mounted.
42+
// Keep flag set so late mounts can re-assert via scheduleSlotLayoutSync()
4343
return
4444
}
45-
// No nodes in graph - safe to clear the flag (no Vue components will mount)
45+
// Either no nodes (nothing to wait for) or slot layouts already exist
46+
// (undo/redo preserved them). Clear the flag so links can render.
4647
layoutStore.setPendingSlotSync(false)
4748
app.canvas?.setDirty(true, true)
4849
return

0 commit comments

Comments
 (0)