mirror of
https://github.com/Comfy-Org/ComfyUI_frontend.git
synced 2026-07-11 09:42:22 +00:00
refactor: migrate remaining first-party readers off deprecated slot mirrors
The deprecation getters fired for every user on the first rendered frame, making extension telemetry meaningless. drawConnections now does one pass over graph links instead of per-node per-input getter scans; the subgraph resolvers take the input from link.resolve() they already call; findInputByType and unpackSubgraph use the slotLinks helpers. Adds the missing coverage: getter read/write behavior for both mirrors, the slotLinks input trio, and a connect-draw-serialize cycle asserting no deprecation warning fires.
This commit is contained in:
@@ -66,11 +66,10 @@ function resolvePromotionSource(
|
||||
const link = subgraphNode.subgraph.getLink(linkId)
|
||||
if (!link) continue
|
||||
|
||||
const { inputNode } = link.resolve(subgraphNode.subgraph)
|
||||
if (!inputNode || !Array.isArray(inputNode.inputs)) continue
|
||||
|
||||
const targetInput = inputNode.inputs.find((entry) => entry.link === linkId)
|
||||
if (!targetInput) continue
|
||||
const { inputNode, input: targetInput } = link.resolve(
|
||||
subgraphNode.subgraph
|
||||
)
|
||||
if (!inputNode || !targetInput) continue
|
||||
|
||||
if (inputNode.isSubgraphNode()) {
|
||||
return {
|
||||
|
||||
@@ -109,7 +109,7 @@ describe('resolveConcretePromotedWidget', () => {
|
||||
subgraph: {
|
||||
inputNode: { slots: [{ name: 'x', linkIds: [1] }] },
|
||||
getLink: () => ({
|
||||
resolve: () => ({ inputNode: recursiveNode })
|
||||
resolve: () => ({ inputNode: recursiveNode, input: recursiveInput })
|
||||
}),
|
||||
getNodeById: () => recursiveNode
|
||||
}
|
||||
|
||||
@@ -25,12 +25,8 @@ export function resolveSubgraphInputLink<TResult>(
|
||||
const link = node.subgraph.getLink(linkId)
|
||||
if (!link) continue
|
||||
|
||||
const { inputNode } = link.resolve(node.subgraph)
|
||||
if (!inputNode) continue
|
||||
if (!Array.isArray(inputNode.inputs)) continue
|
||||
|
||||
const targetInput = inputNode.inputs.find((entry) => entry.link === linkId)
|
||||
if (!targetInput) continue
|
||||
const { inputNode, input: targetInput } = link.resolve(node.subgraph)
|
||||
if (!inputNode || !targetInput) continue
|
||||
|
||||
let cachedTargetWidget:
|
||||
| ReturnType<LGraphNode['getWidgetFromSlot']>
|
||||
|
||||
@@ -14,7 +14,7 @@ import { toLinkId } from '@/types/linkId'
|
||||
import { toRerouteId } from '@/types/rerouteId'
|
||||
import { useLinkStore } from '@/stores/linkStore'
|
||||
import { useRerouteStore } from '@/stores/rerouteStore'
|
||||
import { inputHasLink, outputLinks } from './node/slotLinks'
|
||||
import { inputHasLink, inputLinkId, outputLinks } from './node/slotLinks'
|
||||
import { useNodeBadgeStore } from '@/stores/nodeBadgeStore'
|
||||
import { usePreviewExposureStore } from '@/stores/previewExposureStore'
|
||||
import { useWidgetValueStore } from '@/stores/widgetValueStore'
|
||||
@@ -2091,7 +2091,7 @@ export class LGraph
|
||||
for (const [, link] of subgraphNode.subgraph._links) {
|
||||
let externalParentId: RerouteId | undefined
|
||||
if (link.origin_id === SUBGRAPH_INPUT_ID) {
|
||||
const outerLinkId = subgraphNode.inputs[link.origin_slot].link
|
||||
const outerLinkId = inputLinkId(this, subgraphNode.id, link.origin_slot)
|
||||
if (!outerLinkId) {
|
||||
console.error('Missing Link ID when unpacking')
|
||||
continue
|
||||
|
||||
@@ -83,7 +83,7 @@ function createTestLink(
|
||||
return link
|
||||
}
|
||||
|
||||
describe('drawConnections widget-input slot positioning', () => {
|
||||
describe('drawConnections', () => {
|
||||
let graph: LGraph
|
||||
let canvas: LGraphCanvas
|
||||
let canvasElement: HTMLCanvasElement
|
||||
@@ -174,6 +174,31 @@ describe('drawConnections widget-input slot positioning', () => {
|
||||
expect(arrangeSpy).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('never reads the deprecated slot link mirrors in a connect-draw-serialize cycle', () => {
|
||||
const deprecationCallback = vi.fn()
|
||||
const originalCallbacks = LiteGraph.onDeprecationWarning
|
||||
LiteGraph.onDeprecationWarning = [deprecationCallback]
|
||||
LiteGraph.alwaysRepeatWarnings = true
|
||||
try {
|
||||
const sourceNode = new LGraphNode('Source')
|
||||
sourceNode.addOutput('out', 'STRING')
|
||||
graph.add(sourceNode)
|
||||
|
||||
const targetNode = new LGraphNode('Target')
|
||||
targetNode.addInput('in', 'STRING')
|
||||
graph.add(targetNode)
|
||||
|
||||
sourceNode.connect(0, targetNode, 0)
|
||||
canvas.drawConnections(createMockCtx())
|
||||
graph.asSerialisable()
|
||||
|
||||
expect(deprecationCallback).not.toHaveBeenCalled()
|
||||
} finally {
|
||||
LiteGraph.onDeprecationWarning = originalCallbacks
|
||||
LiteGraph.alwaysRepeatWarnings = false
|
||||
}
|
||||
})
|
||||
|
||||
it('positions widget-input slots when display name differs from slot.widget.name', () => {
|
||||
const sourceNode = new LGraphNode('Source')
|
||||
sourceNode.pos = [0, 100]
|
||||
|
||||
@@ -6044,48 +6044,41 @@ export class LGraphCanvas implements CustomEventDispatcher<LGraphCanvasEventMap>
|
||||
node.arrange()
|
||||
}
|
||||
|
||||
for (const node of nodes) {
|
||||
// for every input (we render just inputs because it is easier as every slot can only have one input)
|
||||
const { inputs } = node
|
||||
if (!inputs?.length) continue
|
||||
// Render every link at its target input (each input holds at most one link).
|
||||
for (const link of graph._links.values()) {
|
||||
const node = graph.getNodeById(link.target_id)
|
||||
const input = node?.inputs[link.target_slot]
|
||||
if (!node || !input) continue
|
||||
|
||||
for (const [i, input] of inputs.entries()) {
|
||||
if (!input || input.link == null) continue
|
||||
const endPos: Point = LiteGraph.vueNodesMode // TODO: still use LG get pos if vue nodes is off until stable
|
||||
? getSlotPosition(node, link.target_slot, true)
|
||||
: node.getInputPos(link.target_slot)
|
||||
|
||||
const link_id = input.link
|
||||
const link = graph._links.get(link_id)
|
||||
if (!link) continue
|
||||
// find link info
|
||||
const start_node = graph.getNodeById(link.origin_id)
|
||||
if (start_node == null) continue
|
||||
|
||||
const endPos: Point = LiteGraph.vueNodesMode // TODO: still use LG get pos if vue nodes is off until stable
|
||||
? getSlotPosition(node, i, true)
|
||||
: node.getInputPos(i)
|
||||
const outputId = link.origin_slot
|
||||
const startPos: Point =
|
||||
outputId === -1
|
||||
? [start_node.pos[0] + 10, start_node.pos[1] + 10]
|
||||
: LiteGraph.vueNodesMode // TODO: still use LG get pos if vue nodes is off until stable
|
||||
? getSlotPosition(start_node, outputId, false)
|
||||
: start_node.getOutputPos(outputId)
|
||||
|
||||
// find link info
|
||||
const start_node = graph.getNodeById(link.origin_id)
|
||||
if (start_node == null) continue
|
||||
const output = start_node.outputs[outputId]
|
||||
if (!output) continue
|
||||
|
||||
const outputId = link.origin_slot
|
||||
const startPos: Point =
|
||||
outputId === -1
|
||||
? [start_node.pos[0] + 10, start_node.pos[1] + 10]
|
||||
: LiteGraph.vueNodesMode // TODO: still use LG get pos if vue nodes is off until stable
|
||||
? getSlotPosition(start_node, outputId, false)
|
||||
: start_node.getOutputPos(outputId)
|
||||
|
||||
const output = start_node.outputs[outputId]
|
||||
if (!output) continue
|
||||
|
||||
this._renderAllLinkSegments(
|
||||
ctx,
|
||||
link,
|
||||
startPos,
|
||||
endPos,
|
||||
visibleReroutes,
|
||||
now,
|
||||
output.dir,
|
||||
input.dir
|
||||
)
|
||||
}
|
||||
this._renderAllLinkSegments(
|
||||
ctx,
|
||||
link,
|
||||
startPos,
|
||||
endPos,
|
||||
visibleReroutes,
|
||||
now,
|
||||
output.dir,
|
||||
input.dir
|
||||
)
|
||||
}
|
||||
|
||||
if (subgraph) {
|
||||
|
||||
@@ -2719,12 +2719,11 @@ export class LGraphNode
|
||||
findInputByType(
|
||||
type: ISlotType
|
||||
): { index: number; slot: INodeInputSlot } | undefined {
|
||||
return findFreeSlotOfType(
|
||||
this.inputs,
|
||||
type,
|
||||
(input) =>
|
||||
input.link == null || !!this.graph?.getLink(input.link)?._dragging
|
||||
)
|
||||
return findFreeSlotOfType(this.inputs, type, (_input, index) => {
|
||||
if (!this.graph) return true
|
||||
const link = inputLink(this.graph, this.id, index)
|
||||
return link == null || !!link._dragging
|
||||
})
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
123
src/lib/litegraph/src/node/NodeSlot.deprecation.test.ts
Normal file
123
src/lib/litegraph/src/node/NodeSlot.deprecation.test.ts
Normal file
@@ -0,0 +1,123 @@
|
||||
import { createTestingPinia } from '@pinia/testing'
|
||||
import { setActivePinia } from 'pinia'
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
|
||||
import type { LinkId } from '@/lib/litegraph/src/LLink'
|
||||
import { LGraph, LGraphNode, LiteGraph } from '@/lib/litegraph/src/litegraph'
|
||||
|
||||
function createConnectedPair() {
|
||||
const graph = new LGraph()
|
||||
const source = new LGraphNode('Source')
|
||||
source.addOutput('out', 'INT')
|
||||
const target = new LGraphNode('Target')
|
||||
target.addInput('in', 'INT')
|
||||
graph.add(source)
|
||||
graph.add(target)
|
||||
const link = source.connect(0, target, 0)!
|
||||
return { graph, source, target, link }
|
||||
}
|
||||
|
||||
describe('deprecated slot link mirrors', () => {
|
||||
const deprecationCallback = vi.fn()
|
||||
const originalCallbacks = LiteGraph.onDeprecationWarning
|
||||
|
||||
beforeEach(() => {
|
||||
setActivePinia(createTestingPinia({ stubActions: false }))
|
||||
deprecationCallback.mockClear()
|
||||
LiteGraph.onDeprecationWarning = [deprecationCallback]
|
||||
LiteGraph.alwaysRepeatWarnings = true
|
||||
})
|
||||
|
||||
afterEach(() => {
|
||||
LiteGraph.onDeprecationWarning = originalCallbacks
|
||||
LiteGraph.alwaysRepeatWarnings = false
|
||||
})
|
||||
|
||||
describe('NodeInputSlot.link', () => {
|
||||
it('returns the link id for a connected input and warns', () => {
|
||||
const { target, link } = createConnectedPair()
|
||||
|
||||
expect(target.inputs[0].link).toBe(link.id)
|
||||
expect(deprecationCallback).toHaveBeenCalledWith(
|
||||
expect.stringContaining('input.link is deprecated'),
|
||||
undefined
|
||||
)
|
||||
})
|
||||
|
||||
it('returns null for a disconnected input and warns', () => {
|
||||
const { target } = createConnectedPair()
|
||||
target.disconnectInput(0)
|
||||
|
||||
expect(target.inputs[0].link).toBeNull()
|
||||
expect(deprecationCallback).toHaveBeenCalledWith(
|
||||
expect.stringContaining('input.link is deprecated'),
|
||||
undefined
|
||||
)
|
||||
})
|
||||
|
||||
it('returns null for an input on a node detached from any graph', () => {
|
||||
const node = new LGraphNode('Detached')
|
||||
node.addInput('in', 'INT')
|
||||
|
||||
expect(node.inputs[0].link).toBeNull()
|
||||
})
|
||||
|
||||
it('ignores writes, warns, and keeps the store-derived value', () => {
|
||||
const { target, link } = createConnectedPair()
|
||||
const input: { link?: LinkId | null } = target.inputs[0]
|
||||
|
||||
expect(() => {
|
||||
input.link = null
|
||||
}).not.toThrow()
|
||||
expect(deprecationCallback).toHaveBeenCalledWith(
|
||||
expect.stringContaining('Assignment to input.link is deprecated'),
|
||||
undefined
|
||||
)
|
||||
expect(target.inputs[0].link).toBe(link.id)
|
||||
})
|
||||
})
|
||||
|
||||
describe('NodeOutputSlot.links', () => {
|
||||
it('returns the link ids for a connected output and warns', () => {
|
||||
const { source, link } = createConnectedPair()
|
||||
|
||||
expect(source.outputs[0].links).toEqual([link.id])
|
||||
expect(deprecationCallback).toHaveBeenCalledWith(
|
||||
expect.stringContaining('output.links is deprecated'),
|
||||
undefined
|
||||
)
|
||||
})
|
||||
|
||||
it('returns null for a disconnected output and warns', () => {
|
||||
const { source } = createConnectedPair()
|
||||
source.disconnectOutput(0)
|
||||
|
||||
expect(source.outputs[0].links).toBeNull()
|
||||
expect(deprecationCallback).toHaveBeenCalledWith(
|
||||
expect.stringContaining('output.links is deprecated'),
|
||||
undefined
|
||||
)
|
||||
})
|
||||
|
||||
it('returns null for an output on a node detached from any graph', () => {
|
||||
const node = new LGraphNode('Detached')
|
||||
node.addOutput('out', 'INT')
|
||||
|
||||
expect(node.outputs[0].links).toBeNull()
|
||||
})
|
||||
|
||||
it('ignores writes, warns, and keeps the store-derived value', () => {
|
||||
const { source, link } = createConnectedPair()
|
||||
const output: { links?: readonly LinkId[] | null } = source.outputs[0]
|
||||
|
||||
expect(() => {
|
||||
output.links = null
|
||||
}).not.toThrow()
|
||||
expect(deprecationCallback).toHaveBeenCalledWith(
|
||||
expect.stringContaining('Assignment to output.links is deprecated'),
|
||||
undefined
|
||||
)
|
||||
expect(source.outputs[0].links).toEqual([link.id])
|
||||
})
|
||||
})
|
||||
})
|
||||
@@ -5,7 +5,15 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import { LGraph, LGraphNode } from '@/lib/litegraph/src/litegraph'
|
||||
import { NodeSlotType } from '@/lib/litegraph/src/types/globalEnums'
|
||||
|
||||
import { outputHasLinks, outputLinkIds, outputLinks } from './slotLinks'
|
||||
import { createTestSubgraph } from '../subgraph/__fixtures__/subgraphHelpers'
|
||||
import {
|
||||
inputHasLink,
|
||||
inputLink,
|
||||
inputLinkId,
|
||||
outputHasLinks,
|
||||
outputLinkIds,
|
||||
outputLinks
|
||||
} from './slotLinks'
|
||||
|
||||
function createConnectedGraph(targetCount: number) {
|
||||
const graph = new LGraph()
|
||||
@@ -47,7 +55,7 @@ describe('slotLinks', () => {
|
||||
|
||||
it('never includes floating links', () => {
|
||||
const { graph, source, targets } = createConnectedGraph(1)
|
||||
const link = graph.getLink(targets[0].inputs[0].link!)!
|
||||
const link = inputLink(graph, targets[0].id, 0)!
|
||||
const reroute = graph.createReroute([0, 0], link)!
|
||||
graph.remove(targets[0])
|
||||
|
||||
@@ -57,6 +65,67 @@ describe('slotLinks', () => {
|
||||
expect(outputLinks(graph, source.id, 0)).toEqual([])
|
||||
})
|
||||
|
||||
it('reports presence, id, and the resolved link for an input slot', () => {
|
||||
const { graph, targets } = createConnectedGraph(1)
|
||||
const target = targets[0]
|
||||
|
||||
expect(inputHasLink(graph, target.id, 0)).toBe(true)
|
||||
const id = inputLinkId(graph, target.id, 0)
|
||||
const link = inputLink(graph, target.id, 0)
|
||||
expect(link?.id).toBe(id)
|
||||
expect(link?.target_id).toBe(target.id)
|
||||
expect(link?.target_slot).toBe(0)
|
||||
})
|
||||
|
||||
it('returns nothing for an unconnected input slot', () => {
|
||||
const { graph, targets } = createConnectedGraph(1)
|
||||
const target = targets[0]
|
||||
target.disconnectInput(0)
|
||||
|
||||
expect(inputHasLink(graph, target.id, 0)).toBe(false)
|
||||
expect(inputLinkId(graph, target.id, 0)).toBeUndefined()
|
||||
expect(inputLink(graph, target.id, 0)).toBeUndefined()
|
||||
})
|
||||
|
||||
it('never reports a floating link on the input side', () => {
|
||||
const { graph, source, targets } = createConnectedGraph(1)
|
||||
const link = inputLink(graph, targets[0].id, 0)!
|
||||
graph.createReroute([0, 0], link)
|
||||
|
||||
graph.remove(source)
|
||||
|
||||
expect(graph.floatingLinks.size).toBe(1)
|
||||
expect(inputHasLink(graph, targets[0].id, 0)).toBe(false)
|
||||
expect(inputLink(graph, targets[0].id, 0)).toBeUndefined()
|
||||
})
|
||||
|
||||
it('resolves links inside a subgraph', () => {
|
||||
const subgraph = createTestSubgraph({ nodeCount: 2 })
|
||||
const [first, second] = subgraph.nodes
|
||||
const innerLink = first.connect(0, second, 0)!
|
||||
|
||||
expect(inputHasLink(subgraph, second.id, 0)).toBe(true)
|
||||
expect(inputLinkId(subgraph, second.id, 0)).toBe(innerLink.id)
|
||||
expect(inputLink(subgraph, second.id, 0)).toBe(innerLink)
|
||||
})
|
||||
|
||||
it('reads empty during the INPUT callback of a disconnect', () => {
|
||||
const { graph, targets } = createConnectedGraph(1)
|
||||
const target = targets[0]
|
||||
const seen: boolean[] = []
|
||||
target.onConnectionsChange = vi.fn(
|
||||
(type: NodeSlotType, _slot: number, connected: boolean) => {
|
||||
if (type === NodeSlotType.INPUT && !connected) {
|
||||
seen.push(inputHasLink(graph, target.id, 0))
|
||||
}
|
||||
}
|
||||
)
|
||||
|
||||
target.disconnectInput(0)
|
||||
|
||||
expect(seen).toEqual([false])
|
||||
})
|
||||
|
||||
it('reads empty during the final OUTPUT callback of a disconnect-all', () => {
|
||||
const { graph, source } = createConnectedGraph(2)
|
||||
const seen: boolean[] = []
|
||||
|
||||
@@ -583,13 +583,12 @@ export class SubgraphNode extends LGraphNode implements BaseLGraph {
|
||||
continue
|
||||
}
|
||||
|
||||
const { inputNode } = link.resolve(this.subgraph)
|
||||
const { inputNode, input: targetInput } = link.resolve(this.subgraph)
|
||||
if (!inputNode) {
|
||||
console.warn('Failed to resolve inputNode', link, this)
|
||||
continue
|
||||
}
|
||||
|
||||
const targetInput = inputNode.inputs.find((inp) => inp.link === linkId)
|
||||
if (!targetInput) {
|
||||
console.warn('Failed to find corresponding input', link, inputNode)
|
||||
continue
|
||||
|
||||
@@ -195,7 +195,12 @@ function makeNestedPromotedModelGraph({
|
||||
},
|
||||
getLink: (id: number) =>
|
||||
id === innerLinkId
|
||||
? { resolve: () => ({ inputNode: leafNode }) }
|
||||
? {
|
||||
resolve: () => ({
|
||||
inputNode: leafNode,
|
||||
input: leafNode.inputs[0]
|
||||
})
|
||||
}
|
||||
: null,
|
||||
getNodeById: (id: string | number) =>
|
||||
String(id) === String(leafNode.id) ? leafNode : null
|
||||
@@ -233,7 +238,12 @@ function makeNestedPromotedModelGraph({
|
||||
},
|
||||
getLink: (id: number) =>
|
||||
id === outerLinkId
|
||||
? { resolve: () => ({ inputNode: innerNode }) }
|
||||
? {
|
||||
resolve: () => ({
|
||||
inputNode: innerNode,
|
||||
input: innerNode.inputs[0]
|
||||
})
|
||||
}
|
||||
: null,
|
||||
getNodeById: (id: string | number) =>
|
||||
String(id) === String(innerNode.id) ? innerNode : null
|
||||
@@ -859,7 +869,12 @@ describe('scanAllModelCandidates', () => {
|
||||
},
|
||||
getLink: (id: number) =>
|
||||
id === linkId
|
||||
? { resolve: () => ({ inputNode: interiorNode }) }
|
||||
? {
|
||||
resolve: () => ({
|
||||
inputNode: interiorNode,
|
||||
input: interiorNode.inputs[0]
|
||||
})
|
||||
}
|
||||
: null,
|
||||
getNodeById: (id: string | number) =>
|
||||
String(id) === String(interiorNode.id) ? interiorNode : null
|
||||
@@ -937,7 +952,12 @@ describe('scanAllModelCandidates', () => {
|
||||
},
|
||||
getLink: (id: number) =>
|
||||
id === linkId
|
||||
? { resolve: () => ({ inputNode: interiorNode }) }
|
||||
? {
|
||||
resolve: () => ({
|
||||
inputNode: interiorNode,
|
||||
input: interiorNode.inputs[0]
|
||||
})
|
||||
}
|
||||
: null,
|
||||
getNodeById: (id: string | number) =>
|
||||
String(id) === String(interiorNode.id) ? interiorNode : null
|
||||
@@ -1013,7 +1033,12 @@ describe('scanAllModelCandidates', () => {
|
||||
},
|
||||
getLink: (id: number) =>
|
||||
id === innerLinkId
|
||||
? { resolve: () => ({ inputNode: leafNode }) }
|
||||
? {
|
||||
resolve: () => ({
|
||||
inputNode: leafNode,
|
||||
input: leafNode.inputs[0]
|
||||
})
|
||||
}
|
||||
: null,
|
||||
getNodeById: (id: string | number) =>
|
||||
String(id) === String(leafNode.id) ? leafNode : null
|
||||
@@ -1051,7 +1076,12 @@ describe('scanAllModelCandidates', () => {
|
||||
},
|
||||
getLink: (id: number) =>
|
||||
id === outerLinkId
|
||||
? { resolve: () => ({ inputNode: innerNode }) }
|
||||
? {
|
||||
resolve: () => ({
|
||||
inputNode: innerNode,
|
||||
input: innerNode.inputs[0]
|
||||
})
|
||||
}
|
||||
: null,
|
||||
getNodeById: (id: string | number) =>
|
||||
String(id) === String(innerNode.id) ? innerNode : null
|
||||
|
||||
Reference in New Issue
Block a user