mirror of
https://github.com/Comfy-Org/ComfyUI_frontend.git
synced 2026-05-03 12:42:01 +00:00
Fix disconnection of subgraphInput links (#6258)
`LLink.disconnect` is intended to cleanup only the link itself. #4800 mistakenly assumed that it would perform all required steps for disconnection. Later, #5015 would partially resolve this by adding some of the missing functionality into `LLink.disconnect`, but this still left output cleanup unhandled and failed to call `node.onConnectionsChanged`. This PR instead moves the disconnection code to call the function that already has robust handling for these items and removes the no-longer-needed and potentially misleading workaround. Resolves #6247 Also un-skipped several SubgraphIO tests. They appear to function fine. I'm assuming the reasons for them being skipped have been resolved. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-6258-Fix-disconnection-of-subgraphInput-links-2966d73d36508112ad1fe602cdcf461b) by [Unito](https://www.unito.io)
This commit is contained in:
@@ -1,6 +1,6 @@
|
||||
import { describe, expect, it, vi } from 'vitest'
|
||||
import { describe, expect } from 'vitest'
|
||||
|
||||
import { LGraph, LGraphNode, LLink } from '@/lib/litegraph/src/litegraph'
|
||||
import { LLink } from '@/lib/litegraph/src/litegraph'
|
||||
|
||||
import { test } from './fixtures/testExtensions'
|
||||
|
||||
@@ -14,84 +14,4 @@ describe('LLink', () => {
|
||||
const link = new LLink(1, 'float', 4, 2, 5, 3)
|
||||
expect(link.serialize()).toMatchSnapshot('Basic')
|
||||
})
|
||||
|
||||
describe('disconnect', () => {
|
||||
it('should clear the target input link reference when disconnecting', () => {
|
||||
// Create a graph and nodes
|
||||
const graph = new LGraph()
|
||||
const sourceNode = new LGraphNode('Source')
|
||||
const targetNode = new LGraphNode('Target')
|
||||
|
||||
// Add nodes to graph
|
||||
graph.add(sourceNode)
|
||||
graph.add(targetNode)
|
||||
|
||||
// Add slots
|
||||
sourceNode.addOutput('out', 'number')
|
||||
targetNode.addInput('in', 'number')
|
||||
|
||||
// Connect the nodes
|
||||
const link = sourceNode.connect(0, targetNode, 0)
|
||||
expect(link).toBeDefined()
|
||||
expect(targetNode.inputs[0].link).toBe(link?.id)
|
||||
|
||||
// Mock setDirtyCanvas
|
||||
const setDirtyCanvasSpy = vi.spyOn(targetNode, 'setDirtyCanvas')
|
||||
|
||||
// Disconnect the link
|
||||
link?.disconnect(graph)
|
||||
|
||||
// Verify the target input's link reference is cleared
|
||||
expect(targetNode.inputs[0].link).toBeNull()
|
||||
|
||||
// Verify setDirtyCanvas was called
|
||||
expect(setDirtyCanvasSpy).toHaveBeenCalledWith(true, false)
|
||||
})
|
||||
|
||||
it('should handle disconnecting when target node is not found', () => {
|
||||
// Create a link with invalid target
|
||||
const graph = new LGraph()
|
||||
const link = new LLink(1, 'number', 1, 0, 999, 0) // Invalid target id
|
||||
|
||||
// Should not throw when disconnecting
|
||||
expect(() => link.disconnect(graph)).not.toThrow()
|
||||
})
|
||||
|
||||
it('should only clear link reference if it matches the current link id', () => {
|
||||
// Create a graph and nodes
|
||||
const graph = new LGraph()
|
||||
const sourceNode1 = new LGraphNode('Source1')
|
||||
const sourceNode2 = new LGraphNode('Source2')
|
||||
const targetNode = new LGraphNode('Target')
|
||||
|
||||
// Add nodes to graph
|
||||
graph.add(sourceNode1)
|
||||
graph.add(sourceNode2)
|
||||
graph.add(targetNode)
|
||||
|
||||
// Add slots
|
||||
sourceNode1.addOutput('out', 'number')
|
||||
sourceNode2.addOutput('out', 'number')
|
||||
targetNode.addInput('in', 'number')
|
||||
|
||||
// Create first connection
|
||||
const link1 = sourceNode1.connect(0, targetNode, 0)
|
||||
expect(link1).toBeDefined()
|
||||
|
||||
// Disconnect first connection
|
||||
targetNode.disconnectInput(0)
|
||||
|
||||
// Create second connection
|
||||
const link2 = sourceNode2.connect(0, targetNode, 0)
|
||||
expect(link2).toBeDefined()
|
||||
expect(targetNode.inputs[0].link).toBe(link2?.id)
|
||||
|
||||
// Try to disconnect the first link (which is already disconnected)
|
||||
// It should not affect the current connection
|
||||
link1?.disconnect(graph)
|
||||
|
||||
// The input should still have the second link
|
||||
expect(targetNode.inputs[0].link).toBe(link2?.id)
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
Reference in New Issue
Block a user