mirror of
https://github.com/Comfy-Org/ComfyUI_frontend.git
synced 2026-04-20 14:30:41 +00:00
refactor: delete ProxyWidget infrastructure
Phase 6: Remove dead ProxyWidget code replaced by PromotedWidgetSlot system. - Delete proxyWidget.ts (Proxy handler, isProxyWidget, registerProxyWidgets) - Delete proxyWidget.test.ts - Delete DisconnectedWidget.ts (only consumer was proxyWidget.ts) - Clean JSDoc reference in promotedWidgetRegistration.ts Amp-Thread-ID: https://ampcode.com/threads/T-019c5018-6d2c-7114-9ef9-c5dc35fcacac Co-authored-by: Amp <amp@ampcode.com>
This commit is contained in:
@@ -10,8 +10,6 @@ import { useCanvasStore } from '@/renderer/core/canvas/canvasStore'
|
||||
|
||||
/**
|
||||
* Registers the promoted widget system using PromotedWidgetSlot instances.
|
||||
* Replaces the legacy registerProxyWidgets which used JavaScript Proxy objects.
|
||||
*
|
||||
* Sets up:
|
||||
* - `subgraph-opened` event: syncs `promoted` flags on interior widgets
|
||||
* - `subgraph-converted` event: auto-promotes recommended widgets
|
||||
@@ -42,7 +40,7 @@ const onConfigure = function (
|
||||
serialisedNode: ISerialisedNode
|
||||
) {
|
||||
if (!this.isSubgraphNode())
|
||||
throw new Error("Can't add proxyWidgets to non-subgraphNode")
|
||||
throw new Error("Can't add promoted widgets to non-subgraphNode")
|
||||
|
||||
const canvasStore = useCanvasStore()
|
||||
this.properties.proxyWidgets ??= []
|
||||
|
||||
@@ -1,147 +0,0 @@
|
||||
import { createTestingPinia } from '@pinia/testing'
|
||||
import { setActivePinia } from 'pinia'
|
||||
import { beforeEach, describe, expect, test, vi } from 'vitest'
|
||||
|
||||
import { registerProxyWidgets } from '@/core/graph/subgraph/proxyWidget'
|
||||
import { promoteWidget } from '@/core/graph/subgraph/proxyWidgetUtils'
|
||||
import { parseProxyWidgets } from '@/core/schemas/proxyWidget'
|
||||
import { LGraphNode } from '@/lib/litegraph/src/litegraph'
|
||||
import type { LGraphCanvas, SubgraphNode } from '@/lib/litegraph/src/litegraph'
|
||||
|
||||
import {
|
||||
createTestSubgraph,
|
||||
createTestSubgraphNode
|
||||
} from '@/lib/litegraph/src/subgraph/__fixtures__/subgraphHelpers'
|
||||
|
||||
const canvasEl: Partial<HTMLCanvasElement> = { addEventListener() {} }
|
||||
const canvas: Partial<LGraphCanvas> = { canvas: canvasEl as HTMLCanvasElement }
|
||||
registerProxyWidgets(canvas as LGraphCanvas)
|
||||
|
||||
vi.mock('@/renderer/core/canvas/canvasStore', () => ({
|
||||
useCanvasStore: () => ({})
|
||||
}))
|
||||
vi.mock('@/stores/domWidgetStore', () => ({
|
||||
useDomWidgetStore: () => ({ widgetStates: new Map() })
|
||||
}))
|
||||
vi.mock('@/services/litegraphService', () => ({
|
||||
useLitegraphService: () => ({ updatePreviews: () => ({}) })
|
||||
}))
|
||||
|
||||
function setupSubgraph(
|
||||
innerNodeCount: number = 0
|
||||
): [SubgraphNode, LGraphNode[]] {
|
||||
const subgraph = createTestSubgraph()
|
||||
const subgraphNode = createTestSubgraphNode(subgraph)
|
||||
subgraphNode._internalConfigureAfterSlots()
|
||||
const graph = subgraphNode.graph!
|
||||
graph.add(subgraphNode)
|
||||
const innerNodes = []
|
||||
for (let i = 0; i < innerNodeCount; i++) {
|
||||
const innerNode = new LGraphNode(`InnerNode${i}`)
|
||||
subgraph.add(innerNode)
|
||||
innerNodes.push(innerNode)
|
||||
}
|
||||
return [subgraphNode, innerNodes]
|
||||
}
|
||||
|
||||
describe('Subgraph proxyWidgets', () => {
|
||||
beforeEach(() => {
|
||||
setActivePinia(createTestingPinia({ stubActions: false }))
|
||||
})
|
||||
|
||||
test('Can add simple widget', () => {
|
||||
const [subgraphNode, innerNodes] = setupSubgraph(1)
|
||||
innerNodes[0].addWidget('text', 'stringWidget', 'value', () => {})
|
||||
subgraphNode.properties.proxyWidgets = [['1', 'stringWidget']]
|
||||
expect(subgraphNode.widgets.length).toBe(1)
|
||||
expect(subgraphNode.properties.proxyWidgets).toStrictEqual([
|
||||
['1', 'stringWidget']
|
||||
])
|
||||
})
|
||||
test('Can add multiple widgets with same name', () => {
|
||||
const [subgraphNode, innerNodes] = setupSubgraph(2)
|
||||
for (const innerNode of innerNodes)
|
||||
innerNode.addWidget('text', 'stringWidget', 'value', () => {})
|
||||
subgraphNode.properties.proxyWidgets = [
|
||||
['1', 'stringWidget'],
|
||||
['2', 'stringWidget']
|
||||
]
|
||||
expect(subgraphNode.widgets.length).toBe(2)
|
||||
expect(subgraphNode.widgets[0].name).not.toEqual(
|
||||
subgraphNode.widgets[1].name
|
||||
)
|
||||
})
|
||||
test('Will serialize existing widgets', () => {
|
||||
const [subgraphNode, innerNodes] = setupSubgraph(1)
|
||||
innerNodes[0].addWidget('text', 'istringWidget', 'value', () => {})
|
||||
subgraphNode.addWidget('text', 'stringWidget', 'value', () => {})
|
||||
|
||||
const proxyWidgets = parseProxyWidgets(subgraphNode.properties.proxyWidgets)
|
||||
proxyWidgets.push(['1', 'istringWidget'])
|
||||
subgraphNode.properties.proxyWidgets = proxyWidgets
|
||||
|
||||
expect(subgraphNode.widgets.length).toBe(2)
|
||||
expect(subgraphNode.widgets[0].name).toBe('stringWidget')
|
||||
subgraphNode.properties.proxyWidgets = [proxyWidgets[1], proxyWidgets[0]]
|
||||
expect(subgraphNode.widgets[0].name).toBe('1: istringWidget')
|
||||
})
|
||||
test('Will mirror changes to value', () => {
|
||||
const [subgraphNode, innerNodes] = setupSubgraph(1)
|
||||
innerNodes[0].addWidget('text', 'stringWidget', 'value', () => {})
|
||||
subgraphNode.properties.proxyWidgets = [['1', 'stringWidget']]
|
||||
expect(subgraphNode.widgets.length).toBe(1)
|
||||
expect(subgraphNode.widgets[0].value).toBe('value')
|
||||
innerNodes[0].widgets![0].value = 'test'
|
||||
expect(subgraphNode.widgets[0].value).toBe('test')
|
||||
subgraphNode.widgets[0].value = 'test2'
|
||||
expect(innerNodes[0].widgets![0].value).toBe('test2')
|
||||
})
|
||||
test('Will not modify position or sizing of existing widgets', () => {
|
||||
const [subgraphNode, innerNodes] = setupSubgraph(1)
|
||||
innerNodes[0].addWidget('text', 'stringWidget', 'value', () => {})
|
||||
subgraphNode.properties.proxyWidgets = [['1', 'stringWidget']]
|
||||
if (!innerNodes[0].widgets) throw new Error('node has no widgets')
|
||||
innerNodes[0].widgets[0].y = 10
|
||||
innerNodes[0].widgets[0].last_y = 11
|
||||
innerNodes[0].widgets[0].computedHeight = 12
|
||||
subgraphNode.widgets[0].y = 20
|
||||
subgraphNode.widgets[0].last_y = 21
|
||||
subgraphNode.widgets[0].computedHeight = 22
|
||||
expect(innerNodes[0].widgets[0].y).toBe(10)
|
||||
expect(innerNodes[0].widgets[0].last_y).toBe(11)
|
||||
expect(innerNodes[0].widgets[0].computedHeight).toBe(12)
|
||||
})
|
||||
test('Can detach and re-attach widgets', () => {
|
||||
const [subgraphNode, innerNodes] = setupSubgraph(1)
|
||||
innerNodes[0].addWidget('text', 'stringWidget', 'value', () => {})
|
||||
subgraphNode.properties.proxyWidgets = [['1', 'stringWidget']]
|
||||
if (!innerNodes[0].widgets) throw new Error('node has no widgets')
|
||||
expect(subgraphNode.widgets[0].value).toBe('value')
|
||||
const poppedWidget = innerNodes[0].widgets.pop()
|
||||
//simulate new draw frame
|
||||
subgraphNode.widgets[0].computedHeight = 10
|
||||
expect(subgraphNode.widgets[0].value).toBe(undefined)
|
||||
innerNodes[0].widgets.push(poppedWidget!)
|
||||
subgraphNode.widgets[0].computedHeight = 10
|
||||
expect(subgraphNode.widgets[0].value).toBe('value')
|
||||
})
|
||||
test('Prevents duplicate promotion', () => {
|
||||
const [subgraphNode, innerNodes] = setupSubgraph(1)
|
||||
innerNodes[0].addWidget('text', 'stringWidget', 'value', () => {})
|
||||
|
||||
const widget = innerNodes[0].widgets![0]
|
||||
|
||||
// Promote once
|
||||
promoteWidget(innerNodes[0], widget, [subgraphNode])
|
||||
expect(subgraphNode.widgets.length).toBe(1)
|
||||
expect(subgraphNode.properties.proxyWidgets).toHaveLength(1)
|
||||
|
||||
// Try to promote again - should not create duplicate
|
||||
promoteWidget(innerNodes[0], widget, [subgraphNode])
|
||||
expect(subgraphNode.widgets.length).toBe(1)
|
||||
expect(subgraphNode.properties.proxyWidgets).toHaveLength(1)
|
||||
expect(subgraphNode.properties.proxyWidgets).toStrictEqual([
|
||||
['1', 'stringWidget']
|
||||
])
|
||||
})
|
||||
})
|
||||
@@ -1,251 +0,0 @@
|
||||
import {
|
||||
demoteWidget,
|
||||
promoteRecommendedWidgets
|
||||
} from '@/core/graph/subgraph/proxyWidgetUtils'
|
||||
import { parseProxyWidgets } from '@/core/schemas/proxyWidget'
|
||||
import type { NodeProperty } from '@/lib/litegraph/src/LGraphNode'
|
||||
import type {
|
||||
LGraph,
|
||||
LGraphCanvas,
|
||||
LGraphNode
|
||||
} from '@/lib/litegraph/src/litegraph'
|
||||
import { SubgraphNode } from '@/lib/litegraph/src/subgraph/SubgraphNode'
|
||||
import type { ISerialisedNode } from '@/lib/litegraph/src/types/serialisation'
|
||||
import type { IBaseWidget } from '@/lib/litegraph/src/types/widgets.ts'
|
||||
import { disconnectedWidget } from '@/lib/litegraph/src/widgets/DisconnectedWidget'
|
||||
import { useCanvasStore } from '@/renderer/core/canvas/canvasStore'
|
||||
import { DOMWidgetImpl } from '@/scripts/domWidget'
|
||||
import { useLitegraphService } from '@/services/litegraphService'
|
||||
import { useDomWidgetStore } from '@/stores/domWidgetStore'
|
||||
import { getNodeByExecutionId } from '@/utils/graphTraversalUtil'
|
||||
|
||||
/**
|
||||
* @typedef {object} Overlay - Each proxy Widget has an associated overlay object
|
||||
* Accessing a property which exists in the overlay object will
|
||||
* instead result in the action being performed on the overlay object
|
||||
* 3 properties are added for locating the proxied widget
|
||||
* @property {LGraph} graph - The graph the widget resides in. Used for widget lookup
|
||||
* @property {string} nodeId - The NodeId the proxy Widget is located on
|
||||
* @property {string} widgetName - The name of the linked widget
|
||||
*
|
||||
* @property {boolean} isProxyWidget - Always true, used as type guard
|
||||
* @property {LGraphNode} node - not included on IBaseWidget, but required for overlay
|
||||
*/
|
||||
type Overlay = Partial<IBaseWidget> & {
|
||||
graph: LGraph
|
||||
nodeId: string
|
||||
widgetName: string
|
||||
isProxyWidget: boolean
|
||||
node?: LGraphNode
|
||||
}
|
||||
// A ProxyWidget can be treated like a normal widget.
|
||||
// the _overlay property can be used to directly access the Overlay object
|
||||
/**
|
||||
* @typedef {object} ProxyWidget - a reference to a widget that can
|
||||
* be displayed and owned by a separate node
|
||||
* @property {Overlay} _overlay - a special property to access the overlay of the widget
|
||||
* Any property that exists in the overlay will be accessed instead of the property
|
||||
* on the linked widget
|
||||
*/
|
||||
type ProxyWidget = IBaseWidget & { _overlay: Overlay }
|
||||
export function isProxyWidget(w: IBaseWidget): w is ProxyWidget {
|
||||
return (w as { _overlay?: Overlay })?._overlay?.isProxyWidget ?? false
|
||||
}
|
||||
export function isDisconnectedWidget(w: ProxyWidget) {
|
||||
return w instanceof disconnectedWidget.constructor
|
||||
}
|
||||
|
||||
export function registerProxyWidgets(canvas: LGraphCanvas) {
|
||||
//NOTE: canvasStore hasn't been initialized yet
|
||||
canvas.canvas.addEventListener<'subgraph-opened'>('subgraph-opened', (e) => {
|
||||
const { subgraph, fromNode } = e.detail
|
||||
const proxyWidgets = parseProxyWidgets(fromNode.properties.proxyWidgets)
|
||||
for (const node of subgraph.nodes) {
|
||||
for (const widget of node.widgets ?? []) {
|
||||
widget.promoted = proxyWidgets.some(
|
||||
([n, w]) => node.id == n && widget.name == w
|
||||
)
|
||||
}
|
||||
}
|
||||
})
|
||||
canvas.canvas.addEventListener<'subgraph-converted'>(
|
||||
'subgraph-converted',
|
||||
(e) => promoteRecommendedWidgets(e.detail.subgraphNode)
|
||||
)
|
||||
SubgraphNode.prototype.onConfigure = onConfigure
|
||||
}
|
||||
|
||||
const originalOnConfigure = SubgraphNode.prototype.onConfigure
|
||||
const onConfigure = function (
|
||||
this: LGraphNode,
|
||||
serialisedNode: ISerialisedNode
|
||||
) {
|
||||
if (!this.isSubgraphNode())
|
||||
throw new Error("Can't add proxyWidgets to non-subgraphNode")
|
||||
|
||||
const canvasStore = useCanvasStore()
|
||||
//Must give value to proxyWidgets prior to defining or it won't serialize
|
||||
this.properties.proxyWidgets ??= []
|
||||
|
||||
originalOnConfigure?.call(this, serialisedNode)
|
||||
|
||||
Object.defineProperty(this.properties, 'proxyWidgets', {
|
||||
get: () =>
|
||||
this.widgets.map((w) =>
|
||||
isProxyWidget(w)
|
||||
? [w._overlay.nodeId, w._overlay.widgetName]
|
||||
: ['-1', w.name]
|
||||
),
|
||||
set: (property: NodeProperty) => {
|
||||
const parsed = parseProxyWidgets(property)
|
||||
const { deactivateWidget, setWidget } = useDomWidgetStore()
|
||||
const isActiveGraph = useCanvasStore().canvas?.graph === this.graph
|
||||
if (isActiveGraph) {
|
||||
for (const w of this.widgets.filter((w) => isProxyWidget(w))) {
|
||||
if (w instanceof DOMWidgetImpl) deactivateWidget(w.id)
|
||||
}
|
||||
}
|
||||
|
||||
const newWidgets = parsed.flatMap(([nodeId, widgetName]) => {
|
||||
if (nodeId === '-1') {
|
||||
const widget = this.widgets.find((w) => w.name === widgetName)
|
||||
return widget ? [widget] : []
|
||||
}
|
||||
const w = newProxyWidget(this, nodeId, widgetName)
|
||||
if (isActiveGraph && w instanceof DOMWidgetImpl) setWidget(w)
|
||||
return [w]
|
||||
})
|
||||
this.widgets = this.widgets.filter((w) => {
|
||||
if (isProxyWidget(w)) return false
|
||||
const widgetName = w.name
|
||||
return !parsed.some(([, name]) => widgetName === name)
|
||||
})
|
||||
this.widgets.push(...newWidgets)
|
||||
|
||||
canvasStore.canvas?.setDirty(true, true)
|
||||
this._setConcreteSlots()
|
||||
this.arrange()
|
||||
}
|
||||
})
|
||||
if (serialisedNode.properties?.proxyWidgets) {
|
||||
this.properties.proxyWidgets = serialisedNode.properties.proxyWidgets
|
||||
const parsed = parseProxyWidgets(serialisedNode.properties.proxyWidgets)
|
||||
serialisedNode.widgets_values?.forEach((v, index) => {
|
||||
if (parsed[index]?.[0] !== '-1') return
|
||||
const widget = this.widgets.find((w) => w.name == parsed[index][1])
|
||||
if (v !== null && widget) widget.value = v
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
function newProxyWidget(
|
||||
subgraphNode: SubgraphNode,
|
||||
nodeId: string,
|
||||
widgetName: string
|
||||
) {
|
||||
const name = `${nodeId}: ${widgetName}`
|
||||
const overlay = {
|
||||
//items specific for proxy management
|
||||
nodeId,
|
||||
graph: subgraphNode.subgraph,
|
||||
widgetName,
|
||||
//Items which normally exist on widgets
|
||||
afterQueued: undefined,
|
||||
computedHeight: undefined,
|
||||
isProxyWidget: true,
|
||||
last_y: undefined,
|
||||
label: name,
|
||||
name,
|
||||
node: subgraphNode,
|
||||
onRemove: undefined,
|
||||
promoted: undefined,
|
||||
serialize: false,
|
||||
width: undefined,
|
||||
y: 0
|
||||
}
|
||||
return newProxyFromOverlay(subgraphNode, overlay)
|
||||
}
|
||||
function resolveLinkedWidget(
|
||||
overlay: Overlay
|
||||
): [LGraphNode | undefined, IBaseWidget | undefined] {
|
||||
const { graph, nodeId, widgetName } = overlay
|
||||
const n = getNodeByExecutionId(graph, nodeId)
|
||||
if (!n) return [undefined, undefined]
|
||||
const widget = n.widgets?.find((w: IBaseWidget) => w.name === widgetName)
|
||||
//Slightly hacky. Force recursive resolution of nested widgets
|
||||
if (widget && isProxyWidget(widget) && isDisconnectedWidget(widget))
|
||||
widget.computedHeight = 20
|
||||
return [n, widget]
|
||||
}
|
||||
|
||||
function newProxyFromOverlay(subgraphNode: SubgraphNode, overlay: Overlay) {
|
||||
const { updatePreviews } = useLitegraphService()
|
||||
let [linkedNode, linkedWidget] = resolveLinkedWidget(overlay)
|
||||
let backingWidget = linkedWidget ?? disconnectedWidget
|
||||
if (overlay.widgetName.startsWith('$$')) {
|
||||
overlay.node = new Proxy(subgraphNode, {
|
||||
get(_t, p) {
|
||||
if (p !== 'imgs') return Reflect.get(subgraphNode, p)
|
||||
if (!linkedNode) return []
|
||||
return linkedNode.imgs
|
||||
}
|
||||
})
|
||||
}
|
||||
/**
|
||||
* A set of handlers which define widget interaction
|
||||
* Many arguments are shared between function calls
|
||||
* @param {IBaseWidget} _t - The "target" the call is originally made on.
|
||||
* This argument is never used, but must be defined for typechecking
|
||||
* @param {string} property - The name of the accessed value.
|
||||
* Checked for conditional logic, but never changed
|
||||
* @param {object} receiver - The object the result is set to
|
||||
* and the value used as 'this' if property is a get/set method
|
||||
* @param {unknown} value - only used on set calls. The thing being assigned
|
||||
*/
|
||||
const handler = {
|
||||
get(_t: IBaseWidget, property: string, receiver: object) {
|
||||
let redirectedTarget: object = backingWidget
|
||||
let redirectedReceiver = receiver
|
||||
if (property == '_overlay') return overlay
|
||||
else if (property == 'value') redirectedReceiver = backingWidget
|
||||
if (Object.prototype.hasOwnProperty.call(overlay, property)) {
|
||||
redirectedTarget = overlay
|
||||
redirectedReceiver = overlay
|
||||
}
|
||||
return Reflect.get(redirectedTarget, property, redirectedReceiver)
|
||||
},
|
||||
set(_t: IBaseWidget, property: string, value: unknown) {
|
||||
let redirectedTarget: object = backingWidget
|
||||
if (property == 'computedHeight') {
|
||||
if (overlay.widgetName.startsWith('$$') && linkedNode) {
|
||||
updatePreviews(linkedNode)
|
||||
}
|
||||
if (linkedNode && linkedWidget?.computedDisabled) {
|
||||
demoteWidget(linkedNode, linkedWidget, [subgraphNode])
|
||||
}
|
||||
//update linkage regularly, but no more than once per frame
|
||||
;[linkedNode, linkedWidget] = resolveLinkedWidget(overlay)
|
||||
backingWidget = linkedWidget ?? disconnectedWidget
|
||||
}
|
||||
if (Object.prototype.hasOwnProperty.call(overlay, property)) {
|
||||
redirectedTarget = overlay
|
||||
}
|
||||
return Reflect.set(redirectedTarget, property, value, redirectedTarget)
|
||||
},
|
||||
getPrototypeOf() {
|
||||
return Reflect.getPrototypeOf(backingWidget)
|
||||
},
|
||||
ownKeys() {
|
||||
return Reflect.ownKeys(backingWidget)
|
||||
},
|
||||
has(_t: IBaseWidget, property: string) {
|
||||
let redirectedTarget: object = backingWidget
|
||||
if (Object.prototype.hasOwnProperty.call(overlay, property)) {
|
||||
redirectedTarget = overlay
|
||||
}
|
||||
return Reflect.has(redirectedTarget, property)
|
||||
}
|
||||
}
|
||||
const w = new Proxy(disconnectedWidget, handler)
|
||||
return w
|
||||
}
|
||||
@@ -1,39 +0,0 @@
|
||||
import { LGraphNode } from '@/lib/litegraph/src/LGraphNode'
|
||||
import type { IButtonWidget } from '@/lib/litegraph/src/types/widgets'
|
||||
|
||||
import { BaseWidget } from './BaseWidget'
|
||||
import type { DrawWidgetOptions } from './BaseWidget'
|
||||
|
||||
class DisconnectedWidget extends BaseWidget<IButtonWidget> {
|
||||
constructor(widget: IButtonWidget) {
|
||||
super(widget, new LGraphNode('DisconnectedPlaceholder'))
|
||||
this.disabled = true
|
||||
}
|
||||
|
||||
override drawWidget(
|
||||
ctx: CanvasRenderingContext2D,
|
||||
{ width, showText = true }: DrawWidgetOptions
|
||||
) {
|
||||
ctx.save()
|
||||
this.drawWidgetShape(ctx, { width, showText })
|
||||
if (showText) {
|
||||
this.drawTruncatingText({ ctx, width, leftPadding: 0, rightPadding: 0 })
|
||||
}
|
||||
ctx.restore()
|
||||
}
|
||||
|
||||
override onClick() {}
|
||||
|
||||
override get _displayValue() {
|
||||
return 'Disconnected'
|
||||
}
|
||||
}
|
||||
const conf: IButtonWidget = {
|
||||
type: 'button',
|
||||
value: undefined,
|
||||
name: 'Disconnected',
|
||||
options: {},
|
||||
y: 0,
|
||||
clicked: false
|
||||
}
|
||||
export const disconnectedWidget = new DisconnectedWidget(conf)
|
||||
Reference in New Issue
Block a user