Cleanup app.graph usage (#7399)

Prior to the release of subgraphs, there was a single graph accessed
through `app.graph`. Now that there's multiple graphs, there's a lot of
code that needs to be reviewed and potentially updated depending on if
it cares about nearby nodes, all nodes, or something else requiring
specific attention.

This was done by simply changing the type of `app.graph` to unknown so
the typechecker will complain about every place it's currently used.
References were then updated to `app.rootGraph` if the previous usage
was correct, or actually rewritten.

By not getting rid of `app.graph`, this change already ensures that
there's no loss of functionality for custom nodes, but the prior typing
of `app.graph` can always be restored if future dissuasion of
`app.graph` usage creates issues.

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-7399-Cleanup-app-graph-usage-2c76d73d365081178743dfdcf07f44d0)
by [Unito](https://www.unito.io)
This commit is contained in:
AustinMroz
2025-12-11 22:37:34 -08:00
committed by GitHub
parent 88bdc605a7
commit f2a0e5102e
39 changed files with 192 additions and 209 deletions

View File

@@ -64,7 +64,7 @@ import type { ComfyExtension, MissingNodeType } from '@/types/comfy'
import { type ExtensionManager } from '@/types/extensionTypes'
import type { NodeExecutionId } from '@/types/nodeIdentification'
import { graphToPrompt } from '@/utils/executionUtil'
import { forEachNode } from '@/utils/graphTraversalUtil'
import { collectAllNodes, forEachNode } from '@/utils/graphTraversalUtil'
import {
getNodeByExecutionId,
triggerCallbackOnAllNodes
@@ -157,15 +157,15 @@ export class ComfyApp {
// TODO: Migrate internal usage to the
/** @deprecated Use {@link rootGraph} instead */
get graph() {
get graph(): unknown {
return this.rootGraphInternal!
}
get rootGraph(): LGraph | undefined {
get rootGraph(): LGraph {
if (!this.rootGraphInternal) {
console.error('ComfyApp graph accessed before initialization')
}
return this.rootGraphInternal
return this.rootGraphInternal!
}
// @ts-expect-error fixme ts strict error
@@ -512,7 +512,7 @@ export class ComfyApp {
}
}
app.graph.setDirtyCanvas(true)
app.canvas.setDirty(true)
useNodeOutputStore().updateNodeImages(node)
}
@@ -553,7 +553,7 @@ export class ComfyApp {
useEventListener(this.canvasElRef, 'dragleave', async () => {
if (!this.dragOverNode) return
this.dragOverNode = null
this.graph.setDirtyCanvas(false, true)
this.canvas.setDirty(false, true)
})
// Add handler for dropping onto a specific node
@@ -562,7 +562,10 @@ export class ComfyApp {
'dragover',
(event: DragEvent) => {
this.canvas.adjustMouseEvent(event)
const node = this.graph.getNodeOnPos(event.canvasX, event.canvasY)
const node = this.canvas.graph?.getNodeOnPos(
event.canvasX,
event.canvasY
)
if (!node?.onDragOver?.(event)) {
this.dragOverNode = null
@@ -573,7 +576,7 @@ export class ComfyApp {
// dragover event is fired very frequently, run this on an animation frame
requestAnimationFrame(() => {
this.graph.setDirtyCanvas(false, true)
this.canvas.setDirty(false, true)
})
},
false
@@ -638,11 +641,11 @@ export class ComfyApp {
})
api.addEventListener('progress', () => {
this.graph.setDirtyCanvas(true, false)
this.canvas.setDirty(true, false)
})
api.addEventListener('executing', () => {
this.graph.setDirtyCanvas(true, false)
this.canvas.setDirty(true, false)
})
api.addEventListener('executed', ({ detail }) => {
@@ -653,14 +656,14 @@ export class ComfyApp {
merge: detail.merge
})
const node = getNodeByExecutionId(this.graph, executionId)
const node = getNodeByExecutionId(this.rootGraph, executionId)
if (node && node.onExecuted) {
node.onExecuted(detail.output)
}
})
api.addEventListener('execution_start', () => {
triggerCallbackOnAllNodes(this.graph, 'onExecutionStart')
triggerCallbackOnAllNodes(this.rootGraph, 'onExecutionStart')
})
api.addEventListener('execution_error', ({ detail }) => {
@@ -844,7 +847,7 @@ export class ComfyApp {
registerProxyWidgets(this.canvas)
this.graph.start()
this.rootGraph.start()
// Ensure the canvas fills the window
useResizeObserver(this.canvasElRef, ([canvasEl]) => {
@@ -1194,17 +1197,18 @@ export class ComfyApp {
try {
// @ts-expect-error Discrepancies between zod and litegraph - in progress
this.graph.configure(graphData)
this.rootGraph.configure(graphData)
// Save original renderer version before scaling (it gets modified during scaling)
const originalMainGraphRenderer = this.graph.extra.workflowRendererVersion
const originalMainGraphRenderer =
this.rootGraph.extra.workflowRendererVersion
// Scale main graph
ensureCorrectLayoutScale(originalMainGraphRenderer)
// Scale all subgraphs that were loaded with the workflow
// Use original main graph renderer as fallback (not the modified one)
for (const subgraph of this.graph.subgraphs.values()) {
for (const subgraph of this.rootGraph.subgraphs.values()) {
ensureCorrectLayoutScale(
subgraph.extra.workflowRendererVersion || originalMainGraphRenderer,
subgraph
@@ -1235,7 +1239,7 @@ export class ComfyApp {
console.error(error)
return
}
for (const node of this.graph.nodes) {
forEachNode(this.rootGraph, (node) => {
const size = node.computeSize()
size[0] = Math.max(node.size[0], size[0])
size[1] = Math.max(node.size[1], size[1])
@@ -1284,7 +1288,7 @@ export class ComfyApp {
}
useExtensionService().invokeExtensions('loadedGraphNode', node)
}
})
if (missingNodeTypes.length && showMissingNodesDialog) {
this.showMissingNodesError(missingNodeTypes)
@@ -1309,14 +1313,14 @@ export class ComfyApp {
useTelemetry()?.trackWorkflowImported(telemetryPayload)
await useWorkflowService().afterLoadNewGraph(
workflow,
this.graph.serialize() as unknown as ComfyWorkflowJSON
this.rootGraph.serialize() as unknown as ComfyWorkflowJSON
)
requestAnimationFrame(() => {
this.graph.setDirtyCanvas(true, true)
this.canvas.setDirty(true, true)
})
}
async graphToPrompt(graph = this.graph) {
async graphToPrompt(graph = this.rootGraph) {
return graphToPrompt(graph, {
sortNodes: useSettingStore().get('Comfy.Workflow.SortNodeIdOnSave')
})
@@ -1351,12 +1355,12 @@ export class ComfyApp {
for (let i = 0; i < batchCount; i++) {
// Allow widgets to run callbacks before a prompt has been queued
// e.g. random seed before every gen
executeWidgetsCallback(this.graph.nodes, 'beforeQueued')
for (const subgraph of this.graph.subgraphs.values()) {
executeWidgetsCallback(subgraph.nodes, 'beforeQueued')
}
forEachNode(this.rootGraph, (node) => {
for (const widget of node.widgets ?? []) widget.beforeQueued?.()
})
const p = await this.graphToPrompt(this.graph)
const p = await this.graphToPrompt(this.rootGraph)
const queuedNodes = collectAllNodes(this.rootGraph)
try {
api.authToken = comfyOrgAuthToken
api.apiKey = comfyOrgApiKey ?? undefined
@@ -1397,16 +1401,7 @@ export class ComfyApp {
// Allow widgets to run callbacks after a prompt has been queued
// e.g. random seed after every gen
executeWidgetsCallback(
p.workflow.nodes
.map((n) => this.graph.getNodeById(n.id))
.filter((n) => !!n),
'afterQueued'
)
for (const subgraph of this.graph.subgraphs.values()) {
executeWidgetsCallback(subgraph.nodes, 'afterQueued')
}
executeWidgetsCallback(queuedNodes, 'afterQueued')
this.canvas.draw(true, true)
await this.ui.queue.update()
}
@@ -1481,7 +1476,7 @@ export class ComfyApp {
importA1111(this.graph, parameters)
useWorkflowService().afterLoadNewGraph(
fileName,
this.graph.serialize() as unknown as ComfyWorkflowJSON
this.rootGraph.serialize() as unknown as ComfyWorkflowJSON
)
return
}
@@ -1512,24 +1507,25 @@ export class ComfyApp {
}
const ids = Object.keys(apiData)
app.graph.clear()
app.rootGraph.clear()
for (const id of ids) {
const data = apiData[id]
const node = LiteGraph.createNode(data.class_type)
if (!node) continue
node.id = isNaN(+id) ? id : +id
node.title = data._meta?.title ?? node.title
app.graph.add(node)
app.rootGraph.add(node)
}
//TODO: Investigate repeat of for loop. Can compress?
for (const id of ids) {
const data = apiData[id]
const node = app.graph.getNodeById(id)
const node = app.rootGraph.getNodeById(id)
for (const input in data.inputs ?? {}) {
const value = data.inputs[input]
if (value instanceof Array) {
const [fromId, fromSlot] = value
const fromNode = app.graph.getNodeById(fromId)
const fromNode = app.rootGraph.getNodeById(fromId)
// @ts-expect-error fixme ts strict error
let toSlot = node.inputs?.findIndex((inp) => inp.name === input)
if (toSlot == null || toSlot === -1) {
@@ -1558,16 +1554,16 @@ export class ComfyApp {
}
}
}
app.graph.arrange()
app.rootGraph.arrange()
for (const id of ids) {
const data = apiData[id]
const node = app.graph.getNodeById(id)
const node = app.rootGraph.getNodeById(id)
for (const input in data.inputs ?? {}) {
const value = data.inputs[input]
if (value instanceof Array) {
const [fromId, fromSlot] = value
const fromNode = app.graph.getNodeById(fromId)
const fromNode = app.rootGraph.getNodeById(fromId)
// @ts-expect-error fixme ts strict error
let toSlot = node.inputs?.findIndex((inp) => inp.name === input)
if (toSlot == null || toSlot === -1) {
@@ -1597,11 +1593,11 @@ export class ComfyApp {
}
}
app.graph.arrange()
app.rootGraph.arrange()
useWorkflowService().afterLoadNewGraph(
fileName,
this.graph.serialize() as unknown as ComfyWorkflowJSON
this.rootGraph.serialize() as unknown as ComfyWorkflowJSON
)
}
@@ -1653,7 +1649,7 @@ export class ComfyApp {
this.registerNodeDef(nodeId, defs[nodeId])
}
// Refresh combo widgets in all nodes including those in subgraphs
forEachNode(this.graph, (node) => {
forEachNode(this.rootGraph, (node) => {
const def = defs[node.type]
// Allow primitive nodes to handle refresh
node.refreshComboInNode?.(defs)
@@ -1718,8 +1714,8 @@ export class ComfyApp {
// Subgraph does not properly implement `clear` and the parent class's
// (`LGraph`) `clear` breaks the subgraph structure.
if (this.graph && !this.canvas.subgraph) {
this.graph.clear()
if (this.rootGraph && !this.canvas.subgraph) {
this.rootGraph.clear()
}
}

View File

@@ -96,13 +96,13 @@ export class ChangeTracker {
const activeId = navigation.at(-1)
if (activeId) {
// Navigate to the saved subgraph
const subgraph = app.graph.subgraphs.get(activeId)
const subgraph = app.rootGraph.subgraphs.get(activeId)
if (subgraph) {
app.canvas.setGraph(subgraph)
}
} else {
// Empty navigation array means root level
app.canvas.setGraph(app.graph)
app.canvas.setGraph(app.rootGraph)
}
}
}
@@ -130,7 +130,7 @@ export class ChangeTracker {
checkState() {
if (!app.graph || this.changeCount) return
const currentState = clone(app.graph.serialize()) as ComfyWorkflowJSON
const currentState = clone(app.rootGraph.serialize()) as ComfyWorkflowJSON
if (!this.activeState) {
this.activeState = currentState
return

View File

@@ -22,7 +22,7 @@ export function clone<T>(obj: T): T {
* There are external callers to this function, so we need to keep it for now
*/
export function applyTextReplacements(app: ComfyApp, value: string): string {
return _applyTextReplacements(app.graph, value)
return _applyTextReplacements(app.rootGraph, value)
}
/** @knipIgnoreUnusedButUsedByCustomNodes */