From 42fc3fa3409abfefe9ed51fc459814f4dfc1911f Mon Sep 17 00:00:00 2001 From: DrJKL Date: Tue, 7 Jul 2026 19:31:03 -0700 Subject: [PATCH] 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. --- src/core/graph/subgraph/promotionUtils.ts | 9 +- .../resolveConcretePromotedWidget.test.ts | 2 +- .../subgraph/resolveSubgraphInputLink.ts | 8 +- src/lib/litegraph/src/LGraph.ts | 4 +- .../src/LGraphCanvas.drawConnections.test.ts | 27 +++- src/lib/litegraph/src/LGraphCanvas.ts | 67 +++++----- src/lib/litegraph/src/LGraphNode.ts | 11 +- .../src/node/NodeSlot.deprecation.test.ts | 123 ++++++++++++++++++ src/lib/litegraph/src/node/slotLinks.test.ts | 73 ++++++++++- .../litegraph/src/subgraph/SubgraphNode.ts | 3 +- .../missingModel/missingModelScan.test.ts | 42 +++++- 11 files changed, 301 insertions(+), 68 deletions(-) create mode 100644 src/lib/litegraph/src/node/NodeSlot.deprecation.test.ts diff --git a/src/core/graph/subgraph/promotionUtils.ts b/src/core/graph/subgraph/promotionUtils.ts index 20c881d392..2f8b645736 100644 --- a/src/core/graph/subgraph/promotionUtils.ts +++ b/src/core/graph/subgraph/promotionUtils.ts @@ -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 { diff --git a/src/core/graph/subgraph/resolveConcretePromotedWidget.test.ts b/src/core/graph/subgraph/resolveConcretePromotedWidget.test.ts index 7fea52598f..05eca7e96c 100644 --- a/src/core/graph/subgraph/resolveConcretePromotedWidget.test.ts +++ b/src/core/graph/subgraph/resolveConcretePromotedWidget.test.ts @@ -109,7 +109,7 @@ describe('resolveConcretePromotedWidget', () => { subgraph: { inputNode: { slots: [{ name: 'x', linkIds: [1] }] }, getLink: () => ({ - resolve: () => ({ inputNode: recursiveNode }) + resolve: () => ({ inputNode: recursiveNode, input: recursiveInput }) }), getNodeById: () => recursiveNode } diff --git a/src/core/graph/subgraph/resolveSubgraphInputLink.ts b/src/core/graph/subgraph/resolveSubgraphInputLink.ts index 595e2912bf..682927af65 100644 --- a/src/core/graph/subgraph/resolveSubgraphInputLink.ts +++ b/src/core/graph/subgraph/resolveSubgraphInputLink.ts @@ -25,12 +25,8 @@ export function resolveSubgraphInputLink( 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 diff --git a/src/lib/litegraph/src/LGraph.ts b/src/lib/litegraph/src/LGraph.ts index 9971634ce0..dbf9b474fe 100644 --- a/src/lib/litegraph/src/LGraph.ts +++ b/src/lib/litegraph/src/LGraph.ts @@ -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 diff --git a/src/lib/litegraph/src/LGraphCanvas.drawConnections.test.ts b/src/lib/litegraph/src/LGraphCanvas.drawConnections.test.ts index e147fba9e7..6ab4d2b8e4 100644 --- a/src/lib/litegraph/src/LGraphCanvas.drawConnections.test.ts +++ b/src/lib/litegraph/src/LGraphCanvas.drawConnections.test.ts @@ -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] diff --git a/src/lib/litegraph/src/LGraphCanvas.ts b/src/lib/litegraph/src/LGraphCanvas.ts index f48cab414e..08f3a92358 100644 --- a/src/lib/litegraph/src/LGraphCanvas.ts +++ b/src/lib/litegraph/src/LGraphCanvas.ts @@ -6044,48 +6044,41 @@ export class LGraphCanvas implements CustomEventDispatcher 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) { diff --git a/src/lib/litegraph/src/LGraphNode.ts b/src/lib/litegraph/src/LGraphNode.ts index 8045e9b99d..18172461b2 100644 --- a/src/lib/litegraph/src/LGraphNode.ts +++ b/src/lib/litegraph/src/LGraphNode.ts @@ -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 + }) } /** diff --git a/src/lib/litegraph/src/node/NodeSlot.deprecation.test.ts b/src/lib/litegraph/src/node/NodeSlot.deprecation.test.ts new file mode 100644 index 0000000000..0d0fc5c43f --- /dev/null +++ b/src/lib/litegraph/src/node/NodeSlot.deprecation.test.ts @@ -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]) + }) + }) +}) diff --git a/src/lib/litegraph/src/node/slotLinks.test.ts b/src/lib/litegraph/src/node/slotLinks.test.ts index 7af234ee13..0831298684 100644 --- a/src/lib/litegraph/src/node/slotLinks.test.ts +++ b/src/lib/litegraph/src/node/slotLinks.test.ts @@ -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[] = [] diff --git a/src/lib/litegraph/src/subgraph/SubgraphNode.ts b/src/lib/litegraph/src/subgraph/SubgraphNode.ts index d5a5e75a96..3954907f36 100644 --- a/src/lib/litegraph/src/subgraph/SubgraphNode.ts +++ b/src/lib/litegraph/src/subgraph/SubgraphNode.ts @@ -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 diff --git a/src/platform/missingModel/missingModelScan.test.ts b/src/platform/missingModel/missingModelScan.test.ts index 774ad4ee51..eda056558e 100644 --- a/src/platform/missingModel/missingModelScan.test.ts +++ b/src/platform/missingModel/missingModelScan.test.ts @@ -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