mirror of
https://github.com/Comfy-Org/ComfyUI_frontend.git
synced 2026-04-20 06:20:11 +00:00
fix: stabilize promoted widget instance state (#11011)
## Summary - fix the follow-up regressions introduced while addressing #10849 - keep promoted widget state isolated per `SubgraphNode` instance - preserve live source-widget synchronization after restore and after reconfigure ## Red -> Green Red on `#10849` head (`3a83180`): - editing promoted widget value on fresh instance A could leak into fresh sibling instance B before save/reload - after load, a promoted widget could stay pinned to serialized state instead of reflecting later direct edits to the inner source widget - reconfiguring without `widgets_values` could retain stale `_instanceWidgetValues` and shadow newer shared state Green on this branch: - fresh sibling instances keep distinct promoted widget values before save/reload - restored promoted widgets resync when the inner source widget changes directly - `configure()` without `widgets_values` clears stale per-instance cache ## Changes - capture untouched sibling fallback values before a promoted-widget write mutates shared state - track the last promoted-instance write per shared source so getters can distinguish sibling writes from direct source edits - clear `_instanceWidgetValues` in `configure()` and `onRemoved()` - add regression tests for the three red scenarios above ## Verified - `pnpm exec vitest run src/core/graph/subgraph src/lib/litegraph/src/subgraph` - `28/28` files passed - `427/427` tests passed - `pnpm exec eslint src/core/graph/subgraph/promotedWidgetView.ts src/lib/litegraph/src/subgraph/SubgraphNode.ts src/lib/litegraph/src/subgraph/SubgraphNode.multiInstance.test.ts` - `pnpm exec oxfmt --check src/core/graph/subgraph/promotedWidgetView.ts src/lib/litegraph/src/subgraph/SubgraphNode.ts src/lib/litegraph/src/subgraph/SubgraphNode.multiInstance.test.ts` - `pnpm typecheck:browser` Follow-up to #10849.
This commit is contained in:
@@ -1,3 +1,6 @@
|
||||
import { isEqual } from 'es-toolkit'
|
||||
|
||||
import type { LGraph } from '@/lib/litegraph/src/LGraph'
|
||||
import type { LGraphNode, NodeId } from '@/lib/litegraph/src/LGraphNode'
|
||||
import type { LGraphCanvas } from '@/lib/litegraph/src/LGraphCanvas'
|
||||
import type { CanvasPointer } from '@/lib/litegraph/src/CanvasPointer'
|
||||
@@ -50,6 +53,43 @@ function hasLegacyMouse(widget: IBaseWidget): widget is LegacyMouseWidget {
|
||||
}
|
||||
|
||||
const designTokenCache = new Map<string, string>()
|
||||
const promotedSourceWriteMetaByGraph = new WeakMap<
|
||||
LGraph,
|
||||
Map<string, PromotedSourceWriteMeta>
|
||||
>()
|
||||
|
||||
interface PromotedSourceWriteMeta {
|
||||
value: IBaseWidget['value']
|
||||
writerInstanceId: string
|
||||
}
|
||||
|
||||
function cloneWidgetValue<TValue extends IBaseWidget['value']>(
|
||||
value: TValue
|
||||
): TValue {
|
||||
return value != null && typeof value === 'object'
|
||||
? (JSON.parse(JSON.stringify(value)) as TValue)
|
||||
: value
|
||||
}
|
||||
|
||||
function getPromotedSourceWriteMeta(
|
||||
graph: LGraph,
|
||||
sourceKey: string
|
||||
): PromotedSourceWriteMeta | undefined {
|
||||
return promotedSourceWriteMetaByGraph.get(graph)?.get(sourceKey)
|
||||
}
|
||||
|
||||
function setPromotedSourceWriteMeta(
|
||||
graph: LGraph,
|
||||
sourceKey: string,
|
||||
meta: PromotedSourceWriteMeta
|
||||
): void {
|
||||
let metaBySource = promotedSourceWriteMetaByGraph.get(graph)
|
||||
if (!metaBySource) {
|
||||
metaBySource = new Map<string, PromotedSourceWriteMeta>()
|
||||
promotedSourceWriteMetaByGraph.set(graph, metaBySource)
|
||||
}
|
||||
metaBySource.set(sourceKey, meta)
|
||||
}
|
||||
|
||||
export function createPromotedWidgetView(
|
||||
subgraphNode: SubgraphNode,
|
||||
@@ -164,16 +204,14 @@ class PromotedWidgetView implements IPromotedWidgetView {
|
||||
: `${this.sourceNodeId}:${this.sourceWidgetName}`
|
||||
}
|
||||
|
||||
get value(): IBaseWidget['value'] {
|
||||
const instanceValue = this.subgraphNode._instanceWidgetValues.get(
|
||||
this._instanceKey
|
||||
)
|
||||
if (instanceValue !== undefined)
|
||||
return instanceValue as IBaseWidget['value']
|
||||
private get _sharedSourceKey(): string {
|
||||
return this.disambiguatingSourceNodeId
|
||||
? `${this.subgraphNode.subgraph.id}:${this.sourceNodeId}:${this.sourceWidgetName}:${this.disambiguatingSourceNodeId}`
|
||||
: `${this.subgraphNode.subgraph.id}:${this.sourceNodeId}:${this.sourceWidgetName}`
|
||||
}
|
||||
|
||||
const state = this.getWidgetState()
|
||||
if (state && isWidgetValue(state.value)) return state.value
|
||||
return this.resolveAtHost()?.widget.value
|
||||
get value(): IBaseWidget['value'] {
|
||||
return this.getTrackedValue()
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -186,14 +224,25 @@ class PromotedWidgetView implements IPromotedWidgetView {
|
||||
* so this is the hook that makes multi-instance execution correct.
|
||||
*/
|
||||
serializeValue(): IBaseWidget['value'] {
|
||||
const v = this.subgraphNode._instanceWidgetValues.get(this._instanceKey)
|
||||
if (v !== undefined) return v as IBaseWidget['value']
|
||||
return this.value
|
||||
return this.getTrackedValue()
|
||||
}
|
||||
|
||||
set value(value: IBaseWidget['value']) {
|
||||
this.captureSiblingFallbackValues()
|
||||
|
||||
// Keep per-instance map in sync for execution (graphToPrompt)
|
||||
this.subgraphNode._instanceWidgetValues.set(this._instanceKey, value)
|
||||
this.subgraphNode._instanceWidgetValues.set(
|
||||
this._instanceKey,
|
||||
cloneWidgetValue(value)
|
||||
)
|
||||
setPromotedSourceWriteMeta(
|
||||
this.subgraphNode.rootGraph,
|
||||
this._sharedSourceKey,
|
||||
{
|
||||
value: cloneWidgetValue(value),
|
||||
writerInstanceId: String(this.subgraphNode.id)
|
||||
}
|
||||
)
|
||||
|
||||
const linkedWidgets = this.getLinkedInputWidgets()
|
||||
if (linkedWidgets.length > 0) {
|
||||
@@ -424,6 +473,39 @@ class PromotedWidgetView implements IPromotedWidgetView {
|
||||
return resolved
|
||||
}
|
||||
|
||||
private getTrackedValue(): IBaseWidget['value'] {
|
||||
const instanceValue = this.subgraphNode._instanceWidgetValues.get(
|
||||
this._instanceKey
|
||||
)
|
||||
const sharedValue = this.getSharedValue()
|
||||
|
||||
if (instanceValue === undefined) return sharedValue
|
||||
|
||||
const sourceWriteMeta = getPromotedSourceWriteMeta(
|
||||
this.subgraphNode.rootGraph,
|
||||
this._sharedSourceKey
|
||||
)
|
||||
if (
|
||||
sharedValue !== undefined &&
|
||||
sourceWriteMeta &&
|
||||
!isEqual(sharedValue, sourceWriteMeta.value)
|
||||
) {
|
||||
this.subgraphNode._instanceWidgetValues.set(
|
||||
this._instanceKey,
|
||||
cloneWidgetValue(sharedValue)
|
||||
)
|
||||
return sharedValue
|
||||
}
|
||||
|
||||
return instanceValue as IBaseWidget['value']
|
||||
}
|
||||
|
||||
private getSharedValue(): IBaseWidget['value'] {
|
||||
const state = this.getWidgetState()
|
||||
if (state && isWidgetValue(state.value)) return state.value
|
||||
return this.resolveAtHost()?.widget.value
|
||||
}
|
||||
|
||||
private getWidgetState() {
|
||||
const linkedState = this.getLinkedInputWidgetStates()[0]
|
||||
if (linkedState) return linkedState
|
||||
@@ -490,6 +572,30 @@ class PromotedWidgetView implements IPromotedWidgetView {
|
||||
.filter((state): state is WidgetState => state !== undefined)
|
||||
}
|
||||
|
||||
private captureSiblingFallbackValues(): void {
|
||||
const { rootGraph } = this.subgraphNode
|
||||
|
||||
for (const node of rootGraph.nodes) {
|
||||
if (node === this.subgraphNode || !node.isSubgraphNode()) continue
|
||||
if (node.subgraph.id !== this.subgraphNode.subgraph.id) continue
|
||||
if (node._instanceWidgetValues.has(this._instanceKey)) continue
|
||||
|
||||
const siblingView = node.widgets.find(
|
||||
(widget): widget is IPromotedWidgetView =>
|
||||
isPromotedWidgetView(widget) &&
|
||||
widget.sourceNodeId === this.sourceNodeId &&
|
||||
widget.sourceWidgetName === this.sourceWidgetName &&
|
||||
widget.disambiguatingSourceNodeId === this.disambiguatingSourceNodeId
|
||||
)
|
||||
if (!siblingView) continue
|
||||
|
||||
node._instanceWidgetValues.set(
|
||||
this._instanceKey,
|
||||
cloneWidgetValue(siblingView.value)
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
private getProjectedWidget(resolved: {
|
||||
node: LGraphNode
|
||||
widget: IBaseWidget
|
||||
|
||||
@@ -3,7 +3,7 @@ import { setActivePinia } from 'pinia'
|
||||
import { beforeEach, describe, expect, it } from 'vitest'
|
||||
|
||||
import type { ISlotType } from '@/lib/litegraph/src/litegraph'
|
||||
import { BaseWidget, LGraphNode } from '@/lib/litegraph/src/litegraph'
|
||||
import { LGraphNode } from '@/lib/litegraph/src/litegraph'
|
||||
|
||||
import {
|
||||
createTestSubgraph,
|
||||
@@ -13,23 +13,18 @@ import {
|
||||
|
||||
function createNodeWithWidget(
|
||||
title: string,
|
||||
widgetValue: unknown = 42,
|
||||
widgetValue: number = 42,
|
||||
slotType: ISlotType = 'number'
|
||||
) {
|
||||
const node = new LGraphNode(title)
|
||||
const input = node.addInput('value', slotType)
|
||||
node.addOutput('out', slotType)
|
||||
|
||||
// @ts-expect-error Abstract class instantiation
|
||||
const widget = new BaseWidget({
|
||||
name: 'widget',
|
||||
type: 'number',
|
||||
value: widgetValue,
|
||||
y: 0,
|
||||
options: { min: 0, max: 100, step: 1 },
|
||||
node
|
||||
const widget = node.addWidget('number', 'widget', widgetValue, () => {}, {
|
||||
min: 0,
|
||||
max: 100,
|
||||
step: 1
|
||||
})
|
||||
node.widgets = [widget]
|
||||
input.widget = { name: widget.name }
|
||||
|
||||
return { node, widget, input }
|
||||
@@ -133,6 +128,106 @@ describe('SubgraphNode multi-instance widget isolation', () => {
|
||||
expect(restoredWidget?.serializeValue?.(restoredInstance, 0)).toBe(33)
|
||||
})
|
||||
|
||||
it('keeps fresh sibling instances isolated before save or reload', () => {
|
||||
const subgraph = createTestSubgraph({
|
||||
inputs: [{ name: 'value', type: 'number' }]
|
||||
})
|
||||
|
||||
const { node } = createNodeWithWidget('TestNode', 7)
|
||||
subgraph.add(node)
|
||||
subgraph.inputNode.slots[0].connect(node.inputs[0], node)
|
||||
|
||||
const instance1 = createTestSubgraphNode(subgraph, { id: 401 })
|
||||
const instance2 = createTestSubgraphNode(subgraph, { id: 402 })
|
||||
instance1.graph!.add(instance1)
|
||||
instance2.graph!.add(instance2)
|
||||
|
||||
const widget1 = instance1.widgets?.[0]
|
||||
const widget2 = instance2.widgets?.[0]
|
||||
|
||||
expect(widget1?.value).toBe(7)
|
||||
expect(widget2?.value).toBe(7)
|
||||
|
||||
widget1!.value = 10
|
||||
|
||||
expect(widget1?.value).toBe(10)
|
||||
expect(widget2?.value).toBe(7)
|
||||
expect(widget1?.serializeValue?.(instance1, 0)).toBe(10)
|
||||
expect(widget2?.serializeValue?.(instance2, 0)).toBe(7)
|
||||
})
|
||||
|
||||
it('syncs restored promoted widgets when the inner source widget changes directly', () => {
|
||||
const subgraph = createTestSubgraph({
|
||||
inputs: [{ name: 'value', type: 'number' }]
|
||||
})
|
||||
|
||||
const { node, widget } = createNodeWithWidget('TestNode', 0)
|
||||
subgraph.add(node)
|
||||
subgraph.inputNode.slots[0].connect(node.inputs[0], node)
|
||||
|
||||
const originalInstance = createTestSubgraphNode(subgraph, { id: 601 })
|
||||
originalInstance.configure({
|
||||
id: 601,
|
||||
type: subgraph.id,
|
||||
pos: [100, 100],
|
||||
size: [200, 100],
|
||||
inputs: [],
|
||||
outputs: [],
|
||||
mode: 0,
|
||||
order: 0,
|
||||
flags: {},
|
||||
properties: { proxyWidgets: [['-1', 'widget']] },
|
||||
widgets_values: [33]
|
||||
})
|
||||
|
||||
const serialized = originalInstance.serialize()
|
||||
|
||||
const restoredInstance = createTestSubgraphNode(subgraph, { id: 602 })
|
||||
restoredInstance.configure({
|
||||
...serialized,
|
||||
id: 602,
|
||||
type: subgraph.id
|
||||
})
|
||||
|
||||
expect(restoredInstance.widgets?.[0].value).toBe(33)
|
||||
|
||||
widget.value = 45
|
||||
|
||||
expect(restoredInstance.widgets?.[0].value).toBe(45)
|
||||
expect(
|
||||
restoredInstance.widgets?.[0].serializeValue?.(restoredInstance, 0)
|
||||
).toBe(45)
|
||||
})
|
||||
|
||||
it('clears stale per-instance values when reconfigured without widgets_values', () => {
|
||||
const subgraph = createTestSubgraph({
|
||||
inputs: [{ name: 'value', type: 'number' }]
|
||||
})
|
||||
|
||||
const { node, widget } = createNodeWithWidget('TestNode', 5)
|
||||
subgraph.add(node)
|
||||
subgraph.inputNode.slots[0].connect(node.inputs[0], node)
|
||||
|
||||
const instance = createTestSubgraphNode(subgraph, { id: 701 })
|
||||
instance.graph!.add(instance)
|
||||
|
||||
const promotedWidget = instance.widgets?.[0]
|
||||
promotedWidget!.value = 11
|
||||
widget.value = 17
|
||||
|
||||
const serialized = instance.serialize()
|
||||
delete serialized.widgets_values
|
||||
|
||||
instance.configure({
|
||||
...serialized,
|
||||
id: instance.id,
|
||||
type: subgraph.id
|
||||
})
|
||||
|
||||
expect(instance.widgets?.[0].value).toBe(17)
|
||||
expect(instance.widgets?.[0].serializeValue?.(instance, 0)).toBe(17)
|
||||
})
|
||||
|
||||
it('skips non-serializable source widgets during serialize', () => {
|
||||
const subgraph = createTestSubgraph({
|
||||
inputs: [{ name: 'value', type: 'number' }]
|
||||
|
||||
@@ -1005,6 +1005,7 @@ export class SubgraphNode extends LGraphNode implements BaseLGraph {
|
||||
readonly _instanceWidgetValues = new Map<string, unknown>()
|
||||
|
||||
override configure(info: ExportedSubgraphInstance): void {
|
||||
this._instanceWidgetValues.clear()
|
||||
this._pendingWidgetsValues = info.widgets_values
|
||||
|
||||
for (const input of this.inputs) {
|
||||
@@ -1546,6 +1547,7 @@ export class SubgraphNode extends LGraphNode implements BaseLGraph {
|
||||
override onRemoved(): void {
|
||||
this._eventAbortController.abort()
|
||||
this._invalidatePromotedViewsCache()
|
||||
this._instanceWidgetValues.clear()
|
||||
|
||||
for (const widget of this.widgets) {
|
||||
if (isPromotedWidgetView(widget)) {
|
||||
|
||||
Reference in New Issue
Block a user