serialize to string at proxyWidgets boundry

While node.properties function with anything serializeable, the format
for proxyWidgets is not a valid option for type. After great
consideration, all access to and from this value goes through a JSON
serialization and parsing always includes a zod validation step.

This is sturdier to outside misuse, has even lower risk of custom node
breakage, and means that there's now proper type checking at the
boundries of interaction.

Performance was a major concern against this, but the path is quite
cold. I estimate the value of optimization here to be 3-4 orders of
magnitude less important than anything occuring during the draw loop
(like access to proxyWidget elements)
This commit is contained in:
Austin Mroz
2025-09-15 15:44:15 -05:00
parent 166cdc385c
commit ccc8d6e441
2 changed files with 54 additions and 23 deletions

View File

@@ -6,12 +6,15 @@ import draggable from 'vuedraggable'
import SearchBox from '@/components/common/SearchBox.vue' import SearchBox from '@/components/common/SearchBox.vue'
import SubgraphNodeWidget from '@/components/selectionbar/SubgraphNodeWidget.vue' import SubgraphNodeWidget from '@/components/selectionbar/SubgraphNodeWidget.vue'
import SidebarTabTemplate from '@/components/sidebar/tabs/SidebarTabTemplate.vue' import SidebarTabTemplate from '@/components/sidebar/tabs/SidebarTabTemplate.vue'
import {
type ProxyWidgetsProperty,
parseProxyWidgets
} from '@/extensions/core/proxyWidget'
import type { LGraphNode } from '@/lib/litegraph/src/litegraph' import type { LGraphNode } from '@/lib/litegraph/src/litegraph'
import { SubgraphNode } from '@/lib/litegraph/src/subgraph/SubgraphNode' import { SubgraphNode } from '@/lib/litegraph/src/subgraph/SubgraphNode'
import type { IBaseWidget } from '@/lib/litegraph/src/types/widgets' import type { IBaseWidget } from '@/lib/litegraph/src/types/widgets'
import { useCanvasStore } from '@/stores/graphStore' import { useCanvasStore } from '@/stores/graphStore'
type ProxyWidgets = [string, string][]
type WidgetItem = [LGraphNode, IBaseWidget] type WidgetItem = [LGraphNode, IBaseWidget]
const { t } = useI18n() const { t } = useI18n()
@@ -37,7 +40,7 @@ const activeWidgets = computed<WidgetItem[]>({
if (triggerUpdate.value < 0) console.log('unreachable') if (triggerUpdate.value < 0) console.log('unreachable')
const node = activeNode.value const node = activeNode.value
if (!node) return [] if (!node) return []
const pw = node.properties.proxyWidgets as ProxyWidgets const pw = parseProxyWidgets(node.properties.proxyWidgets)
return pw.flatMap(([id, name]: [string, string]) => { return pw.flatMap(([id, name]: [string, string]) => {
const wNode = node.subgraph._nodes_by_id[id] const wNode = node.subgraph._nodes_by_id[id]
if (!wNode?.widgets) return [] if (!wNode?.widgets) return []
@@ -51,14 +54,13 @@ const activeWidgets = computed<WidgetItem[]>({
if (!node) if (!node)
throw new Error('Attempted to toggle widgets with no node selected') throw new Error('Attempted to toggle widgets with no node selected')
//map back to id/name //map back to id/name
const pw: ProxyWidgets = value.map(([node, widget]) => [ const pw: ProxyWidgetsProperty = value.map(([node, widget]) => [
`${node.id}`, `${node.id}`,
widget.name widget.name
]) ])
node.properties.proxyWidgets = pw as unknown as string node.properties.proxyWidgets = JSON.stringify(pw)
//force trigger an update //force trigger an update
triggerUpdate.value++ triggerUpdate.value++
canvasStore.getCanvas().setDirty(true, true)
} }
}) })
function toggleVisibility( function toggleVisibility(
@@ -70,19 +72,17 @@ function toggleVisibility(
if (!node) if (!node)
throw new Error('Attempted to toggle widgets with no node selected') throw new Error('Attempted to toggle widgets with no node selected')
if (!isShown) { if (!isShown) {
const proxyWidgets: ProxyWidgets = node.properties const proxyWidgets = parseProxyWidgets(node.properties.proxyWidgets)
.proxyWidgets as ProxyWidgets
proxyWidgets.push([nodeId, widgetName]) proxyWidgets.push([nodeId, widgetName])
node.properties.proxyWidgets = proxyWidgets as unknown as string node.properties.proxyWidgets = JSON.stringify(proxyWidgets)
} else { } else {
let pw = node.properties.proxyWidgets as ProxyWidgets let pw = parseProxyWidgets(node.properties.proxyWidgets)
pw = pw.filter( pw = pw.filter(
(p: [string, string]) => p[1] !== widgetName || p[0] !== nodeId (p: [string, string]) => p[1] !== widgetName || p[0] !== nodeId
) )
node.properties.proxyWidgets = pw as unknown as string node.properties.proxyWidgets = JSON.stringify(pw)
} }
triggerUpdate.value++ triggerUpdate.value++
useCanvasStore().getCanvas().setDirty(true, true)
} }
function nodeWidgets(n: LGraphNode): WidgetItem[] { function nodeWidgets(n: LGraphNode): WidgetItem[] {
@@ -94,7 +94,7 @@ const candidateWidgets = computed<WidgetItem[]>(() => {
const node = activeNode.value const node = activeNode.value
if (!node) return [] if (!node) return []
if (triggerUpdate.value < 0) console.log('unreachable') if (triggerUpdate.value < 0) console.log('unreachable')
const pw = node.properties.proxyWidgets as ProxyWidgets const pw = parseProxyWidgets(node.properties.proxyWidgets)
const interiorNodes = node.subgraph.nodes const interiorNodes = node.subgraph.nodes
//node.widgets ??= [] //node.widgets ??= []
const allWidgets: WidgetItem[] = interiorNodes.flatMap(nodeWidgets) const allWidgets: WidgetItem[] = interiorNodes.flatMap(nodeWidgets)
@@ -119,13 +119,12 @@ const filteredCandidates = computed<WidgetItem[]>(() => {
function showAll() { function showAll() {
const node = activeNode.value const node = activeNode.value
if (!node) return //Not reachable if (!node) return //Not reachable
const pw = node.properties.proxyWidgets as ProxyWidgets const pw = parseProxyWidgets(node.properties.proxyWidgets)
const toAdd: ProxyWidgets = filteredCandidates.value.map( const toAdd: ProxyWidgetsProperty = filteredCandidates.value.map(
([n, w]: WidgetItem) => [`${n.id}`, w.name] ([n, w]: WidgetItem) => [`${n.id}`, w.name]
) )
pw.push(...toAdd) pw.push(...toAdd)
node.properties.proxyWidgets = pw node.properties.proxyWidgets = JSON.stringify(pw)
useCanvasStore().getCanvas().setDirty(true, true)
triggerUpdate.value++ triggerUpdate.value++
} }
function hideAll() { function hideAll() {
@@ -133,14 +132,15 @@ function hideAll() {
if (!node) return //Not reachable if (!node) return //Not reachable
//Not great from a nesting perspective, but path is cold //Not great from a nesting perspective, but path is cold
//and it cleans up potential error states //and it cleans up potential error states
const toKeep = (node.properties.proxyWidgets as ProxyWidgets).filter( const toKeep: ProxyWidgetsProperty = parseProxyWidgets(
node.properties.proxyWidgets
).filter(
([nodeId, widgetName]) => ([nodeId, widgetName]) =>
!filteredActive.value.some( !filteredActive.value.some(
([n, w]: WidgetItem) => n.id == nodeId && w.name === widgetName ([n, w]: WidgetItem) => n.id == nodeId && w.name === widgetName
) )
) )
node.properties.proxyWidgets = toKeep node.properties.proxyWidgets = JSON.stringify(toKeep)
useCanvasStore().getCanvas().setDirty(true, true)
triggerUpdate.value++ triggerUpdate.value++
} }

View File

@@ -1,3 +1,7 @@
import { z } from 'zod'
import { fromZodError } from 'zod-validation-error'
import type { NodeProperty } from '@/lib/litegraph/src/LGraphNode'
import type { LGraph, LGraphNode } from '@/lib/litegraph/src/litegraph' import type { LGraph, LGraphNode } from '@/lib/litegraph/src/litegraph'
import { SubgraphNode } from '@/lib/litegraph/src/subgraph/SubgraphNode' import { SubgraphNode } from '@/lib/litegraph/src/subgraph/SubgraphNode'
import type { IBaseWidget } from '@/lib/litegraph/src/types/widgets.ts' import type { IBaseWidget } from '@/lib/litegraph/src/types/widgets.ts'
@@ -5,6 +9,31 @@ import { disconnectedWidget } from '@/lib/litegraph/src/widgets/DisconnectedWidg
import { DOMWidgetImpl } from '@/scripts/domWidget' import { DOMWidgetImpl } from '@/scripts/domWidget'
import { useExtensionService } from '@/services/extensionService' import { useExtensionService } from '@/services/extensionService'
import { useDomWidgetStore } from '@/stores/domWidgetStore' import { useDomWidgetStore } from '@/stores/domWidgetStore'
import { useCanvasStore } from '@/stores/graphStore'
const canvasStore = useCanvasStore()
export const proxyWidgetsPropertySchema = z.array(
z.tuple([z.string(), z.string()])
)
export type ProxyWidgetsProperty = z.infer<typeof proxyWidgetsPropertySchema>
//export type proxyWidgetsProperty = [string, string][]
export function parseProxyWidgets(
property: NodeProperty | undefined
): ProxyWidgetsProperty {
if (typeof property !== 'string') {
console.error(`Found non-string value for properties.proxyWidgets`)
return []
}
const parsed = JSON.parse(property)
const result = proxyWidgetsPropertySchema.safeParse(parsed)
if (result.success) return result.data ?? []
const error = fromZodError(result.error)
console.error(`Invalid assignment for properties.proxyWidgets:\n${error}`)
return []
}
useExtensionService().registerExtension({ useExtensionService().registerExtension({
name: 'Comfy.SubgraphProxyWidgets', name: 'Comfy.SubgraphProxyWidgets',
@@ -19,11 +48,13 @@ function injectProperty(subgraphNode: SubgraphNode) {
const proxyWidgets = subgraphNode.properties.proxyWidgets const proxyWidgets = subgraphNode.properties.proxyWidgets
Object.defineProperty(subgraphNode.properties, 'proxyWidgets', { Object.defineProperty(subgraphNode.properties, 'proxyWidgets', {
get: () => { get: () => {
return subgraphNode.widgets const result = subgraphNode.widgets
.filter((w) => isProxyWidget(w)) .filter((w) => isProxyWidget(w))
.map((w) => [w._overlay.nodeId, w._overlay.widgetName]) .map((w) => [w._overlay.nodeId, w._overlay.widgetName])
return JSON.stringify(result)
}, },
set: (property) => { set: (property: string) => {
const parsed = parseProxyWidgets(property)
const { widgetStates } = useDomWidgetStore() const { widgetStates } = useDomWidgetStore()
for (const w of subgraphNode.widgets ?? []) { for (const w of subgraphNode.widgets ?? []) {
if (w instanceof DOMWidgetImpl && widgetStates.has(w.id)) { if (w instanceof DOMWidgetImpl && widgetStates.has(w.id)) {
@@ -36,7 +67,7 @@ function injectProperty(subgraphNode: SubgraphNode) {
subgraphNode.widgets = subgraphNode.widgets.filter( subgraphNode.widgets = subgraphNode.widgets.filter(
(w) => !isProxyWidget(w) (w) => !isProxyWidget(w)
) )
for (const [nodeId, widgetName] of property) { for (const [nodeId, widgetName] of parsed) {
const w = addProxyWidget(subgraphNode, `${nodeId}`, widgetName) const w = addProxyWidget(subgraphNode, `${nodeId}`, widgetName)
if (w instanceof DOMWidgetImpl) { if (w instanceof DOMWidgetImpl) {
const widgetState = widgetStates.get(w.id) const widgetState = widgetStates.get(w.id)
@@ -45,7 +76,7 @@ function injectProperty(subgraphNode: SubgraphNode) {
widgetState.widget = w widgetState.widget = w
} }
} }
//TODO: set dirty canvas canvasStore.canvas?.setDirty(true, true)
} }
}) })
subgraphNode.properties.proxyWidgets = proxyWidgets subgraphNode.properties.proxyWidgets = proxyWidgets