mirror of
https://github.com/Comfy-Org/ComfyUI_frontend.git
synced 2026-04-30 11:11:53 +00:00
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:
@@ -31,7 +31,7 @@ function setupSubgraph(
|
|||||||
const subgraph = createTestSubgraph()
|
const subgraph = createTestSubgraph()
|
||||||
const subgraphNode = createTestSubgraphNode(subgraph)
|
const subgraphNode = createTestSubgraphNode(subgraph)
|
||||||
subgraphNode._internalConfigureAfterSlots()
|
subgraphNode._internalConfigureAfterSlots()
|
||||||
const graph = subgraphNode.graph
|
const graph = subgraphNode.graph!
|
||||||
graph.add(subgraphNode)
|
graph.add(subgraphNode)
|
||||||
const innerNodes = []
|
const innerNodes = []
|
||||||
for (let i = 0; i < innerNodeCount; i++) {
|
for (let i = 0; i < innerNodeCount; i++) {
|
||||||
|
|||||||
@@ -194,6 +194,7 @@ export class ExecutableNodeDTO implements ExecutableLGraphNode {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if (!subgraphNode.graph) throw new NullGraphError()
|
||||||
const outerLink = subgraphNode.graph.getLink(linkId)
|
const outerLink = subgraphNode.graph.getLink(linkId)
|
||||||
if (!outerLink)
|
if (!outerLink)
|
||||||
throw new InvalidLinkError(
|
throw new InvalidLinkError(
|
||||||
|
|||||||
@@ -45,7 +45,7 @@ describe.skip('SubgraphConversion', () => {
|
|||||||
it('Should keep interior nodes and links', () => {
|
it('Should keep interior nodes and links', () => {
|
||||||
const subgraph = createTestSubgraph()
|
const subgraph = createTestSubgraph()
|
||||||
const subgraphNode = createTestSubgraphNode(subgraph)
|
const subgraphNode = createTestSubgraphNode(subgraph)
|
||||||
const graph = subgraphNode.graph
|
const graph = subgraphNode.graph!
|
||||||
graph.add(subgraphNode)
|
graph.add(subgraphNode)
|
||||||
|
|
||||||
const node1 = createNode(subgraph, [], ['number'])
|
const node1 = createNode(subgraph, [], ['number'])
|
||||||
@@ -63,7 +63,7 @@ describe.skip('SubgraphConversion', () => {
|
|||||||
outputs: [{ name: 'value', type: 'number' }]
|
outputs: [{ name: 'value', type: 'number' }]
|
||||||
})
|
})
|
||||||
const subgraphNode = createTestSubgraphNode(subgraph)
|
const subgraphNode = createTestSubgraphNode(subgraph)
|
||||||
const graph = subgraphNode.graph
|
const graph = subgraphNode.graph!
|
||||||
graph.add(subgraphNode)
|
graph.add(subgraphNode)
|
||||||
|
|
||||||
const innerNode1 = createNode(subgraph, [], ['number'])
|
const innerNode1 = createNode(subgraph, [], ['number'])
|
||||||
@@ -86,7 +86,7 @@ describe.skip('SubgraphConversion', () => {
|
|||||||
outputs: [{ name: 'value', type: 'number' }]
|
outputs: [{ name: 'value', type: 'number' }]
|
||||||
})
|
})
|
||||||
const subgraphNode = createTestSubgraphNode(subgraph)
|
const subgraphNode = createTestSubgraphNode(subgraph)
|
||||||
const graph = subgraphNode.graph
|
const graph = subgraphNode.graph!
|
||||||
graph.add(subgraphNode)
|
graph.add(subgraphNode)
|
||||||
|
|
||||||
const inner = createNode(subgraph, [], ['number'])
|
const inner = createNode(subgraph, [], ['number'])
|
||||||
@@ -117,7 +117,7 @@ describe.skip('SubgraphConversion', () => {
|
|||||||
]
|
]
|
||||||
})
|
})
|
||||||
const subgraphNode = createTestSubgraphNode(subgraph)
|
const subgraphNode = createTestSubgraphNode(subgraph)
|
||||||
const graph = subgraphNode.graph
|
const graph = subgraphNode.graph!
|
||||||
graph.add(subgraphNode)
|
graph.add(subgraphNode)
|
||||||
|
|
||||||
const inner = createNode(subgraph, [], ['number', 'number'])
|
const inner = createNode(subgraph, [], ['number', 'number'])
|
||||||
@@ -159,7 +159,7 @@ describe.skip('SubgraphConversion', () => {
|
|||||||
]
|
]
|
||||||
})
|
})
|
||||||
const subgraphNode = createTestSubgraphNode(subgraph)
|
const subgraphNode = createTestSubgraphNode(subgraph)
|
||||||
const graph = subgraphNode.graph
|
const graph = subgraphNode.graph!
|
||||||
graph.add(subgraphNode)
|
graph.add(subgraphNode)
|
||||||
|
|
||||||
const inner1 = createNode(subgraph, ['number', 'number'])
|
const inner1 = createNode(subgraph, ['number', 'number'])
|
||||||
|
|||||||
@@ -54,7 +54,7 @@ describe.skip('SubgraphNode Construction', () => {
|
|||||||
it('should maintain reference to root graph', () => {
|
it('should maintain reference to root graph', () => {
|
||||||
const subgraph = createTestSubgraph()
|
const subgraph = createTestSubgraph()
|
||||||
const subgraphNode = createTestSubgraphNode(subgraph)
|
const subgraphNode = createTestSubgraphNode(subgraph)
|
||||||
const parentGraph = subgraphNode.graph
|
const parentGraph = subgraphNode.graph!
|
||||||
|
|
||||||
expect(subgraphNode.rootGraph).toBe(parentGraph.rootGraph)
|
expect(subgraphNode.rootGraph).toBe(parentGraph.rootGraph)
|
||||||
})
|
})
|
||||||
|
|||||||
@@ -5,6 +5,7 @@ import { LGraphNode } from '@/lib/litegraph/src/LGraphNode'
|
|||||||
import type { DrawTitleBoxOptions } from '@/lib/litegraph/src/LGraphNode'
|
import type { DrawTitleBoxOptions } from '@/lib/litegraph/src/LGraphNode'
|
||||||
import { LLink } from '@/lib/litegraph/src/LLink'
|
import { LLink } from '@/lib/litegraph/src/LLink'
|
||||||
import type { ResolvedConnection } 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 { RecursionError } from '@/lib/litegraph/src/infrastructure/RecursionError'
|
||||||
import type {
|
import type {
|
||||||
ISubgraphInput,
|
ISubgraphInput,
|
||||||
@@ -47,8 +48,10 @@ export class SubgraphNode extends LGraphNode implements BaseLGraph {
|
|||||||
|
|
||||||
override readonly type: UUID
|
override readonly type: UUID
|
||||||
override readonly isVirtualNode = true as const
|
override readonly isVirtualNode = true as const
|
||||||
|
override graph: GraphOrSubgraph | null
|
||||||
|
|
||||||
get rootGraph(): LGraph {
|
get rootGraph(): LGraph {
|
||||||
|
if (!this.graph) throw new NullGraphError()
|
||||||
return this.graph.rootGraph
|
return this.graph.rootGraph
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -67,12 +70,13 @@ export class SubgraphNode extends LGraphNode implements BaseLGraph {
|
|||||||
|
|
||||||
constructor(
|
constructor(
|
||||||
/** The (sub)graph that contains this subgraph instance. */
|
/** 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. */
|
/** The definition of this subgraph; how its nodes are configured, etc. */
|
||||||
readonly subgraph: Subgraph,
|
readonly subgraph: Subgraph,
|
||||||
instanceData: ExportedSubgraphInstance
|
instanceData: ExportedSubgraphInstance
|
||||||
) {
|
) {
|
||||||
super(subgraph.name, subgraph.id)
|
super(subgraph.name, subgraph.id)
|
||||||
|
this.graph = graph
|
||||||
|
|
||||||
// Update this node when the subgraph input / output slots are changed
|
// Update this node when the subgraph input / output slots are changed
|
||||||
const subgraphEvents = this.subgraph.events
|
const subgraphEvents = this.subgraph.events
|
||||||
@@ -496,7 +500,7 @@ export class SubgraphNode extends LGraphNode implements BaseLGraph {
|
|||||||
const subgraphInstanceIdPath = [...subgraphNodePath, this.id]
|
const subgraphInstanceIdPath = [...subgraphNodePath, this.id]
|
||||||
|
|
||||||
// Store the subgraph node DTO
|
// Store the subgraph node DTO
|
||||||
const parentSubgraphNode = this.graph.rootGraph
|
const parentSubgraphNode = this.rootGraph
|
||||||
.resolveSubgraphIdPath(subgraphNodePath)
|
.resolveSubgraphIdPath(subgraphNodePath)
|
||||||
.at(-1)
|
.at(-1)
|
||||||
const subgraphNodeDto = new ExecutableNodeDTO(
|
const subgraphNodeDto = new ExecutableNodeDTO(
|
||||||
|
|||||||
@@ -298,9 +298,10 @@ export const useNodeOutputStore = defineStore('nodeOutput', () => {
|
|||||||
* Does not recurse to contents of nested subgraphs.
|
* Does not recurse to contents of nested subgraphs.
|
||||||
*/
|
*/
|
||||||
function revokeSubgraphPreviews(subgraphNode: SubgraphNode) {
|
function revokeSubgraphPreviews(subgraphNode: SubgraphNode) {
|
||||||
const graphId = subgraphNode.graph.isRootGraph
|
const { graph } = subgraphNode
|
||||||
? ''
|
if (!graph) return
|
||||||
: subgraphNode.graph.id + ':'
|
|
||||||
|
const graphId = graph.isRootGraph ? '' : graph.id + ':'
|
||||||
revokePreviewsByLocatorId(graphId + subgraphNode.id)
|
revokePreviewsByLocatorId(graphId + subgraphNode.id)
|
||||||
for (const node of subgraphNode.subgraph.nodes) {
|
for (const node of subgraphNode.subgraph.nodes) {
|
||||||
revokePreviewsByLocatorId(subgraphNode.subgraph.id + node.id)
|
revokePreviewsByLocatorId(subgraphNode.subgraph.id + node.id)
|
||||||
|
|||||||
@@ -94,7 +94,7 @@ describe('useSubgraphStore', () => {
|
|||||||
//mock canvas to provide a minimal subgraphNode
|
//mock canvas to provide a minimal subgraphNode
|
||||||
const subgraph = createTestSubgraph()
|
const subgraph = createTestSubgraph()
|
||||||
const subgraphNode = createTestSubgraphNode(subgraph)
|
const subgraphNode = createTestSubgraphNode(subgraph)
|
||||||
const graph = subgraphNode.graph
|
const graph = subgraphNode.graph!
|
||||||
graph.add(subgraphNode)
|
graph.add(subgraphNode)
|
||||||
vi.mocked(comfyApp.canvas).selectedItems = new Set([subgraphNode])
|
vi.mocked(comfyApp.canvas).selectedItems = new Set([subgraphNode])
|
||||||
vi.mocked(comfyApp.canvas)._serializeItems = vi.fn(() => {
|
vi.mocked(comfyApp.canvas)._serializeItems = vi.fn(() => {
|
||||||
|
|||||||
Reference in New Issue
Block a user