Comfy-Org/ComfyUI_frontend
1
0
Fork 0
mirror of https://github.com/Comfy-Org/ComfyUI_frontend.git synced 2026-05-24 06:35:10 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix: wrap configure() widget restoration in hydration transaction (#10201)

Browse Source 
Wraps `LGraphNode.configure()`'s widget-value restoration phase in a
hydration transaction (`beginHydration` / `commitHydration`), preventing
derived-state callbacks (e.g. CustomCombo's `updateCombo`) from firing
before all widget values are restored.

```mermaid
flowchart LR
    subgraph core["Core Layer"]
        LGN["LGraphNode.configure()"]
        WVS["widgetValueStore"]
    end

    subgraph extensions["Extension Layer"]
        CW["CustomCombo\n(customWidgets.ts)"]
        PN["PrimitiveNode\n(widgetInputs.ts)"]
    end

    LGN -- "beginHydration /\ncommitHydration" --> WVS
    CW -- "isHydrating() guard" --> WVS
    CW -- "onHydrationComplete()" --> WVS
    PN -- "serialize() override\npreserves widgets_values" --> LGN
```

```mermaid
sequenceDiagram
    participant Caller as Paste / Load
    participant Node as LGraphNode.configure()
    participant Store as widgetValueStore
    participant Widget as Widget Setters
    participant Combo as CustomCombo.updateCombo

    Caller->>Node: configure(serializedData)
    Node->>Store: beginHydration(nodeId)
    activate Store
    Note over Store: hydratingNodes.add(nodeId)

    loop For each widget
        Node->>Widget: widget.value = restored_value
        Widget->>Combo: updateCombo() triggered
        Combo->>Store: isHydrating(nodeId)?
        Store-->>Combo: true - early return
        Note over Combo: Side effect suppressed
    end

    Node->>Node: onConfigure(info)
    Note over Node: Extensions register onHydrationComplete callbacks

    Node->>Store: commitHydration(nodeId)
    Note over Store: hydratingNodes.delete(nodeId)
    Store->>Combo: fire queued callbacks
    Note over Combo: updateCombo() runs once with all values present
    deactivate Store
```

```mermaid
flowchart TB
    subgraph before["Before: Race Condition"]
        direction TB
        B1["configure starts"] --> B2["widget1.value = optionA"]
        B2 --> B3["updateCombo fires immediately"]
        B3 --> B4["values list incomplete,\ncomboWidget.value reset"]
        B4 --> B5["widget2.value = optionB"]
        B5 --> B6["updateCombo fires again"]
        B6 --> B7["Combo has wrong value"]
    end

    subgraph after["After: Hydration Transaction"]
        direction TB
        A1["configure starts"] --> A2["beginHydration nodeId"]
        A2 --> A3["widget1.value = optionA"]
        A3 --> A4["updateCombo skipped"]
        A4 --> A5["widget2.value = optionB"]
        A5 --> A6["updateCombo skipped"]
        A6 --> A7["commitHydration nodeId"]
        A7 --> A8["updateCombo runs once,\nall values present"]
    end
```

- **`LGraphNode.configure()`**: Wraps widget restoration + `onConfigure`
in `beginHydration`/`commitHydration` with `try/finally` for exception
safety
- **`PrimitiveNode.serialize()`**: Adds override to preserve
`widgets_values` when widgets are dynamically disconnected
- **`customWidgets.ts`**: Simplifies CustomCombo's `onConfigure` to use
`onHydrationComplete()` instead of manually managing hydration state
- **3 new tests**: Hydration transaction wrapping, `onHydrationComplete`
callback firing, and exception safety

Stacked on #10010. Implements Design A from the widget state
architecture RFC.
This commit is contained in:
Christian Byrne
2026-03-25 21:16:32 -07:00
committed by Connor Byrne
parent 4f8a74c46c
commit 37d4fa96d2
8 changed files with 297 additions and 39 deletions
Download Patch File Download Diff File Expand all files Collapse all files

8
docs/adr/0006-primitive-node-copy-paste-lifecycle.md
View File

@@ -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.

21
src/extensions/core/customWidgets.ts
View File

@@ -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) {

10
src/extensions/core/widgetInputs.ts
View File

@@ -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()

71
src/lib/litegraph/src/LGraphNode.test.ts
View File

@@ -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<string, unknown>()
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

75
src/lib/litegraph/src/LGraphNode.ts
View File

@@ -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)
}
/**

4
src/lib/litegraph/src/subgraph/SubgraphNode.serialize.test.ts
View File

@@ -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()

89
src/stores/widgetValueStore.test.ts
View File

@@ -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()

58
src/stores/widgetValueStore.ts
View File

@@ -34,8 +34,12 @@ export interface WidgetState<
nodeId: NodeId
}
type HydrationCallback = () => void
export const useWidgetValueStore = defineStore('widgetValue', () => {
const graphWidgetStates = ref(new Map<UUID, Map<WidgetKey, WidgetState>>())
const hydratingNodes = new Set<NodeId>()
const hydrationCallbacks = new Map<NodeId, HydrationCallback[]>()
function getWidgetStateMap(graphId: UUID): Map<WidgetKey, WidgetState> {
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<TValue>
}
@@ -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
}
})