mirror of
https://github.com/Comfy-Org/ComfyUI_frontend.git
synced 2026-07-19 02:06:38 +00:00
refactor: harden output.links deprecation, migrate last readers
- setOutputData/setOutputDataType and repairPrimitive read the link store via helpers instead of the warning getter, keeping first-party code out of the extension deprecation telemetry - freeze the getter's returned array so extension push/splice throws instead of silently discarding; narrow the declare to drop undefined - add behavioral tests for the deprecation getter - docs: pseudocode and floating-link claim match shipped buildOutputIndex, migration map gains enumerate-links entry, Decision 5 precedes 6, ECS plan Phase 3c/4b status marks output-side work done
This commit is contained in:
@@ -277,9 +277,14 @@ data is complete and consistent with the class-based graph.
|
||||
> `rerouteStore` (PRs 13436, 13449): "what links pass through this reroute" is
|
||||
> derived per root graph by a cached reverse index over the links' `parentId`
|
||||
> chains, and input-side connectivity is one lookup via
|
||||
> `linkStore.isInputSlotConnected()` / `getInputSlotLink()`. Remaining:
|
||||
> slot mirrors (`input.link` / `output.links`), output-side queries, and
|
||||
> execution order.
|
||||
> `linkStore.isInputSlotConnected()` / `getInputSlotLink()`.
|
||||
>
|
||||
> **Status (2026-07-17):** Output-side queries shipped and the `output.links`
|
||||
> mirror is deleted (PR 13479): `linkStore.isOutputSlotConnected()` /
|
||||
> `getOutputSlotLinks()`, litegraph internals reading through
|
||||
> `node/slotLinks.ts` (see
|
||||
> [output slot connectivity](output-slot-connectivity.md)). Remaining: the
|
||||
> `input.link` slot mirror and execution order.
|
||||
|
||||
**Risk:** Low. Read-only system with equivalence tests.
|
||||
|
||||
@@ -336,7 +341,8 @@ the system knowing about the callback API.
|
||||
> through canonical `LGraph` mutation chokepoints: `_addLink`/`_removeLink` and
|
||||
> `_addReroute`/`_removeReroute` pair every map mutation with store
|
||||
> (un)registration, and `clear()` / subgraph-definition GC unregister whole
|
||||
> graphs. The callback contract above and slot-mirror extraction remain.
|
||||
> graphs. The callback contract above and `input.link` slot-mirror extraction
|
||||
> remain (`output.links` was deleted in PR 13479).
|
||||
|
||||
**Risk:** High. Extensions depend on callback ordering and timing. Must be
|
||||
validated against real-world extensions.
|
||||
|
||||
@@ -69,7 +69,7 @@ function outputIndex(graphId: UUID): ComputedRef<OriginIndex> {
|
||||
const next = computed(() => {
|
||||
const index: OriginIndex = new Map()
|
||||
for (const t of graphTopologies(graphId)) {
|
||||
if (t.originNodeId === UNASSIGNED_NODE_ID) continue
|
||||
if (isFloatingTopology(t)) continue
|
||||
const key = originKey(t.originNodeId, t.originSlot)
|
||||
const links = index.get(key) ?? new Set<LinkTopology>()
|
||||
links.add(t)
|
||||
@@ -83,9 +83,10 @@ function outputIndex(graphId: UUID): ComputedRef<OriginIndex> {
|
||||
```
|
||||
|
||||
The index spans both collections that `graphTopologies` yields, the keyed
|
||||
targets and the unkeyed side set, so a floating link with an assigned
|
||||
origin (one endpoint only) still reports its output as connected. A
|
||||
`SUBGRAPH_INPUT_ID` origin indexes like any other id.
|
||||
targets and the unkeyed side set. Floating links are skipped
|
||||
(`isFloatingTopology`): the queries see fully-assigned links only,
|
||||
matching the mirror they replace (Decision 6). A `SUBGRAPH_INPUT_ID`
|
||||
origin indexes like any other id.
|
||||
|
||||
## Decision 3: Two public queries, mirroring the input pair
|
||||
|
||||
@@ -123,6 +124,24 @@ a mirror read plus a `slotFloatingLinks` scan), widget value propagation
|
||||
(`widgetValuePropagation`), and matchType link revalidation
|
||||
(`dynamicWidgets.changeOutputType`).
|
||||
|
||||
## Decision 5: Wire connected state into the dots (the payoff)
|
||||
|
||||
`NodeSlots.vue` passes `connected` to each slot:
|
||||
|
||||
- input: `linkStore.isInputSlotConnected(rootGraphId, nodeId, index)`
|
||||
(already available)
|
||||
- output: `linkStore.isOutputSlotConnected(rootGraphId, nodeId, index)`
|
||||
(Decision 3)
|
||||
|
||||
`InputSlot` and `OutputSlot` already forward `connected` to the
|
||||
`lg-slot--connected` class. `SlotConnectionDot` needs no prop of its own:
|
||||
the wrapper class is an ancestor styling hook
|
||||
(`.lg-slot--connected .slot-dot`), so the visual is one CSS rule away once
|
||||
the design-standards check (open question 3) picks it. Threading a
|
||||
`connected` prop into the dot before that would be plumbing with no
|
||||
consumer. `compatible` stays driven by the existing drag state
|
||||
(`useSlotLinkDragUIState`), which this phase leaves alone.
|
||||
|
||||
## Decision 6: Delete the mirror (implemented)
|
||||
|
||||
The runtime `output.links[]` field and all nine of its write sites are
|
||||
@@ -160,26 +179,10 @@ the pure helpers in `node/slotLinks.ts` (`outputHasLinks`,
|
||||
|
||||
Extension migration map: presence → `node.isOutputConnected(slot)` /
|
||||
`slot.isConnected`; enumerate targets → `node.getOutputNodes(slot)`;
|
||||
mutate → `node.connect(...)` / `node.disconnectOutput(slot, target?)`.
|
||||
App/Vue code uses `useLinkStore().getOutputSlotLinks(...)` (reactive).
|
||||
|
||||
## Decision 5: Wire connected state into the dots (the payoff)
|
||||
|
||||
`NodeSlots.vue` passes `connected` to each slot:
|
||||
|
||||
- input: `linkStore.isInputSlotConnected(rootGraphId, nodeId, index)`
|
||||
(already available)
|
||||
- output: `linkStore.isOutputSlotConnected(rootGraphId, nodeId, index)`
|
||||
(Decision 3)
|
||||
|
||||
`InputSlot` and `OutputSlot` already forward `connected` to the
|
||||
`lg-slot--connected` class. `SlotConnectionDot` needs no prop of its own:
|
||||
the wrapper class is an ancestor styling hook
|
||||
(`.lg-slot--connected .slot-dot`), so the visual is one CSS rule away once
|
||||
the design-standards check (open question 3) picks it. Threading a
|
||||
`connected` prop into the dot before that would be plumbing with no
|
||||
consumer. `compatible` stays driven by the existing drag state
|
||||
(`useSlotLinkDragUIState`), which this phase leaves alone.
|
||||
enumerate links → `outputLinks(graph, node.id, slot)` / `outputLinkIds`
|
||||
(`node/slotLinks.ts`); mutate → `node.connect(...)` /
|
||||
`node.disconnectOutput(slot, target?)`. App/Vue code uses
|
||||
`useLinkStore().getOutputSlotLinks(...)` (reactive).
|
||||
|
||||
## Scope
|
||||
|
||||
|
||||
@@ -31,7 +31,7 @@ import { useLinkStore } from '@/stores/linkStore'
|
||||
import { usePreviewExposureStore } from '@/stores/previewExposureStore'
|
||||
import { useWidgetValueStore } from '@/stores/widgetValueStore'
|
||||
import type { LinkTopology } from '@/types/linkTopology'
|
||||
import { UNASSIGNED_NODE_ID, toNodeId } from '@/types/nodeId'
|
||||
import { toNodeId } from '@/types/nodeId'
|
||||
import type { NodeId, SerializedNodeId } from '@/types/nodeId'
|
||||
|
||||
interface LegacyProxyEntrySource extends PromotedWidgetSource {
|
||||
@@ -624,17 +624,14 @@ function repairPrimitive(
|
||||
}
|
||||
|
||||
const baseName = userRenamedTitle(primitiveNode) ?? validated.sourceWidgetName
|
||||
const snapshot: SnapshotLink[] = (primitiveOutput.links ?? [])
|
||||
.map((id) => subgraph.links.get(id))
|
||||
.filter(
|
||||
(l): l is NonNullable<typeof l> =>
|
||||
l !== undefined && l.target_id !== UNASSIGNED_NODE_ID
|
||||
)
|
||||
.map((l) => ({
|
||||
primitiveSlot: l.origin_slot,
|
||||
targetNodeId: l.target_id,
|
||||
targetSlot: l.target_slot
|
||||
}))
|
||||
const snapshot: SnapshotLink[] = primitiveOutputTopologies(
|
||||
hostNode,
|
||||
primitiveNode
|
||||
).map((topology) => ({
|
||||
primitiveSlot: topology.originSlot,
|
||||
targetNodeId: topology.targetNodeId,
|
||||
targetSlot: topology.targetSlot
|
||||
}))
|
||||
|
||||
let newSubgraphInput: SubgraphInput | undefined
|
||||
try {
|
||||
|
||||
@@ -1128,13 +1128,8 @@ export class LGraphNode
|
||||
|
||||
if (!this.graph) throw new NullGraphError()
|
||||
|
||||
// if there are connections, pass the data to the connections
|
||||
const { links } = outputs[slot]
|
||||
if (links) {
|
||||
for (const id of links) {
|
||||
const link = this.graph._links.get(id)
|
||||
if (link) link.data = data
|
||||
}
|
||||
for (const link of outputLinks(this.graph, this.id, slot)) {
|
||||
link.data = data
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1152,13 +1147,8 @@ export class LGraphNode
|
||||
|
||||
if (!this.graph) throw new NullGraphError()
|
||||
|
||||
// if there are connections, pass the data to the connections
|
||||
const { links } = outputs[slot]
|
||||
if (links) {
|
||||
for (const id of links) {
|
||||
const link = this.graph._links.get(id)
|
||||
if (link) link.type = type
|
||||
}
|
||||
for (const link of outputLinks(this.graph, this.id, slot)) {
|
||||
link.type = type
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
81
src/lib/litegraph/src/node/NodeOutputSlot.test.ts
Normal file
81
src/lib/litegraph/src/node/NodeOutputSlot.test.ts
Normal file
@@ -0,0 +1,81 @@
|
||||
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 createConnectedGraph() {
|
||||
const graph = new LGraph()
|
||||
const source = new LGraphNode('Source')
|
||||
source.addOutput('out', 'INT')
|
||||
graph.add(source)
|
||||
|
||||
const target = new LGraphNode('Target')
|
||||
target.addInput('in', 'INT')
|
||||
graph.add(target)
|
||||
|
||||
return { graph, source, target }
|
||||
}
|
||||
|
||||
describe('NodeOutputSlot deprecated links getter', () => {
|
||||
const onWarning = vi.fn()
|
||||
|
||||
beforeEach(() => {
|
||||
setActivePinia(createTestingPinia({ stubActions: false }))
|
||||
onWarning.mockClear()
|
||||
LiteGraph.onDeprecationWarning = [onWarning]
|
||||
LiteGraph.alwaysRepeatWarnings = true
|
||||
})
|
||||
|
||||
afterEach(() => {
|
||||
LiteGraph.alwaysRepeatWarnings = false
|
||||
})
|
||||
|
||||
it('fires a deprecation warning on read', () => {
|
||||
const { source } = createConnectedGraph()
|
||||
|
||||
void source.outputs[0].links
|
||||
|
||||
expect(onWarning).toHaveBeenCalledWith(
|
||||
expect.stringContaining('output.links is deprecated'),
|
||||
undefined
|
||||
)
|
||||
})
|
||||
|
||||
it('reflects live link store data across connect and disconnect', () => {
|
||||
const { source, target } = createConnectedGraph()
|
||||
|
||||
const link = source.connect(0, target, 0)
|
||||
expect(source.outputs[0].links).toEqual([link!.id])
|
||||
|
||||
source.disconnectOutput(0)
|
||||
expect(source.outputs[0].links).toBeNull()
|
||||
})
|
||||
|
||||
it('returns null for an unconnected slot and for a graphless node', () => {
|
||||
const { source } = createConnectedGraph()
|
||||
expect(source.outputs[0].links).toBeNull()
|
||||
|
||||
const orphan = new LGraphNode('Orphan')
|
||||
orphan.addOutput('out', 'INT')
|
||||
expect(orphan.outputs[0].links).toBeNull()
|
||||
})
|
||||
|
||||
it('throws on assignment', () => {
|
||||
const { source } = createConnectedGraph()
|
||||
const slot: { links?: unknown } = source.outputs[0]
|
||||
|
||||
expect(() => {
|
||||
slot.links = []
|
||||
}).toThrow(TypeError)
|
||||
})
|
||||
|
||||
it('throws on mutation of the returned array', () => {
|
||||
const { source, target } = createConnectedGraph()
|
||||
source.connect(0, target, 0)
|
||||
|
||||
const links = source.outputs[0].links as LinkId[]
|
||||
expect(() => links.push(links[0])).toThrow(TypeError)
|
||||
})
|
||||
})
|
||||
@@ -21,7 +21,7 @@ import { warnDeprecated } from '@/lib/litegraph/src/utils/feedback'
|
||||
|
||||
export class NodeOutputSlot extends NodeSlot implements INodeOutputSlot {
|
||||
/** @deprecated Derived from the link store via a warning prototype getter; never written. */
|
||||
declare readonly links?: readonly LinkId[] | null
|
||||
declare readonly links: readonly LinkId[] | null
|
||||
_data?: unknown
|
||||
slot_index?: number
|
||||
|
||||
@@ -110,7 +110,7 @@ export class NodeOutputSlot extends NodeSlot implements INodeOutputSlot {
|
||||
* writes throw in strict mode. First-party code uses the slotLinks helpers.
|
||||
*/
|
||||
Object.defineProperty(NodeOutputSlot.prototype, 'links', {
|
||||
get(this: NodeOutputSlot): LinkId[] | null {
|
||||
get(this: NodeOutputSlot): readonly LinkId[] | null {
|
||||
warnDeprecated(
|
||||
'output.links is deprecated. Read connectivity via node.isOutputConnected(slot) / node.getOutputNodes(slot); mutate via node.connect() / node.disconnectOutput().'
|
||||
)
|
||||
@@ -121,7 +121,7 @@ Object.defineProperty(NodeOutputSlot.prototype, 'links', {
|
||||
this._node.id,
|
||||
this._node.outputs.indexOf(this)
|
||||
)
|
||||
return ids.length ? ids : null
|
||||
return ids.length ? Object.freeze(ids) : null
|
||||
},
|
||||
configurable: true,
|
||||
enumerable: false
|
||||
|
||||
Reference in New Issue
Block a user