mirror of
https://github.com/Comfy-Org/ComfyUI_frontend.git
synced 2026-01-30 04:50:04 +00:00
5897e00793
`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)
470 lines
16 KiB
TypeScript
470 lines
16 KiB
TypeScript
// TODO: Fix these tests after migration
|
|
import { describe, expect, it } from 'vitest'
|
|
|
|
import { LGraphNode } from '@/lib/litegraph/src/litegraph'
|
|
import { ToInputFromIoNodeLink } from '@/lib/litegraph/src/canvas/ToInputFromIoNodeLink'
|
|
import { LinkDirection } from '@/lib/litegraph/src//types/globalEnums'
|
|
|
|
import { subgraphTest } from './fixtures/subgraphFixtures'
|
|
import {
|
|
createTestSubgraph,
|
|
createTestSubgraphNode
|
|
} from './fixtures/subgraphHelpers'
|
|
|
|
describe('SubgraphIO - Input Slot Dual-Nature Behavior', () => {
|
|
subgraphTest(
|
|
'input accepts external connections from parent graph',
|
|
({ subgraphWithNode }) => {
|
|
const { subgraph, subgraphNode, parentGraph } = subgraphWithNode
|
|
|
|
subgraph.addInput('test_input', 'number')
|
|
|
|
const externalNode = new LGraphNode('External Source')
|
|
externalNode.addOutput('out', 'number')
|
|
parentGraph.add(externalNode)
|
|
|
|
expect(() => {
|
|
externalNode.connect(0, subgraphNode, 0)
|
|
}).not.toThrow()
|
|
|
|
expect(
|
|
// @ts-expect-error TODO: Fix after merge - link can be null
|
|
externalNode.outputs[0].links?.includes(subgraphNode.inputs[0].link)
|
|
).toBe(true)
|
|
expect(subgraphNode.inputs[0].link).not.toBe(null)
|
|
}
|
|
)
|
|
|
|
subgraphTest(
|
|
'empty input slot creation enables dynamic IO',
|
|
({ simpleSubgraph }) => {
|
|
const initialInputCount = simpleSubgraph.inputs.length
|
|
|
|
// Create empty input slot
|
|
simpleSubgraph.addInput('', '*')
|
|
|
|
// Should create new input
|
|
expect(simpleSubgraph.inputs.length).toBe(initialInputCount + 1)
|
|
|
|
// The empty slot should be configurable
|
|
const emptyInput = simpleSubgraph.inputs.at(-1)
|
|
// @ts-expect-error TODO: Fix after merge - emptyInput possibly undefined
|
|
expect(emptyInput.name).toBe('')
|
|
// @ts-expect-error TODO: Fix after merge - emptyInput possibly undefined
|
|
expect(emptyInput.type).toBe('*')
|
|
}
|
|
)
|
|
|
|
subgraphTest(
|
|
'handles slot removal with active connections',
|
|
({ subgraphWithNode }) => {
|
|
const { subgraph, subgraphNode, parentGraph } = subgraphWithNode
|
|
|
|
const externalNode = new LGraphNode('External Source')
|
|
externalNode.addOutput('out', '*')
|
|
parentGraph.add(externalNode)
|
|
|
|
externalNode.connect(0, subgraphNode, 0)
|
|
|
|
// Verify connection exists
|
|
expect(subgraphNode.inputs[0].link).not.toBe(null)
|
|
|
|
// Remove the existing input (fixture creates one input)
|
|
const inputToRemove = subgraph.inputs[0]
|
|
subgraph.removeInput(inputToRemove)
|
|
|
|
// Connection should be cleaned up
|
|
expect(subgraphNode.inputs.length).toBe(0)
|
|
expect(externalNode.outputs[0].links).toHaveLength(0)
|
|
}
|
|
)
|
|
|
|
subgraphTest('handles link disconnection', ({ subgraphWithNode }) => {
|
|
const { subgraph } = subgraphWithNode
|
|
const internalNode = new LGraphNode('External Source')
|
|
internalNode.addInput('in', '*')
|
|
internalNode.onConnectionsChange = vi.fn()
|
|
subgraph.add(internalNode)
|
|
|
|
const link = subgraph.inputNode.slots[0].connect(
|
|
internalNode.inputs[0],
|
|
internalNode
|
|
)
|
|
new ToInputFromIoNodeLink(
|
|
subgraph,
|
|
subgraph.inputNode,
|
|
subgraph.inputNode.slots[0],
|
|
undefined,
|
|
LinkDirection.CENTER,
|
|
link
|
|
).disconnect()
|
|
|
|
expect(internalNode.inputs[0].link).toBeNull()
|
|
expect(subgraph.inputNode.slots[0].linkIds.length).toBe(0)
|
|
expect(internalNode.onConnectionsChange).toHaveBeenCalled()
|
|
})
|
|
|
|
subgraphTest(
|
|
'handles slot renaming with active connections',
|
|
({ subgraphWithNode }) => {
|
|
const { subgraph, subgraphNode, parentGraph } = subgraphWithNode
|
|
|
|
const externalNode = new LGraphNode('External Source')
|
|
externalNode.addOutput('out', '*')
|
|
parentGraph.add(externalNode)
|
|
|
|
externalNode.connect(0, subgraphNode, 0)
|
|
|
|
// Verify connection exists
|
|
expect(subgraphNode.inputs[0].link).not.toBe(null)
|
|
|
|
// Rename the existing input (fixture creates input named "input")
|
|
const inputToRename = subgraph.inputs[0]
|
|
subgraph.renameInput(inputToRename, 'new_name')
|
|
|
|
// Connection should persist and subgraph definition should be updated
|
|
expect(subgraphNode.inputs[0].link).not.toBe(null)
|
|
expect(subgraph.inputs[0].label).toBe('new_name')
|
|
expect(subgraph.inputs[0].displayName).toBe('new_name')
|
|
}
|
|
)
|
|
})
|
|
|
|
describe('SubgraphIO - Output Slot Dual-Nature Behavior', () => {
|
|
subgraphTest(
|
|
'output provides connections to parent graph',
|
|
({ subgraphWithNode }) => {
|
|
const { subgraph, subgraphNode, parentGraph } = subgraphWithNode
|
|
|
|
// Add an output to the subgraph
|
|
subgraph.addOutput('test_output', 'number')
|
|
|
|
const externalNode = new LGraphNode('External Target')
|
|
externalNode.addInput('in', 'number')
|
|
parentGraph.add(externalNode)
|
|
|
|
// External connection from subgraph output should work
|
|
expect(() => {
|
|
subgraphNode.connect(0, externalNode, 0)
|
|
}).not.toThrow()
|
|
|
|
expect(
|
|
// @ts-expect-error TODO: Fix after merge - link can be null
|
|
subgraphNode.outputs[0].links?.includes(externalNode.inputs[0].link)
|
|
).toBe(true)
|
|
expect(externalNode.inputs[0].link).not.toBe(null)
|
|
}
|
|
)
|
|
|
|
subgraphTest(
|
|
'empty output slot creation enables dynamic IO',
|
|
({ simpleSubgraph }) => {
|
|
const initialOutputCount = simpleSubgraph.outputs.length
|
|
|
|
// Create empty output slot
|
|
simpleSubgraph.addOutput('', '*')
|
|
|
|
// Should create new output
|
|
expect(simpleSubgraph.outputs.length).toBe(initialOutputCount + 1)
|
|
|
|
// The empty slot should be configurable
|
|
const emptyOutput = simpleSubgraph.outputs.at(-1)
|
|
// @ts-expect-error TODO: Fix after merge - emptyOutput possibly undefined
|
|
expect(emptyOutput.name).toBe('')
|
|
// @ts-expect-error TODO: Fix after merge - emptyOutput possibly undefined
|
|
expect(emptyOutput.type).toBe('*')
|
|
}
|
|
)
|
|
|
|
subgraphTest(
|
|
'handles slot removal with active connections',
|
|
({ subgraphWithNode }) => {
|
|
const { subgraph, subgraphNode, parentGraph } = subgraphWithNode
|
|
|
|
const externalNode = new LGraphNode('External Target')
|
|
externalNode.addInput('in', '*')
|
|
parentGraph.add(externalNode)
|
|
|
|
subgraphNode.connect(0, externalNode, 0)
|
|
|
|
// Verify connection exists
|
|
expect(externalNode.inputs[0].link).not.toBe(null)
|
|
|
|
// Remove the existing output (fixture creates one output)
|
|
const outputToRemove = subgraph.outputs[0]
|
|
subgraph.removeOutput(outputToRemove)
|
|
|
|
// Connection should be cleaned up
|
|
expect(subgraphNode.outputs.length).toBe(0)
|
|
expect(externalNode.inputs[0].link).toBe(null)
|
|
}
|
|
)
|
|
|
|
subgraphTest(
|
|
'handles slot renaming updates all references',
|
|
({ subgraphWithNode }) => {
|
|
const { subgraph, subgraphNode, parentGraph } = subgraphWithNode
|
|
|
|
const externalNode = new LGraphNode('External Target')
|
|
externalNode.addInput('in', '*')
|
|
parentGraph.add(externalNode)
|
|
|
|
subgraphNode.connect(0, externalNode, 0)
|
|
|
|
// Verify connection exists
|
|
expect(externalNode.inputs[0].link).not.toBe(null)
|
|
|
|
// Rename the existing output (fixture creates output named "output")
|
|
const outputToRename = subgraph.outputs[0]
|
|
subgraph.renameOutput(outputToRename, 'new_name')
|
|
|
|
// Connection should persist and subgraph definition should be updated
|
|
expect(externalNode.inputs[0].link).not.toBe(null)
|
|
expect(subgraph.outputs[0].label).toBe('new_name')
|
|
expect(subgraph.outputs[0].displayName).toBe('new_name')
|
|
}
|
|
)
|
|
})
|
|
|
|
describe('SubgraphIO - Boundary Connection Management', () => {
|
|
subgraphTest(
|
|
'verifies cross-boundary link resolution',
|
|
({ complexSubgraph }) => {
|
|
const subgraphNode = createTestSubgraphNode(complexSubgraph)
|
|
const parentGraph = subgraphNode.graph!
|
|
|
|
const externalSource = new LGraphNode('External Source')
|
|
externalSource.addOutput('out', 'number')
|
|
parentGraph.add(externalSource)
|
|
|
|
const externalTarget = new LGraphNode('External Target')
|
|
externalTarget.addInput('in', 'number')
|
|
parentGraph.add(externalTarget)
|
|
|
|
externalSource.connect(0, subgraphNode, 0)
|
|
subgraphNode.connect(0, externalTarget, 0)
|
|
|
|
expect(subgraphNode.inputs[0].link).not.toBe(null)
|
|
expect(externalTarget.inputs[0].link).not.toBe(null)
|
|
}
|
|
)
|
|
|
|
subgraphTest(
|
|
'handles bypass nodes that pass through data',
|
|
({ simpleSubgraph }) => {
|
|
const subgraphNode = createTestSubgraphNode(simpleSubgraph)
|
|
const parentGraph = subgraphNode.graph!
|
|
|
|
const externalSource = new LGraphNode('External Source')
|
|
externalSource.addOutput('out', 'number')
|
|
parentGraph.add(externalSource)
|
|
|
|
const externalTarget = new LGraphNode('External Target')
|
|
externalTarget.addInput('in', 'number')
|
|
parentGraph.add(externalTarget)
|
|
|
|
externalSource.connect(0, subgraphNode, 0)
|
|
subgraphNode.connect(0, externalTarget, 0)
|
|
|
|
expect(subgraphNode.inputs[0].link).not.toBe(null)
|
|
expect(externalTarget.inputs[0].link).not.toBe(null)
|
|
}
|
|
)
|
|
|
|
subgraphTest(
|
|
'tests link integrity across subgraph boundaries',
|
|
({ subgraphWithNode }) => {
|
|
const { subgraphNode, parentGraph } = subgraphWithNode
|
|
|
|
const externalSource = new LGraphNode('External Source')
|
|
externalSource.addOutput('out', '*')
|
|
parentGraph.add(externalSource)
|
|
|
|
const externalTarget = new LGraphNode('External Target')
|
|
externalTarget.addInput('in', '*')
|
|
parentGraph.add(externalTarget)
|
|
|
|
externalSource.connect(0, subgraphNode, 0)
|
|
subgraphNode.connect(0, externalTarget, 0)
|
|
|
|
const inputBoundaryLink = subgraphNode.inputs[0].link
|
|
const outputBoundaryLink = externalTarget.inputs[0].link
|
|
|
|
expect(inputBoundaryLink).toBeTruthy()
|
|
expect(outputBoundaryLink).toBeTruthy()
|
|
|
|
// Links should exist in parent graph
|
|
expect(inputBoundaryLink).toBeTruthy()
|
|
expect(outputBoundaryLink).toBeTruthy()
|
|
}
|
|
)
|
|
|
|
subgraphTest(
|
|
'verifies proper link cleanup on slot removal',
|
|
({ complexSubgraph }) => {
|
|
const subgraphNode = createTestSubgraphNode(complexSubgraph)
|
|
const parentGraph = subgraphNode.graph!
|
|
|
|
const externalSource = new LGraphNode('External Source')
|
|
externalSource.addOutput('out', 'number')
|
|
parentGraph.add(externalSource)
|
|
|
|
const externalTarget = new LGraphNode('External Target')
|
|
externalTarget.addInput('in', 'number')
|
|
parentGraph.add(externalTarget)
|
|
|
|
externalSource.connect(0, subgraphNode, 0)
|
|
subgraphNode.connect(0, externalTarget, 0)
|
|
|
|
expect(subgraphNode.inputs[0].link).not.toBe(null)
|
|
expect(externalTarget.inputs[0].link).not.toBe(null)
|
|
|
|
const inputToRemove = complexSubgraph.inputs[0]
|
|
complexSubgraph.removeInput(inputToRemove)
|
|
|
|
expect(subgraphNode.inputs.findIndex((i) => i.name === 'data')).toBe(-1)
|
|
expect(externalSource.outputs[0].links).toHaveLength(0)
|
|
|
|
const outputToRemove = complexSubgraph.outputs[0]
|
|
complexSubgraph.removeOutput(outputToRemove)
|
|
|
|
expect(subgraphNode.outputs.findIndex((o) => o.name === 'result')).toBe(
|
|
-1
|
|
)
|
|
expect(externalTarget.inputs[0].link).toBe(null)
|
|
}
|
|
)
|
|
})
|
|
|
|
describe('SubgraphIO - Advanced Scenarios', () => {
|
|
it('handles multiple inputs and outputs with complex connections', () => {
|
|
const subgraph = createTestSubgraph({
|
|
name: 'Complex IO Test',
|
|
inputs: [
|
|
{ name: 'input1', type: 'number' },
|
|
{ name: 'input2', type: 'string' },
|
|
{ name: 'input3', type: 'boolean' }
|
|
],
|
|
outputs: [
|
|
{ name: 'output1', type: 'number' },
|
|
{ name: 'output2', type: 'string' }
|
|
]
|
|
})
|
|
|
|
const subgraphNode = createTestSubgraphNode(subgraph)
|
|
|
|
// Should have correct number of slots
|
|
expect(subgraphNode.inputs.length).toBe(3)
|
|
expect(subgraphNode.outputs.length).toBe(2)
|
|
|
|
// Each slot should have correct type
|
|
expect(subgraphNode.inputs[0].type).toBe('number')
|
|
expect(subgraphNode.inputs[1].type).toBe('string')
|
|
expect(subgraphNode.inputs[2].type).toBe('boolean')
|
|
expect(subgraphNode.outputs[0].type).toBe('number')
|
|
expect(subgraphNode.outputs[1].type).toBe('string')
|
|
})
|
|
|
|
it('handles dynamic slot creation and removal', () => {
|
|
const subgraph = createTestSubgraph({
|
|
name: 'Dynamic IO Test'
|
|
})
|
|
|
|
const subgraphNode = createTestSubgraphNode(subgraph)
|
|
|
|
// Start with no slots
|
|
expect(subgraphNode.inputs.length).toBe(0)
|
|
expect(subgraphNode.outputs.length).toBe(0)
|
|
|
|
// Add slots dynamically
|
|
subgraph.addInput('dynamic_input', 'number')
|
|
subgraph.addOutput('dynamic_output', 'string')
|
|
|
|
// SubgraphNode should automatically update
|
|
expect(subgraphNode.inputs.length).toBe(1)
|
|
expect(subgraphNode.outputs.length).toBe(1)
|
|
expect(subgraphNode.inputs[0].name).toBe('dynamic_input')
|
|
expect(subgraphNode.outputs[0].name).toBe('dynamic_output')
|
|
|
|
// Remove slots
|
|
subgraph.removeInput(subgraph.inputs[0])
|
|
subgraph.removeOutput(subgraph.outputs[0])
|
|
|
|
// SubgraphNode should automatically update
|
|
expect(subgraphNode.inputs.length).toBe(0)
|
|
expect(subgraphNode.outputs.length).toBe(0)
|
|
})
|
|
|
|
it('maintains slot synchronization across multiple instances', () => {
|
|
const subgraph = createTestSubgraph({
|
|
name: 'Multi-Instance Test',
|
|
inputs: [{ name: 'shared_input', type: 'number' }],
|
|
outputs: [{ name: 'shared_output', type: 'number' }]
|
|
})
|
|
|
|
// Create multiple instances
|
|
const instance1 = createTestSubgraphNode(subgraph)
|
|
const instance2 = createTestSubgraphNode(subgraph)
|
|
const instance3 = createTestSubgraphNode(subgraph)
|
|
|
|
// All instances should have same slots
|
|
expect(instance1.inputs.length).toBe(1)
|
|
expect(instance2.inputs.length).toBe(1)
|
|
expect(instance3.inputs.length).toBe(1)
|
|
|
|
// Modify the subgraph definition
|
|
subgraph.addInput('new_input', 'string')
|
|
subgraph.addOutput('new_output', 'boolean')
|
|
|
|
// All instances should automatically update
|
|
expect(instance1.inputs.length).toBe(2)
|
|
expect(instance2.inputs.length).toBe(2)
|
|
expect(instance3.inputs.length).toBe(2)
|
|
expect(instance1.outputs.length).toBe(2)
|
|
expect(instance2.outputs.length).toBe(2)
|
|
expect(instance3.outputs.length).toBe(2)
|
|
})
|
|
})
|
|
|
|
describe('SubgraphIO - Empty Slot Connection', () => {
|
|
subgraphTest(
|
|
'creates new input and connects when dragging from empty slot inside subgraph',
|
|
({ subgraphWithNode }) => {
|
|
const { subgraph, subgraphNode } = subgraphWithNode
|
|
|
|
// Create a node inside the subgraph that will receive the connection
|
|
const internalNode = new LGraphNode('Internal Node')
|
|
internalNode.addInput('in', 'string')
|
|
subgraph.add(internalNode)
|
|
|
|
// Simulate the connection process from the empty slot to an internal node
|
|
// The -1 indicates a connection from the "empty" slot
|
|
subgraph.inputNode.connectByType(-1, internalNode, 'string')
|
|
|
|
// 1. A new input should have been created on the subgraph
|
|
expect(subgraph.inputs.length).toBe(2) // Fixture adds one input already
|
|
const newInput = subgraph.inputs[1]
|
|
expect(newInput.name).toBe('in')
|
|
expect(newInput.type).toBe('string')
|
|
|
|
// 2. The subgraph node should now have a corresponding real input slot
|
|
expect(subgraphNode.inputs.length).toBe(2)
|
|
const subgraphInputSlot = subgraphNode.inputs[1]
|
|
expect(subgraphInputSlot.name).toBe('in')
|
|
|
|
// 3. A link should be established inside the subgraph
|
|
expect(internalNode.inputs[0].link).not.toBe(null)
|
|
const link = subgraph.links.get(internalNode.inputs[0].link!)
|
|
expect(link).toBeDefined()
|
|
// @ts-expect-error TODO: Fix after merge - link possibly undefined
|
|
expect(link.target_id).toBe(internalNode.id)
|
|
// @ts-expect-error TODO: Fix after merge - link possibly undefined
|
|
expect(link.target_slot).toBe(0)
|
|
// @ts-expect-error TODO: Fix after merge - link possibly undefined
|
|
expect(link.origin_id).toBe(subgraph.inputNode.id)
|
|
// @ts-expect-error TODO: Fix after merge - link possibly undefined
|
|
expect(link.origin_slot).toBe(1) // Should be the second slot
|
|
}
|
|
)
|
|
})
|