From 5897e0079349942c7d3c9d8c19c37fe7f1c041ea Mon Sep 17 00:00:00 2001 From: AustinMroz Date: Fri, 24 Oct 2025 13:37:14 -0700 Subject: [PATCH] Fix disconnection of subgraphInput links (#6258) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `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) --- src/lib/litegraph/src/LLink.ts | 12 --- .../src/canvas/ToInputFromIoNodeLink.ts | 4 +- tests-ui/tests/litegraph/core/LLink.test.ts | 84 +------------------ .../litegraph/subgraph/SubgraphIO.test.ts | 37 ++++++-- 4 files changed, 37 insertions(+), 100 deletions(-) diff --git a/src/lib/litegraph/src/LLink.ts b/src/lib/litegraph/src/LLink.ts index 9c7662cb8..2403af36b 100644 --- a/src/lib/litegraph/src/LLink.ts +++ b/src/lib/litegraph/src/LLink.ts @@ -418,18 +418,6 @@ export class LLink implements LinkSegment, Serialisable { * If `input` or `output`, reroutes will not be automatically removed, and retain a connection to the input or output, respectively. */ disconnect(network: LinkNetwork, keepReroutes?: 'input' | 'output'): void { - // Clean up the target node's input slot - if (this.target_id !== -1) { - const targetNode = network.getNodeById(this.target_id) - if (targetNode) { - const targetInput = targetNode.inputs?.[this.target_slot] - if (targetInput && targetInput.link === this.id) { - targetInput.link = null - targetNode.setDirtyCanvas?.(true, false) - } - } - } - const reroutes = LLink.getReroutes(network, this) const lastReroute = reroutes.at(-1) diff --git a/src/lib/litegraph/src/canvas/ToInputFromIoNodeLink.ts b/src/lib/litegraph/src/canvas/ToInputFromIoNodeLink.ts index f056ef687..e98c59d87 100644 --- a/src/lib/litegraph/src/canvas/ToInputFromIoNodeLink.ts +++ b/src/lib/litegraph/src/canvas/ToInputFromIoNodeLink.ts @@ -137,7 +137,9 @@ export class ToInputFromIoNodeLink implements RenderLink { } disconnect(): boolean { if (!this.existingLink) return false - this.existingLink.disconnect(this.network, 'input') + const { input, inputNode } = this.existingLink.resolve(this.network) + if (!inputNode || !input) return false + this.node._disconnectNodeInput(inputNode, input, this.existingLink) return true } } diff --git a/tests-ui/tests/litegraph/core/LLink.test.ts b/tests-ui/tests/litegraph/core/LLink.test.ts index feb7c98c0..a1cdfbea0 100644 --- a/tests-ui/tests/litegraph/core/LLink.test.ts +++ b/tests-ui/tests/litegraph/core/LLink.test.ts @@ -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) - }) - }) }) diff --git a/tests-ui/tests/litegraph/subgraph/SubgraphIO.test.ts b/tests-ui/tests/litegraph/subgraph/SubgraphIO.test.ts index a7ad49913..25f2ecd10 100644 --- a/tests-ui/tests/litegraph/subgraph/SubgraphIO.test.ts +++ b/tests-ui/tests/litegraph/subgraph/SubgraphIO.test.ts @@ -2,6 +2,8 @@ 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 { @@ -9,7 +11,7 @@ import { createTestSubgraphNode } from './fixtures/subgraphHelpers' -describe.skip('SubgraphIO - Input Slot Dual-Nature Behavior', () => { +describe('SubgraphIO - Input Slot Dual-Nature Behavior', () => { subgraphTest( 'input accepts external connections from parent graph', ({ subgraphWithNode }) => { @@ -77,6 +79,31 @@ describe.skip('SubgraphIO - Input Slot Dual-Nature Behavior', () => { } ) + 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 }) => { @@ -103,7 +130,7 @@ describe.skip('SubgraphIO - Input Slot Dual-Nature Behavior', () => { ) }) -describe.skip('SubgraphIO - Output Slot Dual-Nature Behavior', () => { +describe('SubgraphIO - Output Slot Dual-Nature Behavior', () => { subgraphTest( 'output provides connections to parent graph', ({ subgraphWithNode }) => { @@ -199,7 +226,7 @@ describe.skip('SubgraphIO - Output Slot Dual-Nature Behavior', () => { ) }) -describe.skip('SubgraphIO - Boundary Connection Management', () => { +describe('SubgraphIO - Boundary Connection Management', () => { subgraphTest( 'verifies cross-boundary link resolution', ({ complexSubgraph }) => { @@ -309,7 +336,7 @@ describe.skip('SubgraphIO - Boundary Connection Management', () => { ) }) -describe.skip('SubgraphIO - Advanced Scenarios', () => { +describe('SubgraphIO - Advanced Scenarios', () => { it('handles multiple inputs and outputs with complex connections', () => { const subgraph = createTestSubgraph({ name: 'Complex IO Test', @@ -399,7 +426,7 @@ describe.skip('SubgraphIO - Advanced Scenarios', () => { }) }) -describe.skip('SubgraphIO - Empty Slot Connection', () => { +describe('SubgraphIO - Empty Slot Connection', () => { subgraphTest( 'creates new input and connects when dragging from empty slot inside subgraph', ({ subgraphWithNode }) => {