Skip to content

Commit 3818b7c

Browse files
comfy-pr-botchristian-byrneclaude
authored
Fix widget disconnection issue in subgraphs #4922 (#5015) (#5019)
* [bugfix] Fix widget disconnection issue in subgraphs When disconnecting a node from a SubgraphInput, the target input's link reference was not being cleared in LLink.disconnect(). This caused widgets to remain greyed out because they still thought they were connected (slot.link was not null). The fix ensures that when a link is disconnected, the target node's input slot is properly cleaned up by setting input.link = null. Fixes #4922 🤖 Generated with [Claude Code](https://claude.ai/code) * [test] Add tests for LLink disconnect fix for widget issue Add comprehensive tests for the LLink.disconnect() method to verify that target input link references are properly cleared when disconnecting. This prevents widgets from remaining greyed out after disconnection. Tests cover: - Basic disconnect functionality with link reference cleanup - Edge cases with invalid target nodes - Preventing interference between different connections Related to #4922 🤖 Generated with [Claude Code](https://claude.ai/code) --------- Co-authored-by: Christian Byrne <[email protected]> Co-authored-by: Claude <[email protected]>
1 parent aaaa8c8 commit 3818b7c

File tree

2 files changed

+94
-2
lines changed

2 files changed

+94
-2
lines changed

src/lib/litegraph/src/LLink.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -414,6 +414,18 @@ export class LLink implements LinkSegment, Serialisable<SerialisableLLink> {
414414
* If `input` or `output`, reroutes will not be automatically removed, and retain a connection to the input or output, respectively.
415415
*/
416416
disconnect(network: LinkNetwork, keepReroutes?: 'input' | 'output'): void {
417+
// Clean up the target node's input slot
418+
if (this.target_id !== -1) {
419+
const targetNode = network.getNodeById(this.target_id)
420+
if (targetNode) {
421+
const targetInput = targetNode.inputs?.[this.target_slot]
422+
if (targetInput && targetInput.link === this.id) {
423+
targetInput.link = null
424+
targetNode.setDirtyCanvas?.(true, false)
425+
}
426+
}
427+
}
428+
417429
const reroutes = LLink.getReroutes(network, this)
418430

419431
const lastReroute = reroutes.at(-1)

src/lib/litegraph/test/LLink.test.ts

Lines changed: 82 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
1-
import { describe, expect } from 'vitest'
1+
import { describe, expect, it, vi } from 'vitest'
22

3-
import { LLink } from '@/lib/litegraph/src/litegraph'
3+
import { LGraph, LGraphNode, LLink } from '@/lib/litegraph/src/litegraph'
44

55
import { test } from './testExtensions'
66

@@ -14,4 +14,84 @@ describe('LLink', () => {
1414
const link = new LLink(1, 'float', 4, 2, 5, 3)
1515
expect(link.serialize()).toMatchSnapshot('Basic')
1616
})
17+
18+
describe('disconnect', () => {
19+
it('should clear the target input link reference when disconnecting', () => {
20+
// Create a graph and nodes
21+
const graph = new LGraph()
22+
const sourceNode = new LGraphNode('Source')
23+
const targetNode = new LGraphNode('Target')
24+
25+
// Add nodes to graph
26+
graph.add(sourceNode)
27+
graph.add(targetNode)
28+
29+
// Add slots
30+
sourceNode.addOutput('out', 'number')
31+
targetNode.addInput('in', 'number')
32+
33+
// Connect the nodes
34+
const link = sourceNode.connect(0, targetNode, 0)
35+
expect(link).toBeDefined()
36+
expect(targetNode.inputs[0].link).toBe(link?.id)
37+
38+
// Mock setDirtyCanvas
39+
const setDirtyCanvasSpy = vi.spyOn(targetNode, 'setDirtyCanvas')
40+
41+
// Disconnect the link
42+
link?.disconnect(graph)
43+
44+
// Verify the target input's link reference is cleared
45+
expect(targetNode.inputs[0].link).toBeNull()
46+
47+
// Verify setDirtyCanvas was called
48+
expect(setDirtyCanvasSpy).toHaveBeenCalledWith(true, false)
49+
})
50+
51+
it('should handle disconnecting when target node is not found', () => {
52+
// Create a link with invalid target
53+
const graph = new LGraph()
54+
const link = new LLink(1, 'number', 1, 0, 999, 0) // Invalid target id
55+
56+
// Should not throw when disconnecting
57+
expect(() => link.disconnect(graph)).not.toThrow()
58+
})
59+
60+
it('should only clear link reference if it matches the current link id', () => {
61+
// Create a graph and nodes
62+
const graph = new LGraph()
63+
const sourceNode1 = new LGraphNode('Source1')
64+
const sourceNode2 = new LGraphNode('Source2')
65+
const targetNode = new LGraphNode('Target')
66+
67+
// Add nodes to graph
68+
graph.add(sourceNode1)
69+
graph.add(sourceNode2)
70+
graph.add(targetNode)
71+
72+
// Add slots
73+
sourceNode1.addOutput('out', 'number')
74+
sourceNode2.addOutput('out', 'number')
75+
targetNode.addInput('in', 'number')
76+
77+
// Create first connection
78+
const link1 = sourceNode1.connect(0, targetNode, 0)
79+
expect(link1).toBeDefined()
80+
81+
// Disconnect first connection
82+
targetNode.disconnectInput(0)
83+
84+
// Create second connection
85+
const link2 = sourceNode2.connect(0, targetNode, 0)
86+
expect(link2).toBeDefined()
87+
expect(targetNode.inputs[0].link).toBe(link2?.id)
88+
89+
// Try to disconnect the first link (which is already disconnected)
90+
// It should not affect the current connection
91+
link1?.disconnect(graph)
92+
93+
// The input should still have the second link
94+
expect(targetNode.inputs[0].link).toBe(link2?.id)
95+
})
96+
})
1797
})

0 commit comments

Comments
 (0)