Skip to content

Commit 840f7f0

Browse files
authored
Cleanup: Litegraph/Vue synchronization work (#5789)
## Summary Cleanup and fixes to the existing syncing logic. ## Review Focus This is probably enough to review and test now. Main things that should still work: - moving nodes around - adding new ones - switching back and forth between Vue and Litegraph Let me know if you find any bugs that weren't already present there. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-5789-WIP-Litegraph-Vue-synchronization-work-27a6d73d3650811682cacacb82367b9e) by [Unito](https://www.unito.io)
1 parent 042c2ca commit 840f7f0

File tree

16 files changed

+151
-418
lines changed

16 files changed

+151
-418
lines changed

browser_tests/tests/vueNodes/interactions/links/linkInteraction.spec.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,7 @@ test.describe('Vue Node Link Interaction', () => {
100100

101101
const linkDetails = await comfyPage.page.evaluate((sourceId) => {
102102
const app = window['app']
103-
const graph = app?.canvas?.graph ?? app?.graph
103+
const graph = app?.canvas?.graph
104104
if (!graph) return null
105105

106106
const source = graph.getNodeById(sourceId)
@@ -164,7 +164,7 @@ test.describe('Vue Node Link Interaction', () => {
164164

165165
const graphLinkCount = await comfyPage.page.evaluate((sourceId) => {
166166
const app = window['app']
167-
const graph = app?.canvas?.graph ?? app?.graph
167+
const graph = app?.canvas?.graph
168168
if (!graph) return 0
169169

170170
const source = graph.getNodeById(sourceId)
@@ -207,7 +207,7 @@ test.describe('Vue Node Link Interaction', () => {
207207

208208
const graphLinkCount = await comfyPage.page.evaluate((sourceId) => {
209209
const app = window['app']
210-
const graph = app?.canvas?.graph ?? app?.graph
210+
const graph = app?.canvas?.graph
211211
if (!graph) return 0
212212

213213
const source = graph.getNodeById(sourceId)

src/components/graph/GraphCanvas.vue

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,7 @@ import NodeSearchboxPopover from '@/components/searchbox/NodeSearchBoxPopover.vu
9393
import SideToolbar from '@/components/sidebar/SideToolbar.vue'
9494
import SecondRowWorkflowTabs from '@/components/topbar/SecondRowWorkflowTabs.vue'
9595
import { useChainCallback } from '@/composables/functional/useChainCallback'
96+
import type { VueNodeData } from '@/composables/graph/useGraphNodeManager'
9697
import { useViewportCulling } from '@/composables/graph/useViewportCulling'
9798
import { useVueNodeLifecycle } from '@/composables/graph/useVueNodeLifecycle'
9899
import { useNodeBadge } from '@/composables/node/useNodeBadge'
@@ -189,8 +190,8 @@ watch(
189190
}
190191
)
191192
192-
const allNodes = computed(() =>
193-
Array.from(vueNodeLifecycle.vueNodeData.value.values())
193+
const allNodes = computed((): VueNodeData[] =>
194+
Array.from(vueNodeLifecycle.nodeManager.value?.vueNodeData?.values() ?? [])
194195
)
195196
196197
watchEffect(() => {
@@ -225,7 +226,6 @@ watch(
225226
for (const n of comfyApp.graph.nodes) {
226227
if (!n.widgets) continue
227228
for (const w of n.widgets) {
228-
// @ts-expect-error fixme ts strict error
229229
if (w[IS_CONTROL_WIDGET]) {
230230
updateControlWidgetLabel(w)
231231
if (w.linkedWidgets) {
@@ -364,7 +364,6 @@ const loadCustomNodesI18n = async () => {
364364
365365
const comfyAppReady = ref(false)
366366
const workflowPersistence = useWorkflowPersistence()
367-
// @ts-expect-error fixme ts strict error
368367
useCanvasDrop(canvasRef)
369368
useLitegraphSettings()
370369
useNodeBadge()

src/composables/graph/useVueNodeLifecycle.ts

Lines changed: 35 additions & 80 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,8 @@
11
import { createSharedComposable } from '@vueuse/core'
2-
import { readonly, ref, shallowRef, watch } from 'vue'
2+
import { shallowRef, watch } from 'vue'
33

44
import { useGraphNodeManager } from '@/composables/graph/useGraphNodeManager'
5-
import type {
6-
GraphNodeManager,
7-
VueNodeData
8-
} from '@/composables/graph/useGraphNodeManager'
5+
import type { GraphNodeManager } from '@/composables/graph/useGraphNodeManager'
96
import { useVueFeatureFlags } from '@/composables/useVueFeatureFlags'
107
import type { LGraphCanvas, LGraphNode } from '@/lib/litegraph/src/litegraph'
118
import { useCanvasStore } from '@/renderer/core/canvas/canvasStore'
@@ -22,31 +19,19 @@ function useVueNodeLifecycleIndividual() {
2219
const { shouldRenderVueNodes } = useVueFeatureFlags()
2320

2421
const nodeManager = shallowRef<GraphNodeManager | null>(null)
25-
const cleanupNodeManager = shallowRef<(() => void) | null>(null)
2622

27-
// Sync management
28-
const slotSync = shallowRef<ReturnType<typeof useSlotLayoutSync> | null>(null)
29-
const slotSyncStarted = ref(false)
30-
const linkSync = shallowRef<ReturnType<typeof useLinkLayoutSync> | null>(null)
31-
32-
// Vue node data state
33-
const vueNodeData = ref<ReadonlyMap<string, VueNodeData>>(new Map())
34-
35-
// Trigger for forcing computed re-evaluation
36-
const nodeDataTrigger = ref(0)
23+
const { startSync } = useLayoutSync()
24+
const linkSyncManager = useLinkLayoutSync()
25+
const slotSyncManager = useSlotLayoutSync()
3726

3827
const initializeNodeManager = () => {
3928
// Use canvas graph if available (handles subgraph contexts), fallback to app graph
40-
const activeGraph = comfyApp.canvas?.graph || comfyApp.graph
29+
const activeGraph = comfyApp.canvas?.graph
4130
if (!activeGraph || nodeManager.value) return
4231

4332
// Initialize the core node manager
4433
const manager = useGraphNodeManager(activeGraph)
4534
nodeManager.value = manager
46-
cleanupNodeManager.value = manager.cleanup
47-
48-
// Use the manager's data maps
49-
vueNodeData.value = manager.vueNodeData
5035

5136
// Initialize layout system with existing nodes from active graph
5237
const nodes = activeGraph._nodes.map((node: LGraphNode) => ({
@@ -76,46 +61,29 @@ function useVueNodeLifecycleIndividual() {
7661
}
7762

7863
// Initialize layout sync (one-way: Layout Store → LiteGraph)
79-
const { startSync } = useLayoutSync()
8064
startSync(canvasStore.canvas)
8165

82-
// Initialize link layout sync for event-driven updates
83-
const linkSyncManager = useLinkLayoutSync()
84-
linkSync.value = linkSyncManager
8566
if (comfyApp.canvas) {
8667
linkSyncManager.start(comfyApp.canvas)
8768
}
88-
89-
// Force computed properties to re-evaluate
90-
nodeDataTrigger.value++
9169
}
9270

9371
const disposeNodeManagerAndSyncs = () => {
9472
if (!nodeManager.value) return
9573

9674
try {
97-
cleanupNodeManager.value?.()
75+
nodeManager.value.cleanup()
9876
} catch {
9977
/* empty */
10078
}
10179
nodeManager.value = null
102-
cleanupNodeManager.value = null
103-
104-
// Clean up link layout sync
105-
if (linkSync.value) {
106-
linkSync.value.stop()
107-
linkSync.value = null
108-
}
10980

110-
// Reset reactive maps to clean state
111-
vueNodeData.value = new Map()
81+
linkSyncManager.stop()
11282
}
11383

11484
// Watch for Vue nodes enabled state changes
11585
watch(
116-
() =>
117-
shouldRenderVueNodes.value &&
118-
Boolean(comfyApp.canvas?.graph || comfyApp.graph),
86+
() => shouldRenderVueNodes.value && Boolean(comfyApp.canvas?.graph),
11987
(enabled) => {
12088
if (enabled) {
12189
initializeNodeManager()
@@ -138,47 +106,42 @@ function useVueNodeLifecycleIndividual() {
138106
}
139107

140108
// Switching to Vue
141-
if (vueMode && slotSyncStarted.value) {
142-
slotSync.value?.stop()
143-
slotSyncStarted.value = false
109+
if (vueMode) {
110+
slotSyncManager.stop()
144111
}
145112

146113
// Switching to LG
147114
const shouldRun = Boolean(canvas?.graph) && !vueMode
148-
if (shouldRun && !slotSyncStarted.value && canvas) {
149-
// Initialize slot sync if not already created
150-
if (!slotSync.value) {
151-
slotSync.value = useSlotLayoutSync()
152-
}
153-
const started = slotSync.value.attemptStart(canvas as LGraphCanvas)
154-
slotSyncStarted.value = started
115+
if (shouldRun && canvas) {
116+
slotSyncManager.attemptStart(canvas as LGraphCanvas)
155117
}
156118
},
157119
{ immediate: true }
158120
)
159121

160122
// Handle case where Vue nodes are enabled but graph starts empty
161123
const setupEmptyGraphListener = () => {
124+
const activeGraph = comfyApp.canvas?.graph
162125
if (
163-
shouldRenderVueNodes.value &&
164-
comfyApp.graph &&
165-
!nodeManager.value &&
166-
comfyApp.graph._nodes.length === 0
126+
!shouldRenderVueNodes.value ||
127+
nodeManager.value ||
128+
activeGraph?._nodes.length !== 0
167129
) {
168-
const originalOnNodeAdded = comfyApp.graph.onNodeAdded
169-
comfyApp.graph.onNodeAdded = function (node: LGraphNode) {
170-
// Restore original handler
171-
comfyApp.graph.onNodeAdded = originalOnNodeAdded
172-
173-
// Initialize node manager if needed
174-
if (shouldRenderVueNodes.value && !nodeManager.value) {
175-
initializeNodeManager()
176-
}
177-
178-
// Call original handler
179-
if (originalOnNodeAdded) {
180-
originalOnNodeAdded.call(this, node)
181-
}
130+
return
131+
}
132+
const originalOnNodeAdded = activeGraph.onNodeAdded
133+
activeGraph.onNodeAdded = function (node: LGraphNode) {
134+
// Restore original handler
135+
activeGraph.onNodeAdded = originalOnNodeAdded
136+
137+
// Initialize node manager if needed
138+
if (shouldRenderVueNodes.value && !nodeManager.value) {
139+
initializeNodeManager()
140+
}
141+
142+
// Call original handler
143+
if (originalOnNodeAdded) {
144+
originalOnNodeAdded.call(this, node)
182145
}
183146
}
184147
}
@@ -189,20 +152,12 @@ function useVueNodeLifecycleIndividual() {
189152
nodeManager.value.cleanup()
190153
nodeManager.value = null
191154
}
192-
if (slotSyncStarted.value) {
193-
slotSync.value?.stop()
194-
slotSyncStarted.value = false
195-
}
196-
slotSync.value = null
197-
if (linkSync.value) {
198-
linkSync.value.stop()
199-
linkSync.value = null
200-
}
155+
slotSyncManager.stop()
156+
linkSyncManager.stop()
201157
}
202158

203159
return {
204-
vueNodeData,
205-
nodeManager: readonly(nodeManager),
160+
nodeManager,
206161

207162
// Lifecycle methods
208163
initializeNodeManager,

src/composables/useCanvasDrop.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ import { useModelToNodeStore } from '@/stores/modelToNodeStore'
1414
import { ComfyNodeDefImpl } from '@/stores/nodeDefStore'
1515
import type { RenderedTreeExplorerNode } from '@/types/treeExplorerTypes'
1616

17-
export const useCanvasDrop = (canvasRef: Ref<HTMLCanvasElement>) => {
17+
export const useCanvasDrop = (canvasRef: Ref<HTMLCanvasElement | null>) => {
1818
const modelToNodeStore = useModelToNodeStore()
1919
const litegraphService = useLitegraphService()
2020
const workflowService = useWorkflowService()

src/composables/usePragmaticDragAndDrop.ts

Lines changed: 7 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -2,19 +2,17 @@ import {
22
draggable,
33
dropTargetForElements
44
} from '@atlaskit/pragmatic-drag-and-drop/element/adapter'
5-
import { onBeforeUnmount, onMounted } from 'vue'
5+
import { toValue } from 'vue'
6+
import { type MaybeRefOrGetter, onBeforeUnmount, onMounted } from 'vue'
67

78
export function usePragmaticDroppable(
8-
dropTargetElement: HTMLElement | (() => HTMLElement),
9+
dropTargetElement: MaybeRefOrGetter<HTMLElement | null>,
910
options: Omit<Parameters<typeof dropTargetForElements>[0], 'element'>
1011
) {
1112
let cleanup = () => {}
1213

1314
onMounted(() => {
14-
const element =
15-
typeof dropTargetElement === 'function'
16-
? dropTargetElement()
17-
: dropTargetElement
15+
const element = toValue(dropTargetElement)
1816

1917
if (!element) {
2018
return
@@ -32,16 +30,13 @@ export function usePragmaticDroppable(
3230
}
3331

3432
export function usePragmaticDraggable(
35-
draggableElement: HTMLElement | (() => HTMLElement),
33+
draggableElement: MaybeRefOrGetter<HTMLElement | null>,
3634
options: Omit<Parameters<typeof draggable>[0], 'element'>
3735
) {
3836
let cleanup = () => {}
3937

4038
onMounted(() => {
41-
const element =
42-
typeof draggableElement === 'function'
43-
? draggableElement()
44-
: draggableElement
39+
const element = toValue(draggableElement)
4540

4641
if (!element) {
4742
return
@@ -51,6 +46,7 @@ export function usePragmaticDraggable(
5146
element,
5247
...options
5348
})
49+
// TODO: Change to onScopeDispose
5450
})
5551

5652
onBeforeUnmount(() => {

src/extensions/core/groupNode.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -170,7 +170,7 @@ class GroupNodeBuilder {
170170
// Use the built in copyToClipboard function to generate the node data we need
171171
try {
172172
// @ts-expect-error fixme ts strict error
173-
const serialised = serialise(this.nodes, app.canvas.graph)
173+
const serialised = serialise(this.nodes, app.canvas?.graph)
174174
const config = JSON.parse(serialised)
175175

176176
storeLinkTypes(config)

src/lib/litegraph/src/LGraphCanvas.ts

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -757,9 +757,7 @@ export class LGraphCanvas
757757

758758
// Initialize link renderer if graph is available
759759
if (graph) {
760-
this.linkRenderer = new LitegraphLinkAdapter(graph)
761-
// Disable layout writes during render
762-
this.linkRenderer.enableLayoutStoreWrites = false
760+
this.linkRenderer = new LitegraphLinkAdapter(false)
763761
}
764762

765763
this.linkConnector.events.addEventListener('link-created', () =>
@@ -1858,9 +1856,7 @@ export class LGraphCanvas
18581856
newGraph.attachCanvas(this)
18591857

18601858
// Re-initialize link renderer with new graph
1861-
this.linkRenderer = new LitegraphLinkAdapter(newGraph)
1862-
// Disable layout writes during render
1863-
this.linkRenderer.enableLayoutStoreWrites = false
1859+
this.linkRenderer = new LitegraphLinkAdapter(false)
18641860

18651861
this.dispatch('litegraph:set-graph', { newGraph, oldGraph: graph })
18661862
this.#dirty()

src/lib/litegraph/src/types/widgets.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -251,6 +251,8 @@ export interface IBaseWidget<
251251
TType extends string = string,
252252
TOptions extends IWidgetOptions<unknown> = IWidgetOptions<unknown>
253253
> {
254+
[symbol: symbol]: boolean
255+
254256
linkedWidgets?: IBaseWidget[]
255257

256258
name: string

src/renderer/core/canvas/canvasStore.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,7 @@ export const useCanvasStore = defineStore('canvas', () => {
116116
newCanvas.canvas,
117117
'litegraph:set-graph',
118118
(event: CustomEvent<{ newGraph: LGraph; oldGraph: LGraph }>) => {
119-
const newGraph = event.detail?.newGraph || app.canvas?.graph
119+
const newGraph = event.detail?.newGraph ?? app.canvas?.graph // TODO: Ambiguous Graph
120120
currentGraph.value = newGraph
121121
isInSubgraph.value = Boolean(app.canvas?.subgraph)
122122
}

src/renderer/core/canvas/links/slotLinkCompatibility.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ import type {
1010
SlotDragSource,
1111
SlotDropCandidate
1212
} from '@/renderer/core/canvas/links/slotLinkDragState'
13-
import { app } from '@/scripts/app'
1413

1514
interface CompatibilityResult {
1615
allowable: boolean
@@ -21,7 +20,7 @@ interface CompatibilityResult {
2120
function resolveNode(nodeId: NodeId) {
2221
const pinia = getActivePinia()
2322
const canvasStore = pinia ? useCanvasStore() : null
24-
const graph = canvasStore?.canvas?.graph ?? app.canvas?.graph
23+
const graph = canvasStore?.canvas?.graph
2524
if (!graph) return null
2625
const id = typeof nodeId === 'string' ? Number(nodeId) : nodeId
2726
if (Number.isNaN(id)) return null

0 commit comments

Comments
 (0)