Skip to content

Commit 5224c63

Browse files
[fix] Prevent incompatible connections to SubgraphInputNode occupied slots (#4984)
## Summary This PR fixes #4681 by building upon the foundation laid in PR #1182 (litegraph.js). It prevents incompatible type connections when dragging from a normal node's output to a SubgraphInputNode's occupied slot. Before: https://github.com/user-attachments/assets/03def938-dccc-4b2c-b65b-745abf02a13b After: https://github.com/user-attachments/assets/7a0a2ed4-9ecd-4147-be56-d643d448d4cb ## Background PR #1182 implemented: - `isValidTarget()` method in SubgraphInput/SubgraphOutput classes for validation - Visual feedback during drag (40% opacity for invalid targets) - Validation at the slot level However, there was a missing piece: while the visual feedback correctly showed invalid targets, the actual connection would still be made when dropped. ## Changes This PR extends PR #1182 by adding the missing connection prevention: 1. **Added `canConnectToSubgraphInput()` method** to render link classes: - `MovingOutputLink` - `ToOutputRenderLink` - `FloatingRenderLink` - All methods use the existing `SubgraphInput.isValidTarget()` from PR #1182 2. **Added validation in `LinkConnector.dropOnIoNode()`**: - Checks `canConnectToSubgraphInput()` before allowing the connection - Logs a warning when rejecting invalid connections - Follows the same pattern as regular node connections 3. **Added `isSubgraphInputValidDrop()` method**: - Provides validation for hover states - Ensures consistent validation across the UI
1 parent 89c78b0 commit 5224c63

File tree

5 files changed

+344
-0
lines changed

5 files changed

+344
-0
lines changed

src/lib/litegraph/src/canvas/FloatingRenderLink.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,10 @@ export class FloatingRenderLink implements RenderLink {
135135
return true
136136
}
137137

138+
canConnectToSubgraphInput(input: SubgraphInput): boolean {
139+
return this.toType === 'output' && input.isValidTarget(this.fromSlot)
140+
}
141+
138142
connectToInput(
139143
node: LGraphNode,
140144
input: INodeInputSlot,

src/lib/litegraph/src/canvas/LinkConnector.ts

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -681,6 +681,20 @@ export class LinkConnector {
681681
let targetSlot = input
682682

683683
for (const link of renderLinks) {
684+
// Validate the connection type before proceeding
685+
if (
686+
'canConnectToSubgraphInput' in link &&
687+
!link.canConnectToSubgraphInput(targetSlot)
688+
) {
689+
console.warn(
690+
'Invalid connection type',
691+
link.fromSlot.type,
692+
'->',
693+
targetSlot.type
694+
)
695+
continue
696+
}
697+
684698
link.connectToSubgraphInput(targetSlot, this.events)
685699

686700
// If we just connected to an EmptySubgraphInput, check if we should reuse the slot
@@ -941,6 +955,14 @@ export class LinkConnector {
941955
)
942956
}
943957

958+
isSubgraphInputValidDrop(input: SubgraphInput): boolean {
959+
return this.renderLinks.some(
960+
(link) =>
961+
'canConnectToSubgraphInput' in link &&
962+
link.canConnectToSubgraphInput(input)
963+
)
964+
}
965+
944966
/**
945967
* Checks if a reroute is a valid drop target for any of the links being connected.
946968
* @param reroute The reroute that would be dropped on.

src/lib/litegraph/src/canvas/MovingOutputLink.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,10 @@ export class MovingOutputLink extends MovingLinkBase {
5555
return reroute.origin_id !== this.outputNode.id
5656
}
5757

58+
canConnectToSubgraphInput(input: SubgraphInput): boolean {
59+
return input.isValidTarget(this.fromSlot)
60+
}
61+
5862
connectToInput(): never {
5963
throw new Error('MovingOutputLink cannot connect to an input.')
6064
}

src/lib/litegraph/src/canvas/ToOutputRenderLink.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,10 @@ export class ToOutputRenderLink implements RenderLink {
5858
return true
5959
}
6060

61+
canConnectToSubgraphInput(input: SubgraphInput): boolean {
62+
return input.isValidTarget(this.fromSlot)
63+
}
64+
6165
connectToOutput(
6266
node: LGraphNode,
6367
output: INodeOutputSlot,
Lines changed: 310 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,310 @@
1+
import { beforeEach, describe, expect, it, vi } from 'vitest'
2+
3+
import { LinkConnector } from '@/lib/litegraph/src/canvas/LinkConnector'
4+
import { MovingOutputLink } from '@/lib/litegraph/src/canvas/MovingOutputLink'
5+
import { ToOutputRenderLink } from '@/lib/litegraph/src/canvas/ToOutputRenderLink'
6+
import { LGraphNode, LLink } from '@/lib/litegraph/src/litegraph'
7+
import { NodeInputSlot } from '@/lib/litegraph/src/node/NodeInputSlot'
8+
9+
import { createTestSubgraph } from '../subgraph/fixtures/subgraphHelpers'
10+
11+
describe('LinkConnector SubgraphInput connection validation', () => {
12+
let connector: LinkConnector
13+
const mockSetConnectingLinks = vi.fn()
14+
15+
beforeEach(() => {
16+
connector = new LinkConnector(mockSetConnectingLinks)
17+
vi.clearAllMocks()
18+
})
19+
20+
describe('MovingOutputLink validation', () => {
21+
it('should implement canConnectToSubgraphInput method', () => {
22+
const subgraph = createTestSubgraph({
23+
inputs: [{ name: 'number_input', type: 'number' }]
24+
})
25+
26+
const sourceNode = new LGraphNode('SourceNode')
27+
sourceNode.addOutput('number_out', 'number')
28+
subgraph.add(sourceNode)
29+
30+
const targetNode = new LGraphNode('TargetNode')
31+
targetNode.addInput('number_in', 'number')
32+
subgraph.add(targetNode)
33+
34+
const link = new LLink(1, 'number', sourceNode.id, 0, targetNode.id, 0)
35+
subgraph._links.set(link.id, link)
36+
37+
const movingLink = new MovingOutputLink(subgraph, link)
38+
39+
// Verify the method exists
40+
expect(typeof movingLink.canConnectToSubgraphInput).toBe('function')
41+
})
42+
43+
it('should validate type compatibility correctly', () => {
44+
const subgraph = createTestSubgraph({
45+
inputs: [{ name: 'number_input', type: 'number' }]
46+
})
47+
48+
const sourceNode = new LGraphNode('SourceNode')
49+
sourceNode.addOutput('number_out', 'number')
50+
sourceNode.addOutput('string_out', 'string')
51+
subgraph.add(sourceNode)
52+
53+
const targetNode = new LGraphNode('TargetNode')
54+
targetNode.addInput('number_in', 'number')
55+
targetNode.addInput('string_in', 'string')
56+
subgraph.add(targetNode)
57+
58+
// Create valid link (number -> number)
59+
const validLink = new LLink(
60+
1,
61+
'number',
62+
sourceNode.id,
63+
0,
64+
targetNode.id,
65+
0
66+
)
67+
subgraph._links.set(validLink.id, validLink)
68+
const validMovingLink = new MovingOutputLink(subgraph, validLink)
69+
70+
// Create invalid link (string -> number)
71+
const invalidLink = new LLink(
72+
2,
73+
'string',
74+
sourceNode.id,
75+
1,
76+
targetNode.id,
77+
1
78+
)
79+
subgraph._links.set(invalidLink.id, invalidLink)
80+
const invalidMovingLink = new MovingOutputLink(subgraph, invalidLink)
81+
82+
const numberInput = subgraph.inputs[0]
83+
84+
// Test validation
85+
expect(validMovingLink.canConnectToSubgraphInput(numberInput)).toBe(true)
86+
expect(invalidMovingLink.canConnectToSubgraphInput(numberInput)).toBe(
87+
false
88+
)
89+
})
90+
91+
it('should handle wildcard types', () => {
92+
const subgraph = createTestSubgraph({
93+
inputs: [{ name: 'wildcard_input', type: '*' }]
94+
})
95+
96+
const sourceNode = new LGraphNode('SourceNode')
97+
sourceNode.addOutput('number_out', 'number')
98+
subgraph.add(sourceNode)
99+
100+
const targetNode = new LGraphNode('TargetNode')
101+
targetNode.addInput('number_in', 'number')
102+
subgraph.add(targetNode)
103+
104+
const link = new LLink(1, 'number', sourceNode.id, 0, targetNode.id, 0)
105+
subgraph._links.set(link.id, link)
106+
const movingLink = new MovingOutputLink(subgraph, link)
107+
108+
const wildcardInput = subgraph.inputs[0]
109+
110+
// Wildcard should accept any type
111+
expect(movingLink.canConnectToSubgraphInput(wildcardInput)).toBe(true)
112+
})
113+
})
114+
115+
describe('ToOutputRenderLink validation', () => {
116+
it('should implement canConnectToSubgraphInput method', () => {
117+
// Create a minimal valid setup
118+
const subgraph = createTestSubgraph()
119+
const node = new LGraphNode('TestNode')
120+
node.id = 1
121+
node.addInput('test_in', 'number')
122+
subgraph.add(node)
123+
124+
const slot = node.inputs[0] as NodeInputSlot
125+
const renderLink = new ToOutputRenderLink(subgraph, node, slot)
126+
127+
// Verify the method exists
128+
expect(typeof renderLink.canConnectToSubgraphInput).toBe('function')
129+
})
130+
})
131+
132+
describe('dropOnIoNode validation', () => {
133+
it('should prevent invalid connections when dropping on SubgraphInputNode', () => {
134+
const subgraph = createTestSubgraph({
135+
inputs: [{ name: 'number_input', type: 'number' }]
136+
})
137+
138+
const sourceNode = new LGraphNode('SourceNode')
139+
sourceNode.addOutput('string_out', 'string')
140+
subgraph.add(sourceNode)
141+
142+
const targetNode = new LGraphNode('TargetNode')
143+
targetNode.addInput('string_in', 'string')
144+
subgraph.add(targetNode)
145+
146+
// Create an invalid link (string output -> string input, but subgraph expects number)
147+
const link = new LLink(1, 'string', sourceNode.id, 0, targetNode.id, 0)
148+
subgraph._links.set(link.id, link)
149+
const movingLink = new MovingOutputLink(subgraph, link)
150+
151+
// Mock console.warn to verify it's called
152+
const consoleWarnSpy = vi
153+
.spyOn(console, 'warn')
154+
.mockImplementation(() => {})
155+
156+
// Add the link to the connector
157+
connector.renderLinks.push(movingLink)
158+
connector.state.connectingTo = 'output'
159+
160+
// Create mock event
161+
const mockEvent = {
162+
canvasX: 100,
163+
canvasY: 100
164+
} as any
165+
166+
// Mock the getSlotInPosition to return the subgraph input
167+
const mockGetSlotInPosition = vi.fn().mockReturnValue(subgraph.inputs[0])
168+
subgraph.inputNode.getSlotInPosition = mockGetSlotInPosition
169+
170+
// Spy on connectToSubgraphInput to ensure it's NOT called
171+
const connectSpy = vi.spyOn(movingLink, 'connectToSubgraphInput')
172+
173+
// Drop on the SubgraphInputNode
174+
connector.dropOnIoNode(subgraph.inputNode, mockEvent)
175+
176+
// Verify that the invalid connection was skipped
177+
expect(consoleWarnSpy).toHaveBeenCalledWith(
178+
'Invalid connection type',
179+
'string',
180+
'->',
181+
'number'
182+
)
183+
expect(connectSpy).not.toHaveBeenCalled()
184+
185+
consoleWarnSpy.mockRestore()
186+
})
187+
188+
it('should allow valid connections when dropping on SubgraphInputNode', () => {
189+
const subgraph = createTestSubgraph({
190+
inputs: [{ name: 'number_input', type: 'number' }]
191+
})
192+
193+
const sourceNode = new LGraphNode('SourceNode')
194+
sourceNode.addOutput('number_out', 'number')
195+
subgraph.add(sourceNode)
196+
197+
const targetNode = new LGraphNode('TargetNode')
198+
targetNode.addInput('number_in', 'number')
199+
subgraph.add(targetNode)
200+
201+
// Create a valid link (number -> number)
202+
const link = new LLink(1, 'number', sourceNode.id, 0, targetNode.id, 0)
203+
subgraph._links.set(link.id, link)
204+
const movingLink = new MovingOutputLink(subgraph, link)
205+
206+
// Add the link to the connector
207+
connector.renderLinks.push(movingLink)
208+
connector.state.connectingTo = 'output'
209+
210+
// Create mock event
211+
const mockEvent = {
212+
canvasX: 100,
213+
canvasY: 100
214+
} as any
215+
216+
// Mock the getSlotInPosition to return the subgraph input
217+
const mockGetSlotInPosition = vi.fn().mockReturnValue(subgraph.inputs[0])
218+
subgraph.inputNode.getSlotInPosition = mockGetSlotInPosition
219+
220+
// Spy on connectToSubgraphInput to ensure it IS called
221+
const connectSpy = vi.spyOn(movingLink, 'connectToSubgraphInput')
222+
223+
// Drop on the SubgraphInputNode
224+
connector.dropOnIoNode(subgraph.inputNode, mockEvent)
225+
226+
// Verify that the valid connection was made
227+
expect(connectSpy).toHaveBeenCalledWith(
228+
subgraph.inputs[0],
229+
connector.events
230+
)
231+
})
232+
})
233+
234+
describe('isSubgraphInputValidDrop', () => {
235+
it('should check if render links can connect to SubgraphInput', () => {
236+
const subgraph = createTestSubgraph({
237+
inputs: [{ name: 'number_input', type: 'number' }]
238+
})
239+
240+
const sourceNode = new LGraphNode('SourceNode')
241+
sourceNode.addOutput('number_out', 'number')
242+
sourceNode.addOutput('string_out', 'string')
243+
subgraph.add(sourceNode)
244+
245+
const targetNode = new LGraphNode('TargetNode')
246+
targetNode.addInput('number_in', 'number')
247+
targetNode.addInput('string_in', 'string')
248+
subgraph.add(targetNode)
249+
250+
// Create valid and invalid links
251+
const validLink = new LLink(
252+
1,
253+
'number',
254+
sourceNode.id,
255+
0,
256+
targetNode.id,
257+
0
258+
)
259+
const invalidLink = new LLink(
260+
2,
261+
'string',
262+
sourceNode.id,
263+
1,
264+
targetNode.id,
265+
1
266+
)
267+
subgraph._links.set(validLink.id, validLink)
268+
subgraph._links.set(invalidLink.id, invalidLink)
269+
270+
const validMovingLink = new MovingOutputLink(subgraph, validLink)
271+
const invalidMovingLink = new MovingOutputLink(subgraph, invalidLink)
272+
273+
const subgraphInput = subgraph.inputs[0]
274+
275+
// Test with only invalid link
276+
connector.renderLinks.length = 0
277+
connector.renderLinks.push(invalidMovingLink)
278+
expect(connector.isSubgraphInputValidDrop(subgraphInput)).toBe(false)
279+
280+
// Test with valid link
281+
connector.renderLinks.length = 0
282+
connector.renderLinks.push(validMovingLink)
283+
expect(connector.isSubgraphInputValidDrop(subgraphInput)).toBe(true)
284+
285+
// Test with mixed links
286+
connector.renderLinks.length = 0
287+
connector.renderLinks.push(invalidMovingLink, validMovingLink)
288+
expect(connector.isSubgraphInputValidDrop(subgraphInput)).toBe(true)
289+
})
290+
291+
it('should handle render links without canConnectToSubgraphInput method', () => {
292+
const subgraph = createTestSubgraph({
293+
inputs: [{ name: 'number_input', type: 'number' }]
294+
})
295+
296+
// Create a mock render link without the method
297+
const mockLink = {
298+
fromSlot: { type: 'number' }
299+
// No canConnectToSubgraphInput method
300+
} as any
301+
302+
connector.renderLinks.push(mockLink)
303+
304+
const subgraphInput = subgraph.inputs[0]
305+
306+
// Should return false as the link doesn't have the method
307+
expect(connector.isSubgraphInputValidDrop(subgraphInput)).toBe(false)
308+
})
309+
})
310+
})

0 commit comments

Comments
 (0)