diff --git a/src/composables/useCoreCommands.ts b/src/composables/useCoreCommands.ts index ce5587104..851cee2da 100644 --- a/src/composables/useCoreCommands.ts +++ b/src/composables/useCoreCommands.ts @@ -180,8 +180,6 @@ export function useCoreCommands(): ComfyCommand[] { const subgraph = app.canvas.subgraph const nonIoNodes = getAllNonIoNodesInSubgraph(subgraph) nonIoNodes.forEach((node) => subgraph.remove(node)) - } else { - app.graph.clear() } api.dispatchCustomEvent('graphCleared') } diff --git a/src/lib/litegraph/src/LGraph.ts b/src/lib/litegraph/src/LGraph.ts index 17b7f6c17..139919c60 100644 --- a/src/lib/litegraph/src/LGraph.ts +++ b/src/lib/litegraph/src/LGraph.ts @@ -314,6 +314,7 @@ export class LGraph if (this._nodes) { for (const _node of this._nodes) { _node.onRemoved?.() + this.onNodeRemoved?.(_node) } } diff --git a/src/scripts/app.ts b/src/scripts/app.ts index 1e9df767e..b257b85c5 100644 --- a/src/scripts/app.ts +++ b/src/scripts/app.ts @@ -1058,6 +1058,8 @@ export class ComfyApp { checkForRerouteMigration = false } = {} ) { + useWorkflowService().beforeLoadNewGraph() + if (clean !== false) { this.clean() } @@ -1093,7 +1095,6 @@ export class ComfyApp { severity: 'warn' }) } - useWorkflowService().beforeLoadNewGraph() useSubgraphService().loadSubgraphs(graphData) const missingNodeTypes: MissingNodeType[] = [] @@ -1765,6 +1766,12 @@ export class ComfyApp { executionStore.lastExecutionError = null useDomWidgetStore().clear() + + // 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() + } } clientPosToCanvasPos(pos: Vector2): Vector2 { diff --git a/src/scripts/ui.ts b/src/scripts/ui.ts index b2d075665..8ab6814c8 100644 --- a/src/scripts/ui.ts +++ b/src/scripts/ui.ts @@ -634,7 +634,6 @@ export class ComfyUI { confirm('Clear workflow?') ) { app.clean() - app.graph.clear() useLitegraphService().resetView() api.dispatchCustomEvent('graphCleared') } diff --git a/tests-ui/tests/composables/useCoreCommands.test.ts b/tests-ui/tests/composables/useCoreCommands.test.ts index a9e801858..0e1ead4d9 100644 --- a/tests-ui/tests/composables/useCoreCommands.test.ts +++ b/tests-ui/tests/composables/useCoreCommands.test.ts @@ -6,17 +6,25 @@ import { api } from '@/scripts/api' import { app } from '@/scripts/app' import { useSettingStore } from '@/stores/settingStore' -vi.mock('@/scripts/app', () => ({ - app: { - clean: vi.fn(), - canvas: { - subgraph: null - }, - graph: { - clear: vi.fn() +vi.mock('@/scripts/app', () => { + const mockGraphClear = vi.fn() + const mockCanvas = { subgraph: undefined } + + return { + app: { + clean: vi.fn(() => { + // Simulate app.clean() calling graph.clear() only when not in subgraph + if (!mockCanvas.subgraph) { + mockGraphClear() + } + }), + canvas: mockCanvas, + graph: { + clear: mockGraphClear + } } } -})) +}) vi.mock('@/scripts/api', () => ({ api: { diff --git a/tests-ui/tests/litegraph/core/LGraph.test.ts b/tests-ui/tests/litegraph/core/LGraph.test.ts index 15d9d4efe..61d3da734 100644 --- a/tests-ui/tests/litegraph/core/LGraph.test.ts +++ b/tests-ui/tests/litegraph/core/LGraph.test.ts @@ -1,6 +1,6 @@ import { describe } from 'vitest' -import { LGraph, LiteGraph } from '@/lib/litegraph/src/litegraph' +import { LGraph, LGraphNode, LiteGraph } from '@/lib/litegraph/src/litegraph' import { test } from './fixtures/testExtensions' @@ -128,6 +128,54 @@ describe('Floating Links / Reroutes', () => { }) }) +describe('Graph Clearing and Callbacks', () => { + test('clear() calls both node.onRemoved() and graph.onNodeRemoved()', ({ + expect + }) => { + const graph = new LGraph() + + // Create test nodes with onRemoved callbacks + const node1 = new LGraphNode('TestNode1') + const node2 = new LGraphNode('TestNode2') + + // Add nodes to graph + graph.add(node1) + graph.add(node2) + + // Track callback invocations + const nodeRemovedCallbacks = new Set() + const graphRemovedCallbacks = new Set() + + // Set up node.onRemoved() callbacks + node1.onRemoved = () => { + nodeRemovedCallbacks.add(String(node1.id)) + } + node2.onRemoved = () => { + nodeRemovedCallbacks.add(String(node2.id)) + } + + // Set up graph.onNodeRemoved() callback + graph.onNodeRemoved = (node) => { + graphRemovedCallbacks.add(String(node.id)) + } + + // Verify nodes are in graph before clearing + expect(graph.nodes.length).toBe(2) + + // Clear the graph + graph.clear() + + // Verify both types of callbacks were called + expect(nodeRemovedCallbacks).toContain(String(node1.id)) + expect(nodeRemovedCallbacks).toContain(String(node2.id)) + expect(graphRemovedCallbacks).toContain(String(node1.id)) + expect(graphRemovedCallbacks).toContain(String(node2.id)) + + // Verify nodes were actually removed + expect(graph.nodes.length).toBe(0) + }) +}) + describe('Legacy LGraph Compatibility Layer', () => { test('can be extended via prototype', ({ expect, minimalGraph }) => { // @ts-expect-error Should always be an error.