From a6620a4ddcc0ea0ab85144141de4145baa50b1be Mon Sep 17 00:00:00 2001 From: AustinMroz Date: Mon, 9 Feb 2026 13:55:23 -0800 Subject: [PATCH] Fix edge cases in subgraph removal logic (#8758) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #8187 made removal of subgraphs cleanup the subgraph definition for the removed graph and call onRemove handlers. However, it missed some edge cases and broke subgraph conversion of selections containing subgraphs which this PR tries to address. - Deeply nested subgraphs are now also cleaned - Adding a subgraphNode to the graph also ensures nested subgraphs are added to subgraph definitions Reminder: under this change, nodes can continue to exist after their onRemoved handler has been called - It may be better to instead perform the "garbage collection" of subgraphs outside of the graph removal step to better handle edge cases like subgraph conversion where a subgraph may continue to persist after a parent subgraphNode has been removed from a graph. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-8758-Fix-edge-cases-in-subgraph-removal-logic-3026d73d36508177b34ffdd2e0a114fe) by [Unito](https://www.unito.io) --- src/lib/litegraph/src/LGraph.ts | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/src/lib/litegraph/src/LGraph.ts b/src/lib/litegraph/src/LGraph.ts index d26734664..579d4a565 100644 --- a/src/lib/litegraph/src/LGraph.ts +++ b/src/lib/litegraph/src/LGraph.ts @@ -8,6 +8,7 @@ import type { UUID } from '@/lib/litegraph/src/utils/uuid' import { createUuidv4, zeroUuid } from '@/lib/litegraph/src/utils/uuid' import { useLayoutMutations } from '@/renderer/core/layout/operations/layoutMutations' import { LayoutSource } from '@/renderer/core/layout/types' +import { forEachNode } from '@/utils/graphTraversalUtil' import type { DragAndScaleState } from './DragAndScale' import { LGraphCanvas } from './LGraphCanvas' @@ -973,6 +974,13 @@ export class LGraph this.canvasAction((c) => c.startGhostPlacement(node, opts.dragEvent)) } + if (node.isSubgraphNode?.()) { + forEachNode(node.subgraph, (innerNode) => { + if (innerNode.isSubgraphNode()) + this.subgraphs.set(innerNode.subgraph.id, innerNode.subgraph) + }) + } + // to chain actions return node } @@ -1034,14 +1042,14 @@ export class LGraph } } - // Subgraph cleanup (use local const to avoid type narrowing affecting node.graph assignment) - const subgraphNode = node.isSubgraphNode() ? node : null - if (subgraphNode) { - for (const innerNode of subgraphNode.subgraph.nodes) { + if (node.isSubgraphNode()) { + forEachNode(node.subgraph, (innerNode) => { innerNode.onRemoved?.() - subgraphNode.subgraph.onNodeRemoved?.(innerNode) - } - this.rootGraph.subgraphs.delete(subgraphNode.subgraph.id) + innerNode.graph?.onNodeRemoved?.(innerNode) + if (innerNode.isSubgraphNode()) + this.rootGraph.subgraphs.delete(innerNode.subgraph.id) + }) + this.rootGraph.subgraphs.delete(node.subgraph.id) } // callback