mirror of
https://github.com/Comfy-Org/ComfyUI_frontend.git
synced 2026-02-22 15:54:09 +00:00
fix: skip MatchType recalculation during graph configuration (#9004)
## Summary Fixes COM-14955: "Bug: Switch node in subgraph causes link disconnection on export" ## Problem When a MatchType node (like Switch) inside a subgraph is configured/restored, `LGraphNode.configure()` calls `onConnectionsChange` for each input sequentially. The `withComfyMatchType` callback was running before all links were restored, seeing incomplete state and incorrectly computing types, which could cause link disconnection. ## Solution Add early return when `app.configuringGraph` is true to defer type recalculation until after all links are restored. This pattern is already used throughout the codebase: - `widgetInputs.ts` - `rerouteNode.ts` - `customWidgets.ts` Post-configure recomputation is handled by the existing `requestAnimationFrame` callback in `applyMatchType`. ## Changes - `src/core/graph/widgets/dynamicWidgets.ts` - Added 1 line: `if (app.configuringGraph) return` - `src/core/graph/widgets/matchTypeConfiguring.test.ts` - New test file with 3 tests ## Testing - All existing tests pass - Added 3 new tests: - `skips type recalculation when configuringGraph is true` - `performs type recalculation during normal operation` - `connects both inputs with same type` ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-9004-fix-skip-MatchType-recalculation-during-graph-configuration-30d6d73d365081339088ffd8aebba107) by [Unito](https://www.unito.io)
This commit is contained in:
@@ -277,6 +277,7 @@ function withComfyMatchType(node: LGraphNode): asserts node is MatchTypeNode {
|
||||
) {
|
||||
const input = this.inputs[slot]
|
||||
if (contype !== LiteGraph.INPUT || !this.graph || !input) return
|
||||
if (app.configuringGraph) return
|
||||
const [matchKey, matchGroup] = Object.entries(
|
||||
this.comfyDynamic.matchType
|
||||
).find(([, group]) => input.name in group) ?? ['', undefined]
|
||||
|
||||
129
src/core/graph/widgets/matchTypeConfiguring.test.ts
Normal file
129
src/core/graph/widgets/matchTypeConfiguring.test.ts
Normal file
@@ -0,0 +1,129 @@
|
||||
import { createTestingPinia } from '@pinia/testing'
|
||||
import { setActivePinia } from 'pinia'
|
||||
import { beforeEach, describe, expect, test, vi } from 'vitest'
|
||||
|
||||
import { LGraph, LGraphNode, LiteGraph } from '@/lib/litegraph/src/litegraph'
|
||||
import { transformInputSpecV1ToV2 } from '@/schemas/nodeDef/migration'
|
||||
import { app } from '@/scripts/app'
|
||||
import { useLitegraphService } from '@/services/litegraphService'
|
||||
|
||||
setActivePinia(createTestingPinia())
|
||||
|
||||
const { addNodeInput } = useLitegraphService()
|
||||
|
||||
function createMatchTypeNode(graph: LGraph) {
|
||||
const node = new LGraphNode('switch')
|
||||
;(node.constructor as { nodeData: unknown }).nodeData = {
|
||||
name: 'ComfySwitchAny',
|
||||
output_matchtypes: ['a']
|
||||
}
|
||||
node.addOutput('out', '*')
|
||||
graph.add(node)
|
||||
|
||||
addNodeInput(
|
||||
node,
|
||||
transformInputSpecV1ToV2(
|
||||
[
|
||||
'COMFY_MATCHTYPE_V3',
|
||||
{ template: { allowed_types: '*', template_id: 'a' } }
|
||||
],
|
||||
{ name: 'on_true', isOptional: false }
|
||||
)
|
||||
)
|
||||
addNodeInput(
|
||||
node,
|
||||
transformInputSpecV1ToV2(
|
||||
[
|
||||
'COMFY_MATCHTYPE_V3',
|
||||
{ template: { allowed_types: '*', template_id: 'a' } }
|
||||
],
|
||||
{ name: 'on_false', isOptional: false }
|
||||
)
|
||||
)
|
||||
|
||||
return node
|
||||
}
|
||||
|
||||
function createSourceNode(graph: LGraph, type: string) {
|
||||
const node = new LGraphNode('source')
|
||||
node.addOutput('out', type)
|
||||
graph.add(node)
|
||||
return node
|
||||
}
|
||||
|
||||
describe('MatchType during configure', () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks()
|
||||
})
|
||||
|
||||
test('skips type recalculation when configuringGraph is true', () => {
|
||||
const graph = new LGraph()
|
||||
const switchNode = createMatchTypeNode(graph)
|
||||
const source1 = createSourceNode(graph, 'IMAGE')
|
||||
const source2 = createSourceNode(graph, 'IMAGE')
|
||||
|
||||
source1.connect(0, switchNode, 0)
|
||||
source2.connect(0, switchNode, 1)
|
||||
|
||||
expect(switchNode.inputs[0].link).not.toBeNull()
|
||||
expect(switchNode.inputs[1].link).not.toBeNull()
|
||||
|
||||
const link1Id = switchNode.inputs[0].link!
|
||||
const link2Id = switchNode.inputs[1].link!
|
||||
|
||||
const outputTypeBefore = switchNode.outputs[0].type
|
||||
;(
|
||||
app as unknown as { configuringGraphLevel: number }
|
||||
).configuringGraphLevel = 1
|
||||
|
||||
try {
|
||||
const link1 = graph.links[link1Id]
|
||||
switchNode.onConnectionsChange?.(
|
||||
LiteGraph.INPUT,
|
||||
0,
|
||||
true,
|
||||
link1,
|
||||
switchNode.inputs[0]
|
||||
)
|
||||
|
||||
expect(switchNode.inputs[0].link).toBe(link1Id)
|
||||
expect(switchNode.inputs[1].link).toBe(link2Id)
|
||||
expect(graph.links[link1Id]).toBeDefined()
|
||||
expect(graph.links[link2Id]).toBeDefined()
|
||||
expect(switchNode.outputs[0].type).toBe(outputTypeBefore)
|
||||
} finally {
|
||||
;(
|
||||
app as unknown as { configuringGraphLevel: number }
|
||||
).configuringGraphLevel = 0
|
||||
}
|
||||
})
|
||||
|
||||
test('performs type recalculation during normal operation', () => {
|
||||
const graph = new LGraph()
|
||||
const switchNode = createMatchTypeNode(graph)
|
||||
const source1 = createSourceNode(graph, 'IMAGE')
|
||||
|
||||
expect(app.configuringGraph).toBe(false)
|
||||
|
||||
source1.connect(0, switchNode, 0)
|
||||
|
||||
expect(switchNode.inputs[0].link).not.toBeNull()
|
||||
expect(switchNode.outputs[0].type).toBe('IMAGE')
|
||||
})
|
||||
|
||||
test('connects both inputs with same type', () => {
|
||||
const graph = new LGraph()
|
||||
const switchNode = createMatchTypeNode(graph)
|
||||
const source1 = createSourceNode(graph, 'IMAGE')
|
||||
const source2 = createSourceNode(graph, 'IMAGE')
|
||||
|
||||
expect(app.configuringGraph).toBe(false)
|
||||
|
||||
source1.connect(0, switchNode, 0)
|
||||
source2.connect(0, switchNode, 1)
|
||||
|
||||
expect(switchNode.inputs[0].link).not.toBeNull()
|
||||
expect(switchNode.inputs[1].link).not.toBeNull()
|
||||
expect(switchNode.outputs[0].type).toBe('IMAGE')
|
||||
})
|
||||
})
|
||||
Reference in New Issue
Block a user