fix: make SubgraphNode.graph nullable to allow proper cleanup

- Remove readonly from SubgraphNode.graph constructor parameter
- Add override graph: GraphOrSubgraph | null as class property
- Add NullGraphError guard in rootGraph getter
- Add null guard in ExecutableNodeDTO.resolveInput
- Add null guard in imagePreviewStore.revokeSubgraphPreviews
- Update test files with non-null assertions where graph is accessed

Amp-Thread-ID: https://ampcode.com/threads/T-019bdd79-24d5-73d6-bfb8-294c4a1e9592
Co-authored-by: Amp <amp@ampcode.com>
This commit is contained in:
bymyself
2026-01-20 14:15:53 -08:00
parent 9f297208ce
commit 6ae4797749
7 changed files with 19 additions and 13 deletions

View File

@@ -31,7 +31,7 @@ function setupSubgraph(
const subgraph = createTestSubgraph()
const subgraphNode = createTestSubgraphNode(subgraph)
subgraphNode._internalConfigureAfterSlots()
const graph = subgraphNode.graph
const graph = subgraphNode.graph!
graph.add(subgraphNode)
const innerNodes = []
for (let i = 0; i < innerNodeCount; i++) {

View File

@@ -198,6 +198,7 @@ export class ExecutableNodeDTO implements ExecutableLGraphNode {
}
}
if (!subgraphNode.graph) throw new NullGraphError()
const outerLink = subgraphNode.graph.getLink(linkId)
if (!outerLink)
throw new InvalidLinkError(

View File

@@ -45,7 +45,7 @@ describe.skip('SubgraphConversion', () => {
it('Should keep interior nodes and links', () => {
const subgraph = createTestSubgraph()
const subgraphNode = createTestSubgraphNode(subgraph)
const graph = subgraphNode.graph
const graph = subgraphNode.graph!
graph.add(subgraphNode)
const node1 = createNode(subgraph, [], ['number'])
@@ -63,7 +63,7 @@ describe.skip('SubgraphConversion', () => {
outputs: [{ name: 'value', type: 'number' }]
})
const subgraphNode = createTestSubgraphNode(subgraph)
const graph = subgraphNode.graph
const graph = subgraphNode.graph!
graph.add(subgraphNode)
const innerNode1 = createNode(subgraph, [], ['number'])
@@ -86,7 +86,7 @@ describe.skip('SubgraphConversion', () => {
outputs: [{ name: 'value', type: 'number' }]
})
const subgraphNode = createTestSubgraphNode(subgraph)
const graph = subgraphNode.graph
const graph = subgraphNode.graph!
graph.add(subgraphNode)
const inner = createNode(subgraph, [], ['number'])
@@ -117,7 +117,7 @@ describe.skip('SubgraphConversion', () => {
]
})
const subgraphNode = createTestSubgraphNode(subgraph)
const graph = subgraphNode.graph
const graph = subgraphNode.graph!
graph.add(subgraphNode)
const inner = createNode(subgraph, [], ['number', 'number'])
@@ -159,7 +159,7 @@ describe.skip('SubgraphConversion', () => {
]
})
const subgraphNode = createTestSubgraphNode(subgraph)
const graph = subgraphNode.graph
const graph = subgraphNode.graph!
graph.add(subgraphNode)
const inner1 = createNode(subgraph, ['number', 'number'])

View File

@@ -53,7 +53,7 @@ describe.skip('SubgraphNode Construction', () => {
it('should maintain reference to root graph', () => {
const subgraph = createTestSubgraph()
const subgraphNode = createTestSubgraphNode(subgraph)
const parentGraph = subgraphNode.graph
const parentGraph = subgraphNode.graph!
expect(subgraphNode.rootGraph).toBe(parentGraph.rootGraph)
})

View File

@@ -5,6 +5,7 @@ import { LGraphNode } from '@/lib/litegraph/src/LGraphNode'
import type { DrawTitleBoxOptions } from '@/lib/litegraph/src/LGraphNode'
import { LLink } from '@/lib/litegraph/src/LLink'
import type { ResolvedConnection } from '@/lib/litegraph/src/LLink'
import { NullGraphError } from '@/lib/litegraph/src/infrastructure/NullGraphError'
import { RecursionError } from '@/lib/litegraph/src/infrastructure/RecursionError'
import type {
ISubgraphInput,
@@ -47,8 +48,10 @@ export class SubgraphNode extends LGraphNode implements BaseLGraph {
override readonly type: UUID
override readonly isVirtualNode = true as const
override graph: GraphOrSubgraph | null
get rootGraph(): LGraph {
if (!this.graph) throw new NullGraphError()
return this.graph.rootGraph
}
@@ -67,12 +70,13 @@ export class SubgraphNode extends LGraphNode implements BaseLGraph {
constructor(
/** The (sub)graph that contains this subgraph instance. */
override readonly graph: GraphOrSubgraph,
graph: GraphOrSubgraph,
/** The definition of this subgraph; how its nodes are configured, etc. */
readonly subgraph: Subgraph,
instanceData: ExportedSubgraphInstance
) {
super(subgraph.name, subgraph.id)
this.graph = graph
// Update this node when the subgraph input / output slots are changed
const subgraphEvents = this.subgraph.events
@@ -495,7 +499,7 @@ export class SubgraphNode extends LGraphNode implements BaseLGraph {
const subgraphInstanceIdPath = [...subgraphNodePath, this.id]
// Store the subgraph node DTO
const parentSubgraphNode = this.graph.rootGraph
const parentSubgraphNode = this.rootGraph
.resolveSubgraphIdPath(subgraphNodePath)
.at(-1)
const subgraphNodeDto = new ExecutableNodeDTO(

View File

@@ -298,9 +298,10 @@ export const useNodeOutputStore = defineStore('nodeOutput', () => {
* Does not recurse to contents of nested subgraphs.
*/
function revokeSubgraphPreviews(subgraphNode: SubgraphNode) {
const graphId = subgraphNode.graph.isRootGraph
? ''
: subgraphNode.graph.id + ':'
const { graph } = subgraphNode
if (!graph) return
const graphId = graph.isRootGraph ? '' : graph.id + ':'
revokePreviewsByLocatorId(graphId + subgraphNode.id)
for (const node of subgraphNode.subgraph.nodes) {
revokePreviewsByLocatorId(subgraphNode.subgraph.id + node.id)

View File

@@ -87,7 +87,7 @@ describe('useSubgraphStore', () => {
//mock canvas to provide a minimal subgraphNode
const subgraph = createTestSubgraph()
const subgraphNode = createTestSubgraphNode(subgraph)
const graph = subgraphNode.graph
const graph = subgraphNode.graph!
graph.add(subgraphNode)
vi.mocked(comfyApp.canvas).selectedItems = new Set([subgraphNode])
vi.mocked(comfyApp.canvas)._serializeItems = vi.fn(() => ({