Skip to content

Commit a7c2115

Browse files
DrJKLampcode-comgithub-actionsactions-user
authored
feat: add WidgetValueStore for centralized widget value management (#8594)
## Summary Implements Phase 1 of the **Vue-owns-truth** pattern for widget values. Widget values are now canonical in a Pinia store; `widget.value` delegates to the store while preserving full backward compatibility. ## Changes - **New store**: `src/stores/widgetValueStore.ts` - centralized widget value storage with `get/set/remove/removeNode` API - **BaseWidget integration**: `widget.value` getter/setter now delegates to store when widget is associated with a node - **LGraphNode wiring**: `addCustomWidget()` automatically calls `widget.setNodeId(this.id)` to wire widgets to their nodes - **Test fixes**: Added Pinia setup to test files that use widgets ## Why This foundation enables: - Vue components to reactively bind to widget values via `computed(() => store.get(...))` - Future Yjs/CRDT backing for real-time collaboration - Cleaner separation between Vue state and LiteGraph rendering ## Backward Compatibility | Extension Pattern | Status | |-------------------|--------| | `widget.value = x` | ✅ Works unchanged | | `node.widgets[i].value` | ✅ Works unchanged | | `widget.callback` | ✅ Still fires | | `node.onWidgetChanged` | ✅ Still fires | ## Testing - ✅ 4252 unit tests pass - ✅ Build succeeds ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-8594-feat-add-WidgetValueStore-for-centralized-widget-value-management-2fc6d73d36508160886fcb9f3ebd941e) by [Unito](https://www.unito.io) --------- Co-authored-by: Amp <amp@ampcode.com> Co-authored-by: github-actions <github-actions@github.com> Co-authored-by: GitHub Action <action@github.com>
1 parent d044bed commit a7c2115

File tree

36 files changed

+809
-181
lines changed

36 files changed

+809
-181
lines changed

browser_tests/tests/subgraph-duplicate-ids.spec.ts

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -5,13 +5,6 @@ import { comfyPageFixture as test } from '../fixtures/ComfyPage'
55
test.describe('Subgraph duplicate ID remapping', { tag: ['@subgraph'] }, () => {
66
const WORKFLOW = 'subgraphs/subgraph-nested-duplicate-ids'
77

8-
test.beforeEach(async ({ comfyPage }) => {
9-
await comfyPage.settings.setSetting(
10-
'Comfy.Graph.DeduplicateSubgraphNodeIds',
11-
true
12-
)
13-
})
14-
158
test('All node IDs are globally unique after loading', async ({
169
comfyPage
1710
}) => {

browser_tests/tests/subgraph.spec.ts

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,10 @@ test.describe('Subgraph Operations', { tag: ['@slow', '@subgraph'] }, () => {
6161
await subgraphNode.navigateIntoSubgraph()
6262

6363
const initialCount = await getSubgraphSlotCount(comfyPage, 'inputs')
64-
const vaeEncodeNode = await comfyPage.nodeOps.getNodeRefById('2')
64+
const [vaeEncodeNode] = await comfyPage.nodeOps.getNodeRefsByType(
65+
'VAEEncode',
66+
true
67+
)
6568

6669
await comfyPage.subgraph.connectFromInput(vaeEncodeNode, 0)
6770
await comfyPage.nextFrame()
@@ -77,7 +80,10 @@ test.describe('Subgraph Operations', { tag: ['@slow', '@subgraph'] }, () => {
7780
await subgraphNode.navigateIntoSubgraph()
7881

7982
const initialCount = await getSubgraphSlotCount(comfyPage, 'outputs')
80-
const vaeEncodeNode = await comfyPage.nodeOps.getNodeRefById('2')
83+
const [vaeEncodeNode] = await comfyPage.nodeOps.getNodeRefsByType(
84+
'VAEEncode',
85+
true
86+
)
8187

8288
await comfyPage.subgraph.connectToOutput(vaeEncodeNode, 0)
8389
await comfyPage.nextFrame()
-40 Bytes
Loading

src/components/rightSidePanel/parameters/WidgetItem.vue

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -83,10 +83,7 @@ const favoriteNode = computed(() =>
8383
)
8484
8585
const widgetValue = computed({
86-
get: () => {
87-
widget.vueTrack?.()
88-
return widget.value
89-
},
86+
get: () => widget.value,
9087
set: (newValue: string | number | boolean | object) => {
9188
emit('update:widgetValue', newValue)
9289
}
Lines changed: 53 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1,49 +1,76 @@
11
import { setActivePinia } from 'pinia'
22
import { createTestingPinia } from '@pinia/testing'
3-
import { describe, expect, it, vi } from 'vitest'
4-
import { nextTick, watch } from 'vue'
3+
import { beforeEach, describe, expect, it, vi } from 'vitest'
4+
import { computed, nextTick, watch } from 'vue'
55

66
import { useGraphNodeManager } from '@/composables/graph/useGraphNodeManager'
7-
import { LGraph, LGraphNode } from '@/lib/litegraph/src/litegraph'
7+
import { BaseWidget, LGraph, LGraphNode } from '@/lib/litegraph/src/litegraph'
88
import { NodeSlotType } from '@/lib/litegraph/src/types/globalEnums'
9+
import { useWidgetValueStore } from '@/stores/widgetValueStore'
910

10-
setActivePinia(createTestingPinia())
11+
describe('Node Reactivity', () => {
12+
beforeEach(() => {
13+
setActivePinia(createTestingPinia({ stubActions: false }))
14+
})
1115

12-
function createTestGraph() {
13-
const graph = new LGraph()
14-
const node = new LGraphNode('test')
15-
node.addInput('input', 'INT')
16-
node.addWidget('number', 'testnum', 2, () => undefined, {})
17-
graph.add(node)
16+
function createTestGraph() {
17+
const graph = new LGraph()
18+
const node = new LGraphNode('test')
19+
node.addInput('input', 'INT')
20+
node.addWidget('number', 'testnum', 2, () => undefined, {})
21+
graph.add(node)
1822

19-
const { vueNodeData } = useGraphNodeManager(graph)
20-
const onReactivityUpdate = vi.fn()
21-
watch(vueNodeData, onReactivityUpdate)
23+
const { vueNodeData } = useGraphNodeManager(graph)
2224

23-
return [node, graph, onReactivityUpdate] as const
24-
}
25+
return { node, graph, vueNodeData }
26+
}
2527

26-
describe('Node Reactivity', () => {
27-
it('should trigger on callback', async () => {
28-
const [node, , onReactivityUpdate] = createTestGraph()
28+
it('widget values are reactive through the store', async () => {
29+
const { node } = createTestGraph()
30+
const store = useWidgetValueStore()
31+
const widget = node.widgets![0]
32+
33+
// Verify widget is a BaseWidget with correct value and node assignment
34+
expect(widget).toBeInstanceOf(BaseWidget)
35+
expect(widget.value).toBe(2)
36+
expect((widget as BaseWidget).node.id).toBe(node.id)
37+
38+
// Initial value should be in store after setNodeId was called
39+
expect(store.getWidget(node.id, 'testnum')?.value).toBe(2)
2940

30-
node.widgets![0].callback!(2)
41+
const onValueChange = vi.fn()
42+
const widgetValue = computed(
43+
() => store.getWidget(node.id, 'testnum')?.value
44+
)
45+
watch(widgetValue, onValueChange)
46+
47+
widget.value = 42
3148
await nextTick()
32-
expect(onReactivityUpdate).toHaveBeenCalledTimes(1)
49+
50+
expect(widgetValue.value).toBe(42)
51+
expect(onValueChange).toHaveBeenCalledTimes(1)
3352
})
3453

35-
it('should remain reactive after a connection is made', async () => {
36-
const [node, graph, onReactivityUpdate] = createTestGraph()
54+
it('widget values remain reactive after a connection is made', async () => {
55+
const { node, graph } = createTestGraph()
56+
const store = useWidgetValueStore()
57+
const onValueChange = vi.fn()
3758

3859
graph.trigger('node:slot-links:changed', {
39-
nodeId: '1',
60+
nodeId: String(node.id),
4061
slotType: NodeSlotType.INPUT
4162
})
4263
await nextTick()
43-
onReactivityUpdate.mockClear()
4464

45-
node.widgets![0].callback!(2)
65+
const widgetValue = computed(
66+
() => store.getWidget(node.id, 'testnum')?.value
67+
)
68+
watch(widgetValue, onValueChange)
69+
70+
node.widgets![0].value = 99
4671
await nextTick()
47-
expect(onReactivityUpdate).toHaveBeenCalledTimes(1)
72+
73+
expect(onValueChange).toHaveBeenCalledTimes(1)
74+
expect(widgetValue.value).toBe(99)
4875
})
4976
})

src/composables/graph/useGraphNodeManager.ts

Lines changed: 52 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -3,18 +3,15 @@
33
* Provides event-driven reactivity with performance optimizations
44
*/
55
import { reactiveComputed } from '@vueuse/core'
6-
import { customRef, reactive, shallowReactive } from 'vue'
6+
import { reactive, shallowReactive } from 'vue'
77

88
import { useChainCallback } from '@/composables/functional/useChainCallback'
99
import { isProxyWidget } from '@/core/graph/subgraph/proxyWidget'
1010
import type {
1111
INodeInputSlot,
1212
INodeOutputSlot
1313
} from '@/lib/litegraph/src/interfaces'
14-
import type {
15-
IBaseWidget,
16-
IWidgetOptions
17-
} from '@/lib/litegraph/src/types/widgets'
14+
import type { IBaseWidget } from '@/lib/litegraph/src/types/widgets'
1815
import { useLayoutMutations } from '@/renderer/core/layout/operations/layoutMutations'
1916
import { LayoutSource } from '@/renderer/core/layout/types'
2017
import type { NodeId } from '@/renderer/core/layout/types'
@@ -41,19 +38,37 @@ export interface WidgetSlotMetadata {
4138
linked: boolean
4239
}
4340

41+
/**
42+
* Minimal render-specific widget data extracted from LiteGraph widgets.
43+
* Value and metadata (label, hidden, disabled, etc.) are accessed via widgetValueStore.
44+
*/
4445
export interface SafeWidgetData {
46+
nodeId?: NodeId
4547
name: string
4648
type: string
47-
value: WidgetValue
48-
borderStyle?: string
49+
/** Callback to invoke when widget value changes (wraps LiteGraph callback + triggerDraw) */
4950
callback?: ((value: unknown) => void) | undefined
51+
/** Control widget for seed randomization/increment/decrement */
5052
controlWidget?: SafeControlWidget
53+
/** Whether widget has custom layout size computation */
5154
hasLayoutSize?: boolean
55+
/** Whether widget is a DOM widget */
5256
isDOMWidget?: boolean
53-
label?: string
57+
/** Node type (for subgraph promoted widgets) */
5458
nodeType?: string
55-
options?: IWidgetOptions
59+
/**
60+
* Widget options needed for render decisions.
61+
* Note: Most metadata should be accessed via widgetValueStore.getWidget().
62+
*/
63+
options?: {
64+
canvasOnly?: boolean
65+
advanced?: boolean
66+
hidden?: boolean
67+
read_only?: boolean
68+
}
69+
/** Input specification from node definition */
5670
spec?: InputSpec
71+
/** Input slot metadata (index and link status) */
5772
slotMetadata?: WidgetSlotMetadata
5873
}
5974

@@ -95,23 +110,6 @@ export interface GraphNodeManager {
95110
cleanup(): void
96111
}
97112

98-
function widgetWithVueTrack(
99-
widget: IBaseWidget
100-
): asserts widget is IBaseWidget & { vueTrack: () => void } {
101-
if (widget.vueTrack) return
102-
103-
customRef((track, trigger) => {
104-
widget.callback = useChainCallback(widget.callback, trigger)
105-
widget.vueTrack = track
106-
return { get() {}, set() {} }
107-
})
108-
}
109-
function useReactiveWidgetValue(widget: IBaseWidget) {
110-
widgetWithVueTrack(widget)
111-
widget.vueTrack()
112-
return widget.value
113-
}
114-
115113
function getControlWidget(widget: IBaseWidget): SafeControlWidget | undefined {
116114
const cagWidget = widget.linkedWidgets?.find(
117115
(w) => w.name == 'control_after_generate'
@@ -133,26 +131,18 @@ function getNodeType(node: LGraphNode, widget: IBaseWidget) {
133131
* Shared widget enhancements used by both safeWidgetMapper and Right Side Panel
134132
*/
135133
interface SharedWidgetEnhancements {
136-
/** Reactive widget value that updates when the widget changes */
137-
value: WidgetValue
138134
/** Control widget for seed randomization/increment/decrement */
139135
controlWidget?: SafeControlWidget
140136
/** Input specification from node definition */
141137
spec?: InputSpec
142138
/** Node type (for subgraph promoted widgets) */
143139
nodeType?: string
144-
/** Border style for promoted/advanced widgets */
145-
borderStyle?: string
146-
/** Widget label */
147-
label?: string
148-
/** Widget options */
149-
options?: IWidgetOptions
150140
}
151141

152142
/**
153143
* Extracts common widget enhancements shared across different rendering contexts.
154-
* This function centralizes the logic for extracting metadata and reactive values
155-
* from widgets, ensuring consistency between Nodes 2.0 and Right Side Panel.
144+
* This function centralizes the logic for extracting metadata from widgets.
145+
* Note: Value and metadata (label, options, hidden, etc.) are accessed via widgetValueStore.
156146
*/
157147
export function getSharedWidgetEnhancements(
158148
node: LGraphNode,
@@ -161,17 +151,9 @@ export function getSharedWidgetEnhancements(
161151
const nodeDefStore = useNodeDefStore()
162152

163153
return {
164-
value: useReactiveWidgetValue(widget),
165154
controlWidget: getControlWidget(widget),
166155
spec: nodeDefStore.getInputSpecForWidget(node, widget.name),
167-
nodeType: getNodeType(node, widget),
168-
borderStyle: widget.promoted
169-
? 'ring ring-component-node-widget-promoted'
170-
: widget.advanced
171-
? 'ring ring-component-node-widget-advanced'
172-
: undefined,
173-
label: widget.label,
174-
options: widget.options as IWidgetOptions
156+
nodeType: getNodeType(node, widget)
175157
}
176158
}
177159

@@ -212,7 +194,7 @@ function safeWidgetMapper(
212194
): (widget: IBaseWidget) => SafeWidgetData {
213195
return function (widget) {
214196
try {
215-
// Get shared enhancements used by both Nodes 2.0 and Right Side Panel
197+
// Get shared enhancements (controlWidget, spec, nodeType)
216198
const sharedEnhancements = getSharedWidgetEnhancements(node, widget)
217199
const slotInfo = slotMetadata.get(widget.name)
218200

@@ -228,20 +210,41 @@ function safeWidgetMapper(
228210
node.widgets?.forEach((w) => w.triggerDraw?.())
229211
}
230212

213+
// Extract only render-critical options (canvasOnly, advanced, read_only)
214+
const options = widget.options
215+
? {
216+
canvasOnly: widget.options.canvasOnly,
217+
advanced: widget.advanced,
218+
hidden: widget.options.hidden,
219+
read_only: widget.options.read_only
220+
}
221+
: undefined
222+
const subgraphId = node.isSubgraphNode() && node.subgraph.id
223+
224+
const localId = isProxyWidget(widget)
225+
? widget._overlay?.nodeId
226+
: undefined
227+
const nodeId =
228+
subgraphId && localId ? `${subgraphId}:${localId}` : undefined
229+
const name = isProxyWidget(widget)
230+
? widget._overlay.widgetName
231+
: widget.name
232+
231233
return {
232-
name: widget.name,
234+
nodeId,
235+
name,
233236
type: widget.type,
234237
...sharedEnhancements,
235238
callback,
236239
hasLayoutSize: typeof widget.computeLayoutSize === 'function',
237240
isDOMWidget: isDOMWidget(widget),
241+
options,
238242
slotMetadata: slotInfo
239243
}
240244
} catch (error) {
241245
return {
242246
name: widget.name || 'unknown',
243-
type: widget.type || 'text',
244-
value: undefined
247+
type: widget.type || 'text'
245248
}
246249
}
247250
}

src/core/graph/subgraph/proxyWidget.test.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
1-
import { describe, expect, test, vi } from 'vitest'
1+
import { createTestingPinia } from '@pinia/testing'
2+
import { setActivePinia } from 'pinia'
3+
import { beforeEach, describe, expect, test, vi } from 'vitest'
24

35
import { registerProxyWidgets } from '@/core/graph/subgraph/proxyWidget'
46
import { promoteWidget } from '@/core/graph/subgraph/proxyWidgetUtils'
@@ -43,6 +45,10 @@ function setupSubgraph(
4345
}
4446

4547
describe('Subgraph proxyWidgets', () => {
48+
beforeEach(() => {
49+
setActivePinia(createTestingPinia({ stubActions: false }))
50+
})
51+
4652
test('Can add simple widget', () => {
4753
const [subgraphNode, innerNodes] = setupSubgraph(1)
4854
innerNodes[0].addWidget('text', 'stringWidget', 'value', () => {})

src/core/graph/subgraph/proxyWidget.ts

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -115,9 +115,11 @@ const onConfigure = function (
115115
if (isActiveGraph && w instanceof DOMWidgetImpl) setWidget(w)
116116
return [w]
117117
})
118-
this.widgets = this.widgets.filter(
119-
(w) => !isProxyWidget(w) && !parsed.some(([, name]) => w.name === name)
120-
)
118+
this.widgets = this.widgets.filter((w) => {
119+
if (isProxyWidget(w)) return false
120+
const widgetName = w.name
121+
return !parsed.some(([, name]) => widgetName === name)
122+
})
121123
this.widgets.push(...newWidgets)
122124

123125
canvasStore.canvas?.setDirty(true, true)
@@ -152,6 +154,7 @@ function newProxyWidget(
152154
computedHeight: undefined,
153155
isProxyWidget: true,
154156
last_y: undefined,
157+
label: name,
155158
name,
156159
node: subgraphNode,
157160
onRemove: undefined,

0 commit comments

Comments
 (0)