From 6ae4797749ce1cde216a57f142a1d3480e4982b3 Mon Sep 17 00:00:00 2001 From: bymyself Date: Tue, 20 Jan 2026 14:15:53 -0800 Subject: [PATCH] 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 --- src/core/graph/subgraph/proxyWidget.test.ts | 2 +- src/lib/litegraph/src/subgraph/ExecutableNodeDTO.ts | 1 + .../litegraph/src/subgraph/SubgraphConversion.test.ts | 10 +++++----- src/lib/litegraph/src/subgraph/SubgraphNode.test.ts | 2 +- src/lib/litegraph/src/subgraph/SubgraphNode.ts | 8 ++++++-- src/stores/imagePreviewStore.ts | 7 ++++--- src/stores/subgraphStore.test.ts | 2 +- 7 files changed, 19 insertions(+), 13 deletions(-) diff --git a/src/core/graph/subgraph/proxyWidget.test.ts b/src/core/graph/subgraph/proxyWidget.test.ts index 5b961ad80..956e4b9b4 100644 --- a/src/core/graph/subgraph/proxyWidget.test.ts +++ b/src/core/graph/subgraph/proxyWidget.test.ts @@ -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++) { diff --git a/src/lib/litegraph/src/subgraph/ExecutableNodeDTO.ts b/src/lib/litegraph/src/subgraph/ExecutableNodeDTO.ts index 9e9454a81..4a7ac929f 100644 --- a/src/lib/litegraph/src/subgraph/ExecutableNodeDTO.ts +++ b/src/lib/litegraph/src/subgraph/ExecutableNodeDTO.ts @@ -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( diff --git a/src/lib/litegraph/src/subgraph/SubgraphConversion.test.ts b/src/lib/litegraph/src/subgraph/SubgraphConversion.test.ts index 4e5cbae35..cfa540a34 100644 --- a/src/lib/litegraph/src/subgraph/SubgraphConversion.test.ts +++ b/src/lib/litegraph/src/subgraph/SubgraphConversion.test.ts @@ -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']) diff --git a/src/lib/litegraph/src/subgraph/SubgraphNode.test.ts b/src/lib/litegraph/src/subgraph/SubgraphNode.test.ts index 96e2e5dd3..d5ad55572 100644 --- a/src/lib/litegraph/src/subgraph/SubgraphNode.test.ts +++ b/src/lib/litegraph/src/subgraph/SubgraphNode.test.ts @@ -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) }) diff --git a/src/lib/litegraph/src/subgraph/SubgraphNode.ts b/src/lib/litegraph/src/subgraph/SubgraphNode.ts index e26e1cd48..eb8ea8c85 100644 --- a/src/lib/litegraph/src/subgraph/SubgraphNode.ts +++ b/src/lib/litegraph/src/subgraph/SubgraphNode.ts @@ -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( diff --git a/src/stores/imagePreviewStore.ts b/src/stores/imagePreviewStore.ts index db2c9abaa..1b14f2d13 100644 --- a/src/stores/imagePreviewStore.ts +++ b/src/stores/imagePreviewStore.ts @@ -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) diff --git a/src/stores/subgraphStore.test.ts b/src/stores/subgraphStore.test.ts index 68f528ae4..bc2229d19 100644 --- a/src/stores/subgraphStore.test.ts +++ b/src/stores/subgraphStore.test.ts @@ -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(() => ({