diff --git a/docs/adr/0006-primitive-node-copy-paste-lifecycle.md b/docs/adr/0006-primitive-node-copy-paste-lifecycle.md index e8025d910a..257db6b1c6 100644 --- a/docs/adr/0006-primitive-node-copy-paste-lifecycle.md +++ b/docs/adr/0006-primitive-node-copy-paste-lifecycle.md @@ -4,7 +4,7 @@ Date: 2026-02-22 ## Status -Proposed +Accepted (Option A) ## Context @@ -42,4 +42,8 @@ Primitives act as a synchronization mechanism — no own state, just a projectio ## Decision -Pending. Option A is the most pragmatic first step. Option B can be revisited after Option A ships and stabilizes. +Option A. Override `serialize()` on PrimitiveNode to preserve `widgets_values` through copy-paste. This is the lowest-risk fix with no change to connection lifecycle semantics. + +Prerequisite: PR [#10010](https://github.com/Comfy-Org/ComfyUI_frontend/pull/10010) replaced `clone().serialize()` with direct serialization in `_serializeItems`, eliminating the code path that dropped `widgets_values` for widget-less clones. Option A provides the PrimitiveNode-specific fallback for any remaining edge cases. + +Option B can be revisited after Option A ships and stabilizes. diff --git a/src/extensions/core/customWidgets.ts b/src/extensions/core/customWidgets.ts index 1908c88647..bc2eabb70f 100644 --- a/src/extensions/core/customWidgets.ts +++ b/src/extensions/core/customWidgets.ts @@ -29,6 +29,7 @@ function onCustomComboCreated(this: LGraphNode) { ).map((w) => `${w.value}`) ) if (app.configuringGraph || !this.graph) return + if (useWidgetValueStore().isHydrating(this.id)) return if (values.includes(`${comboWidget.value}`)) return comboWidget.value = values[0] ?? '' comboWidget.callback?.(comboWidget.value) @@ -57,12 +58,17 @@ function onCustomComboCreated(this: LGraphNode) { }, set(v: string) { localValue = v - const state = useWidgetValueStore().getWidget( + const store = useWidgetValueStore() + + store.getOrCreateWidget( app.rootGraph.id, node.id, - widgetName - ) - if (state) state.value = v + widgetName, + v + ).value = v + + if (store.isHydrating(node.id)) return + updateCombo() if (!node.widgets) return const lastWidget = node.widgets.at(-1) @@ -91,6 +97,13 @@ function onCustomComboCreated(this: LGraphNode) { y: 0 }) addOption(this) + + this.onConfigure = useChainCallback( + this.onConfigure, + function (this: LGraphNode) { + useWidgetValueStore().onHydrationComplete(this.id, updateCombo) + } + ) } function onCustomIntCreated(this: LGraphNode) { diff --git a/src/extensions/core/widgetInputs.ts b/src/extensions/core/widgetInputs.ts index d78b285725..6d4ef637fe 100644 --- a/src/extensions/core/widgetInputs.ts +++ b/src/extensions/core/widgetInputs.ts @@ -71,6 +71,16 @@ export class PrimitiveNode extends LGraphNode { } } + override serialize() { + const o = super.serialize() + // PrimitiveNode creates widgets dynamically on connection. When + // disconnected, this.widgets is empty so the base serialize() omits + // widgets_values. Fall back to the snapshot saved during configure(). + if (!o.widgets_values && this.widgets_values) + o.widgets_values = [...this.widgets_values] + return o + } + override onAfterGraphConfigured() { if (this.outputs[0].links?.length && !this.widgets?.length) { this._onFirstConnection() diff --git a/src/lib/litegraph/src/LGraphNode.test.ts b/src/lib/litegraph/src/LGraphNode.test.ts index 9a692f4c10..fc476e9db8 100644 --- a/src/lib/litegraph/src/LGraphNode.test.ts +++ b/src/lib/litegraph/src/LGraphNode.test.ts @@ -15,6 +15,7 @@ import { NodeInputSlot, NodeOutputSlot } from '@/lib/litegraph/src/litegraph' +import { useWidgetValueStore } from '@/stores/widgetValueStore' import { test } from './__fixtures__/testExtensions' import { createMockLGraphNodeWithArrayBoundingRect } from '@/utils/__tests__/litegraphTestUtils' @@ -589,6 +590,76 @@ describe('LGraphNode', () => { }) }) + describe('configure hydration transaction', () => { + test('wraps widget-value restoration in hydration transaction', () => { + const store = useWidgetValueStore() + const hydrationLog: boolean[] = [] + + const testNode = new LGraphNode('TestNode') + testNode.serialize_widgets = true + testNode.addWidget('number', 'a', 0, null) + testNode.addWidget('number', 'b', 0, null) + + // Spy on widget value setters to record hydration state + const storage = new Map() + for (const widget of testNode.widgets!) { + Object.defineProperty(widget, 'value', { + get: () => storage.get(widget.name), + set(v) { + hydrationLog.push(store.isHydrating(testNode.id)) + storage.set(widget.name, v) + }, + configurable: true + }) + } + + testNode.configure( + getMockISerialisedNode({ + id: 42, + widgets_values: [10, 20] + }) + ) + + // Both widget setters ran while hydration was active + expect(hydrationLog.every(Boolean)).toBe(true) + // Hydration is complete after configure returns + expect(store.isHydrating(42)).toBe(false) + }) + + test('fires onHydrationComplete callbacks after configure', () => { + const store = useWidgetValueStore() + const calls: string[] = [] + + const testNode = new LGraphNode('TestNode') + testNode.serialize_widgets = true + testNode.addWidget('number', 'a', 0, null) + + testNode.onConfigure = function () { + store.onHydrationComplete(this.id, () => calls.push('done')) + } + + testNode.configure( + getMockISerialisedNode({ id: 99, widgets_values: [42] }) + ) + + expect(calls).toEqual(['done']) + }) + + test('commitHydration is safe even if onConfigure throws', () => { + const store = useWidgetValueStore() + + const testNode = new LGraphNode('TestNode') + testNode.onConfigure = () => { + throw new Error('boom') + } + + expect(() => + testNode.configure(getMockISerialisedNode({ id: 7 })) + ).toThrow('boom') + expect(store.isHydrating(7)).toBe(false) + }) + }) + describe('getInputSlotPos', () => { let inputSlot: INodeInputSlot diff --git a/src/lib/litegraph/src/LGraphNode.ts b/src/lib/litegraph/src/LGraphNode.ts index 934239cc7d..fcf1f63c7b 100644 --- a/src/lib/litegraph/src/LGraphNode.ts +++ b/src/lib/litegraph/src/LGraphNode.ts @@ -8,6 +8,9 @@ import { import type { SlotPositionContext } from '@/renderer/core/canvas/litegraph/slotCalculations' import { useLayoutMutations } from '@/renderer/core/layout/operations/layoutMutations' import { LayoutSource } from '@/renderer/core/layout/types' +import { getActivePinia } from 'pinia' + +import { useWidgetValueStore } from '@/stores/widgetValueStore' import { adjustColor } from '@/utils/colorUtil' import type { ColorAdjustOptions } from '@/utils/colorUtil' import { @@ -898,44 +901,54 @@ export class LGraphNode // SubgraphNode callback. this._internalConfigureAfterSlots?.() - if (this.widgets) { - for (const w of this.widgets) { - if (!w) continue + // Hydration transaction: suppress derived-state callbacks (e.g. + // CustomCombo's updateCombo) until all widget values are restored. + // onConfigure handlers may commit early (CustomCombo does); the + // final commitHydration is idempotent in that case. + const store = getActivePinia() ? useWidgetValueStore() : null + store?.beginHydration(this.id) + try { + if (this.widgets) { + for (const w of this.widgets) { + if (!w) continue - const input = this.inputs.find((i) => i.widget?.name === w.name) - if (input?.label) w.label = input.label + const input = this.inputs.find((i) => i.widget?.name === w.name) + if (input?.label) w.label = input.label - if ( - w.options?.property && - this.properties[w.options.property] != undefined - ) - w.value = JSON.parse( - JSON.stringify(this.properties[w.options.property]) + if ( + w.options?.property && + this.properties[w.options.property] != undefined ) - } + w.value = JSON.parse( + JSON.stringify(this.properties[w.options.property]) + ) + } - if (info.widgets_values) { - let i = 0 - for (const widget of this.widgets ?? []) { - if (widget.serialize === false) continue - if (i >= info.widgets_values.length) break - widget.value = info.widgets_values[i++] + if (info.widgets_values) { + let i = 0 + for (const widget of this.widgets ?? []) { + if (widget.serialize === false) continue + if (i >= info.widgets_values.length) break + widget.value = info.widgets_values[i++] + } } } + + // Sync the state of this.resizable. + if (this.pinned) this.resizable = false + + if (this.widgets_up) { + console.warn( + `[LiteGraph] Node type "${this.type}" uses deprecated property "widgets_up". ` + + 'This property is unsupported and will be removed. ' + + 'Use "widgets_start_y" or a custom arrange() override instead.' + ) + } + + this.onConfigure?.(info) + } finally { + store?.commitHydration(this.id) } - - // Sync the state of this.resizable. - if (this.pinned) this.resizable = false - - if (this.widgets_up) { - console.warn( - `[LiteGraph] Node type "${this.type}" uses deprecated property "widgets_up". ` + - 'This property is unsupported and will be removed. ' + - 'Use "widgets_start_y" or a custom arrange() override instead.' - ) - } - - this.onConfigure?.(info) } /** diff --git a/src/lib/litegraph/src/subgraph/SubgraphNode.serialize.test.ts b/src/lib/litegraph/src/subgraph/SubgraphNode.serialize.test.ts index 2123885d01..be3e14a06f 100644 --- a/src/lib/litegraph/src/subgraph/SubgraphNode.serialize.test.ts +++ b/src/lib/litegraph/src/subgraph/SubgraphNode.serialize.test.ts @@ -91,8 +91,8 @@ describe('SubgraphNode.serialize() state isolation (#9976)', () => { // The SubgraphNode should have promotions registered (from _setWidget) const promotions = store.getPromotions(rootGraph.id, subgraphNode.id) expect(promotions).toHaveLength(1) - expect(promotions[0].interiorNodeId).toBe(String(interiorNode.id)) - expect(promotions[0].widgetName).toBe('seed') + expect(promotions[0].sourceNodeId).toBe(String(interiorNode.id)) + expect(promotions[0].sourceWidgetName).toBe('seed') // Serialize — should write proxyWidgets from promotionStore const serialized = subgraphNode.serialize() diff --git a/src/stores/widgetValueStore.test.ts b/src/stores/widgetValueStore.test.ts index 2bcff9f096..3754982b9d 100644 --- a/src/stores/widgetValueStore.test.ts +++ b/src/stores/widgetValueStore.test.ts @@ -155,6 +155,95 @@ describe('useWidgetValueStore', () => { }) }) + describe('getOrCreateWidget', () => { + it('creates a new entry when widget does not exist', () => { + const store = useWidgetValueStore() + const state = store.getOrCreateWidget(graphA, 'node-1', 'option1', 'foo') + + expect(state.nodeId).toBe('node-1') + expect(state.name).toBe('option1') + expect(state.value).toBe('foo') + }) + + it('returns existing entry without overwriting value', () => { + const store = useWidgetValueStore() + store.registerWidget(graphA, widget('node-1', 'option1', 'string', 'bar')) + + const state = store.getOrCreateWidget( + graphA, + 'node-1', + 'option1', + 'should-not-overwrite' + ) + expect(state.value).toBe('bar') + }) + + it('is idempotent — repeated calls return same reactive entry', () => { + const store = useWidgetValueStore() + const first = store.getOrCreateWidget(graphA, 'node-1', 'w', 'a') + const second = store.getOrCreateWidget(graphA, 'node-1', 'w', 'b') + + expect(first).toBe(second) + expect(first.value).toBe('a') + }) + }) + + describe('hydration transactions', () => { + it('beginHydration / isHydrating / commitHydration lifecycle', () => { + const store = useWidgetValueStore() + + expect(store.isHydrating('node-1')).toBe(false) + + store.beginHydration('node-1') + expect(store.isHydrating('node-1')).toBe(true) + expect(store.isHydrating('node-2')).toBe(false) + + store.commitHydration('node-1') + expect(store.isHydrating('node-1')).toBe(false) + }) + + it('commitHydration fires registered callbacks', () => { + const store = useWidgetValueStore() + const calls: string[] = [] + + store.beginHydration('node-1') + store.onHydrationComplete('node-1', () => calls.push('a')) + store.onHydrationComplete('node-1', () => calls.push('b')) + + expect(calls).toHaveLength(0) + + store.commitHydration('node-1') + expect(calls).toEqual(['a', 'b']) + }) + + it('onHydrationComplete fires immediately when not hydrating', () => { + const store = useWidgetValueStore() + const calls: string[] = [] + + store.onHydrationComplete('node-1', () => calls.push('immediate')) + expect(calls).toEqual(['immediate']) + }) + + it('commitHydration is safe to call when not hydrating', () => { + const store = useWidgetValueStore() + expect(() => store.commitHydration('node-1')).not.toThrow() + }) + + it('hydration is node-scoped — independent per node', () => { + const store = useWidgetValueStore() + + store.beginHydration('node-1') + store.beginHydration('node-2') + + store.commitHydration('node-1') + expect(store.isHydrating('node-1')).toBe(false) + expect(store.isHydrating('node-2')).toBe(true) + + store.commitHydration('node-2') + expect(store.isHydrating('node-2')).toBe(false) + }) + }) + describe('graph isolation', () => { it('isolates widget states by graph', () => { const store = useWidgetValueStore() diff --git a/src/stores/widgetValueStore.ts b/src/stores/widgetValueStore.ts index 4ee14bbd47..f82a3306e4 100644 --- a/src/stores/widgetValueStore.ts +++ b/src/stores/widgetValueStore.ts @@ -34,8 +34,12 @@ export interface WidgetState< nodeId: NodeId } +type HydrationCallback = () => void + export const useWidgetValueStore = defineStore('widgetValue', () => { const graphWidgetStates = ref(new Map>()) + const hydratingNodes = new Set() + const hydrationCallbacks = new Map() function getWidgetStateMap(graphId: UUID): Map { const widgetStates = graphWidgetStates.value.get(graphId) @@ -57,6 +61,8 @@ export const useWidgetValueStore = defineStore('widgetValue', () => { const widgetStates = getWidgetStateMap(graphId) const key = makeKey(state.nodeId, state.name) widgetStates.set(key, state) + // Return the reactive proxy from the map (not the raw input) so that + // callers who hold a reference see Vue-tracked mutations. return widgetStates.get(key) as WidgetState } @@ -76,6 +82,53 @@ export const useWidgetValueStore = defineStore('widgetValue', () => { return getWidgetStateMap(graphId).get(makeKey(nodeId, widgetName)) } + function getOrCreateWidget( + graphId: UUID, + nodeId: NodeId, + widgetName: string, + defaultValue?: unknown + ): WidgetState { + const widgetStates = getWidgetStateMap(graphId) + const key = makeKey(nodeId, widgetName) + const existing = widgetStates.get(key) + if (existing) return existing + + const state: WidgetState = { + nodeId, + name: widgetName, + type: 'string', + value: defaultValue, + options: {} + } + widgetStates.set(key, state) + return widgetStates.get(key)! + } + + function beginHydration(nodeId: NodeId): void { + hydratingNodes.add(nodeId) + } + + function commitHydration(nodeId: NodeId): void { + hydratingNodes.delete(nodeId) + const callbacks = hydrationCallbacks.get(nodeId) + if (!callbacks) return + + hydrationCallbacks.delete(nodeId) + for (const cb of callbacks) cb() + } + + function isHydrating(nodeId: NodeId): boolean { + return hydratingNodes.has(nodeId) + } + + function onHydrationComplete(nodeId: NodeId, callback: HydrationCallback) { + if (!hydratingNodes.has(nodeId)) return callback() + + const existing = hydrationCallbacks.get(nodeId) ?? [] + if (!existing.includes(callback)) existing.push(callback) + hydrationCallbacks.set(nodeId, existing) + } + function clearGraph(graphId: UUID): void { graphWidgetStates.value.delete(graphId) } @@ -83,7 +136,12 @@ export const useWidgetValueStore = defineStore('widgetValue', () => { return { registerWidget, getWidget, + getOrCreateWidget, getNodeWidgets, + beginHydration, + commitHydration, + isHydrating, + onHydrationComplete, clearGraph } })