From ccc8d6e441c50528138751d7c41d66685572d7a2 Mon Sep 17 00:00:00 2001 From: Austin Mroz Date: Mon, 15 Sep 2025 15:44:15 -0500 Subject: [PATCH] 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) --- src/components/selectionbar/SubgraphNode.vue | 38 +++++++++---------- src/extensions/core/proxyWidget.ts | 39 ++++++++++++++++++-- 2 files changed, 54 insertions(+), 23 deletions(-) diff --git a/src/components/selectionbar/SubgraphNode.vue b/src/components/selectionbar/SubgraphNode.vue index fb894f0d2..1b6cd0c9d 100644 --- a/src/components/selectionbar/SubgraphNode.vue +++ b/src/components/selectionbar/SubgraphNode.vue @@ -6,12 +6,15 @@ import draggable from 'vuedraggable' import SearchBox from '@/components/common/SearchBox.vue' import SubgraphNodeWidget from '@/components/selectionbar/SubgraphNodeWidget.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 { SubgraphNode } from '@/lib/litegraph/src/subgraph/SubgraphNode' import type { IBaseWidget } from '@/lib/litegraph/src/types/widgets' import { useCanvasStore } from '@/stores/graphStore' -type ProxyWidgets = [string, string][] type WidgetItem = [LGraphNode, IBaseWidget] const { t } = useI18n() @@ -37,7 +40,7 @@ const activeWidgets = computed({ if (triggerUpdate.value < 0) console.log('unreachable') const node = activeNode.value if (!node) return [] - const pw = node.properties.proxyWidgets as ProxyWidgets + const pw = parseProxyWidgets(node.properties.proxyWidgets) return pw.flatMap(([id, name]: [string, string]) => { const wNode = node.subgraph._nodes_by_id[id] if (!wNode?.widgets) return [] @@ -51,14 +54,13 @@ const activeWidgets = computed({ if (!node) throw new Error('Attempted to toggle widgets with no node selected') //map back to id/name - const pw: ProxyWidgets = value.map(([node, widget]) => [ + const pw: ProxyWidgetsProperty = value.map(([node, widget]) => [ `${node.id}`, widget.name ]) - node.properties.proxyWidgets = pw as unknown as string + node.properties.proxyWidgets = JSON.stringify(pw) //force trigger an update triggerUpdate.value++ - canvasStore.getCanvas().setDirty(true, true) } }) function toggleVisibility( @@ -70,19 +72,17 @@ function toggleVisibility( if (!node) throw new Error('Attempted to toggle widgets with no node selected') if (!isShown) { - const proxyWidgets: ProxyWidgets = node.properties - .proxyWidgets as ProxyWidgets + const proxyWidgets = parseProxyWidgets(node.properties.proxyWidgets) proxyWidgets.push([nodeId, widgetName]) - node.properties.proxyWidgets = proxyWidgets as unknown as string + node.properties.proxyWidgets = JSON.stringify(proxyWidgets) } else { - let pw = node.properties.proxyWidgets as ProxyWidgets + let pw = parseProxyWidgets(node.properties.proxyWidgets) pw = pw.filter( (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++ - useCanvasStore().getCanvas().setDirty(true, true) } function nodeWidgets(n: LGraphNode): WidgetItem[] { @@ -94,7 +94,7 @@ const candidateWidgets = computed(() => { const node = activeNode.value if (!node) return [] 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 //node.widgets ??= [] const allWidgets: WidgetItem[] = interiorNodes.flatMap(nodeWidgets) @@ -119,13 +119,12 @@ const filteredCandidates = computed(() => { function showAll() { const node = activeNode.value if (!node) return //Not reachable - const pw = node.properties.proxyWidgets as ProxyWidgets - const toAdd: ProxyWidgets = filteredCandidates.value.map( + const pw = parseProxyWidgets(node.properties.proxyWidgets) + const toAdd: ProxyWidgetsProperty = filteredCandidates.value.map( ([n, w]: WidgetItem) => [`${n.id}`, w.name] ) pw.push(...toAdd) - node.properties.proxyWidgets = pw - useCanvasStore().getCanvas().setDirty(true, true) + node.properties.proxyWidgets = JSON.stringify(pw) triggerUpdate.value++ } function hideAll() { @@ -133,14 +132,15 @@ function hideAll() { if (!node) return //Not reachable //Not great from a nesting perspective, but path is cold //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]) => !filteredActive.value.some( ([n, w]: WidgetItem) => n.id == nodeId && w.name === widgetName ) ) - node.properties.proxyWidgets = toKeep - useCanvasStore().getCanvas().setDirty(true, true) + node.properties.proxyWidgets = JSON.stringify(toKeep) triggerUpdate.value++ } diff --git a/src/extensions/core/proxyWidget.ts b/src/extensions/core/proxyWidget.ts index 29cccf5a5..615566bc7 100644 --- a/src/extensions/core/proxyWidget.ts +++ b/src/extensions/core/proxyWidget.ts @@ -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 { SubgraphNode } from '@/lib/litegraph/src/subgraph/SubgraphNode' 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 { useExtensionService } from '@/services/extensionService' 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 +//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({ name: 'Comfy.SubgraphProxyWidgets', @@ -19,11 +48,13 @@ function injectProperty(subgraphNode: SubgraphNode) { const proxyWidgets = subgraphNode.properties.proxyWidgets Object.defineProperty(subgraphNode.properties, 'proxyWidgets', { get: () => { - return subgraphNode.widgets + const result = subgraphNode.widgets .filter((w) => isProxyWidget(w)) .map((w) => [w._overlay.nodeId, w._overlay.widgetName]) + return JSON.stringify(result) }, - set: (property) => { + set: (property: string) => { + const parsed = parseProxyWidgets(property) const { widgetStates } = useDomWidgetStore() for (const w of subgraphNode.widgets ?? []) { if (w instanceof DOMWidgetImpl && widgetStates.has(w.id)) { @@ -36,7 +67,7 @@ function injectProperty(subgraphNode: SubgraphNode) { subgraphNode.widgets = subgraphNode.widgets.filter( (w) => !isProxyWidget(w) ) - for (const [nodeId, widgetName] of property) { + for (const [nodeId, widgetName] of parsed) { const w = addProxyWidget(subgraphNode, `${nodeId}`, widgetName) if (w instanceof DOMWidgetImpl) { const widgetState = widgetStates.get(w.id) @@ -45,7 +76,7 @@ function injectProperty(subgraphNode: SubgraphNode) { widgetState.widget = w } } - //TODO: set dirty canvas + canvasStore.canvas?.setDirty(true, true) } }) subgraphNode.properties.proxyWidgets = proxyWidgets