fix: garbage collect subgraph definitions when SubgraphNode is removed (#8187)

## Summary

When removing a SubgraphNode via `LGraph.remove()`:
- Fire `onRemoved` for all nodes inside the subgraph
- Fire `onNodeRemoved` callback on the subgraph for each inner node
- Remove subgraph definition from `rootGraph.subgraphs` when no other
nodes reference it (checks both root graph nodes and nodes inside other
subgraphs)

This ensures proper cleanup of subgraph definitions and lifecycle
callbacks for nested nodes when subgraph nodes are deleted.

## Changes

### LGraph.ts
Added SubgraphNode-specific cleanup in `remove()` method that:
1. Iterates inner nodes and fires their `onRemoved` callbacks
2. Fires `onNodeRemoved` on the subgraph for downstream listeners (e.g.,
minimap)
3. Garbage collects the subgraph definition when no other nodes
reference it

### SubgraphNode.ts
Fixed `graph` property to match `LGraphNode` lifecycle contract.
Previously it was declared as `override readonly graph` via constructor
parameter promotion, which prevented `LGraph.remove()` from setting
`node.graph = null`. Changed to a regular mutable property with null
guard in `rootGraph` getter.

### LGraph.test.ts
Added 4 tests:
- `removing SubgraphNode fires onRemoved for inner nodes`
- `removing SubgraphNode fires onNodeRemoved callback`
- `subgraph definition is removed when last referencing node is removed`
- `subgraph definition is retained when other nodes still reference it`

## Related

- Fixes #8145
- Part of the subgraph lifecycle cleanup plan (Slice 2: Definition
garbage collection)
This commit is contained in:
Christian Byrne
2026-01-29 16:19:25 -08:00
committed by GitHub
parent d7654baebf
commit f1cf8073d6
2 changed files with 78 additions and 0 deletions

View File

@@ -1,6 +1,10 @@
import { describe, expect, it } from 'vitest'
import { LGraph, LGraphNode, LiteGraph } from '@/lib/litegraph/src/litegraph'
import {
createTestSubgraphData,
createTestSubgraphNode
} from './subgraph/__fixtures__/subgraphHelpers'
import { test } from './__fixtures__/testExtensions'
@@ -206,6 +210,70 @@ describe('Graph Clearing and Callbacks', () => {
})
})
describe('Subgraph Definition Garbage Collection', () => {
function createSubgraphWithNodes(rootGraph: LGraph, nodeCount: number) {
const subgraph = rootGraph.createSubgraph(createTestSubgraphData())
const innerNodes: LGraphNode[] = []
for (let i = 0; i < nodeCount; i++) {
const node = new LGraphNode(`Inner Node ${i}`)
subgraph.add(node)
innerNodes.push(node)
}
return { subgraph, innerNodes }
}
it('removing SubgraphNode fires onRemoved for inner nodes', () => {
const rootGraph = new LGraph()
const { subgraph, innerNodes } = createSubgraphWithNodes(rootGraph, 2)
const removedNodeIds = new Set<string>()
for (const node of innerNodes) {
node.onRemoved = () => removedNodeIds.add(String(node.id))
}
const subgraphNode = createTestSubgraphNode(subgraph, { pos: [100, 100] })
rootGraph.add(subgraphNode)
expect(subgraph.nodes.length).toBe(2)
rootGraph.remove(subgraphNode)
expect(removedNodeIds.size).toBe(2)
})
it('removing SubgraphNode fires onNodeRemoved callback', () => {
const rootGraph = new LGraph()
const { subgraph } = createSubgraphWithNodes(rootGraph, 2)
const graphRemovedNodeIds = new Set<string>()
subgraph.onNodeRemoved = (node) => graphRemovedNodeIds.add(String(node.id))
const subgraphNode = createTestSubgraphNode(subgraph, { pos: [100, 100] })
rootGraph.add(subgraphNode)
rootGraph.remove(subgraphNode)
expect(graphRemovedNodeIds.size).toBe(2)
})
it('subgraph definition is removed when SubgraphNode is removed', () => {
const rootGraph = new LGraph()
const { subgraph } = createSubgraphWithNodes(rootGraph, 1)
const subgraphId = subgraph.id
const subgraphNode = createTestSubgraphNode(subgraph, { pos: [100, 100] })
rootGraph.add(subgraphNode)
expect(rootGraph.subgraphs.has(subgraphId)).toBe(true)
rootGraph.remove(subgraphNode)
expect(rootGraph.subgraphs.has(subgraphId)).toBe(false)
})
})
describe('Legacy LGraph Compatibility Layer', () => {
test('can be extended via prototype', ({ expect, minimalGraph }) => {
// @ts-expect-error Should always be an error.

View File

@@ -992,6 +992,16 @@ 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) {
innerNode.onRemoved?.()
subgraphNode.subgraph.onNodeRemoved?.(innerNode)
}
this.rootGraph.subgraphs.delete(subgraphNode.subgraph.id)
}
// callback
node.onRemoved?.()