Compare commits

...

1 Commits

Author SHA1 Message Date
Dante
949fc5343d fix(widgets): collapse duplicate COLOR widget rendering on Color to RGB Int (FE-842) (#12447)
## Summary

Fix the duplicate \`<WidgetColorPicker>\` rendering on the \`Color to
RGB Int\` node (and any other COLOR-using V3 node that the runtime
double-registers a widget for).
<img width="480" alt="after-fix-dedupe-proof"
src="https://github.com/user-attachments/assets/5c801806-ed5d-493f-92b6-e0b99dd8e408"
/>

## Changes

- **What**:
- \`useProcessedWidgets.getWidgetIdentity\`: fall back to the host
\`nodeId\` parameter for the dedupe identity root when neither
\`storeNodeId/widget.nodeId\` nor \`sourceExecutionId\` is set. Normal
root-graph widgets now dedupe identically to promoted/execution-scoped
widgets, so any duplicate same-name+same-type widget collapses to one
render. \`sourceExecutionId\` precedence is preserved.
- \`useColorWidget\`: read top-level \`default\` from the V2 spec (falls
back to nested \`options.default\` for hand-authored V2 specs), and
short-circuit if a same-name color widget already exists on
\`node.widgets\` so a second \`addWidget('color', …)\` call from
upstream hooks (or a \`configure\` round-trip) no longer duplicates the
row.
- **Tests**:
- New \`useColorWidget.test.ts\` covers top-level default,
nested-options fallback, no-default fallback, and the idempotency guard.
- \`useProcessedWidgets.test.ts\` gets a regression case for two
identical color widgets on the same node collapsing to one render, plus
an updated \`getWidgetIdentity\` case for the host-nodeId fallback.

## Review Focus

- \`getWidgetIdentity\` precedence change. The fallback only fires when
none of \`storeNodeId\`, \`widget.nodeId\`, or \`sourceExecutionId\` are
present, so promoted/exec-scoped widgets (incl. the \"unresolved
same-name promoted entries distinct by source execution identity\"
\`NodeWidgets\` test) are unaffected.
- \`useColorWidget\` idempotency guard is defensive — the root cause of
the second \`addWidget\` call (cloud-only hook or persisted
\`info.widgets\` configure round-trip) is not in this diff; that's
tracked separately.

Fixes
[FE-842](https://linear.app/comfyorg/issue/FE-842/color-to-rgb-int-node-shows-duplicate-color-widgets)
2026-05-25 11:50:28 +00:00
5 changed files with 152 additions and 8 deletions

View File

@@ -12,19 +12,22 @@ test.describe('Vue Widget Reactivity', { tag: '@vue-nodes' }, () => {
await comfyPage.page.evaluate(() => {
const graph = window.graph as TestGraphAccess
const node = graph._nodes_by_id['4']
node.widgets!.push(node.widgets![0])
node.widgets!.push({ ...node.widgets![0], name: 'added_widget_1' })
})
await expect(loadCheckpointNode).toHaveCount(2)
await comfyPage.page.evaluate(() => {
const graph = window.graph as TestGraphAccess
const node = graph._nodes_by_id['4']
node.widgets![2] = node.widgets![0]
node.widgets![2] = { ...node.widgets![0], name: 'added_widget_2' }
})
await expect(loadCheckpointNode).toHaveCount(3)
await comfyPage.page.evaluate(() => {
const graph = window.graph as TestGraphAccess
const node = graph._nodes_by_id['4']
node.widgets!.splice(0, 0, node.widgets![0])
node.widgets!.splice(0, 0, {
...node.widgets![0],
name: 'added_widget_3'
})
})
await expect(loadCheckpointNode).toHaveCount(4)
})

View File

@@ -54,15 +54,30 @@ describe('getWidgetIdentity', () => {
expect(renderKey).toBe(dedupeIdentity)
})
it('returns transient renderKey for widgets without stable identity', () => {
it('falls back to host nodeId so duplicate normal widgets dedupe', () => {
const widget = createMockWidget({
nodeId: undefined,
storeNodeId: undefined,
sourceExecutionId: undefined
})
const { dedupeIdentity, renderKey } = getWidgetIdentity(widget, '5', 3)
expect(dedupeIdentity).toBe('node:5:test_widget:test_widget:combo')
expect(renderKey).toBe(dedupeIdentity)
})
it('returns transient renderKey when no nodeId is available at all', () => {
const widget = createMockWidget({
nodeId: undefined,
storeNodeId: undefined,
sourceExecutionId: undefined
})
const { dedupeIdentity, renderKey } = getWidgetIdentity(
widget,
undefined,
3
)
expect(dedupeIdentity).toBeUndefined()
expect(renderKey).toBe('transient:5:test_widget:test_widget:combo:3')
expect(renderKey).toBe('transient::test_widget:test_widget:combo:3')
})
it('uses sourceExecutionId for identity when no nodeId', () => {
@@ -360,6 +375,46 @@ describe('computeProcessedWidgets borderStyle', () => {
expect(result).toHaveLength(1)
expect(result[0].hidden).toBe(false)
})
it('collapses duplicate normal widgets on the same node to one render', () => {
const colorA = createMockWidget({
name: 'color',
type: 'color',
nodeId: undefined,
storeNodeId: undefined,
sourceExecutionId: undefined
})
const colorB = createMockWidget({
name: 'color',
type: 'color',
nodeId: undefined,
storeNodeId: undefined,
sourceExecutionId: undefined
})
const result = computeProcessedWidgets({
nodeData: {
id: '1',
type: 'ColorToRGBInt',
widgets: [colorA, colorB],
title: 'Color to RGB Int',
mode: 0,
selected: false,
executing: false,
inputs: [],
outputs: []
},
graphId: 'graph-test',
showAdvanced: false,
isGraphReady: false,
rootGraph: null,
ui: noopUi
})
expect(result).toHaveLength(1)
expect(result[0].name).toBe('color')
expect(result[0].renderKey).toBe('node:1:color:color:color')
})
})
describe('createWidgetUpdateHandler (via computeProcessedWidgets)', () => {

View File

@@ -129,11 +129,15 @@ export function getWidgetIdentity(
const rawWidgetId = widget.storeNodeId ?? widget.nodeId
const storeWidgetName = widget.storeName ?? widget.name
const slotNameForIdentity = widget.slotName ?? widget.name
const hostNodeIdRoot =
nodeId !== undefined && nodeId !== ''
? `node:${String(stripGraphPrefix(nodeId))}`
: undefined
const stableIdentityRoot = rawWidgetId
? `node:${String(stripGraphPrefix(rawWidgetId))}`
: widget.sourceExecutionId
? `exec:${widget.sourceExecutionId}`
: undefined
: hostNodeIdRoot
const dedupeIdentity = stableIdentityRoot
? `${stableIdentityRoot}:${storeWidgetName}:${slotNameForIdentity}:${widget.type}`

View File

@@ -0,0 +1,76 @@
import { describe, expect, it, vi } from 'vitest'
import type { LGraphNode } from '@/lib/litegraph/src/litegraph'
import type {
IColorWidget,
IWidgetOptions
} from '@/lib/litegraph/src/types/widgets'
import type { InputSpec } from '@/schemas/nodeDef/nodeDefSchemaV2'
import { useColorWidget } from '@/renderer/extensions/vueNodes/widgets/composables/useColorWidget'
function createMockNode(): LGraphNode {
const widgets: IColorWidget[] = []
const addWidget = vi.fn(
(
type: string,
name: string,
value: string,
_callback: () => void,
options: IWidgetOptions
) => {
const widget = {
type,
name,
value,
options,
callback: _callback
} as unknown as IColorWidget
widgets.push(widget)
return widget
}
)
return { widgets, addWidget } as unknown as LGraphNode
}
const colorSpec: InputSpec = {
type: 'COLOR',
name: 'color',
default: '#ffffff',
socketless: true
}
describe('useColorWidget', () => {
it('reads the top-level default from the V2 spec', () => {
const node = createMockNode()
const widget = useColorWidget()(node, colorSpec)
expect(widget.value).toBe('#ffffff')
})
it('falls back to nested options.default when top-level default is absent', () => {
const node = createMockNode()
const widget = useColorWidget()(node, {
type: 'COLOR',
name: 'color',
options: { default: '#abcdef' }
} as InputSpec)
expect(widget.value).toBe('#abcdef')
})
it('falls back to #000000 when no default is declared', () => {
const node = createMockNode()
const widget = useColorWidget()(node, {
type: 'COLOR',
name: 'color'
} as InputSpec)
expect(widget.value).toBe('#000000')
})
it('returns the existing widget instead of creating a duplicate', () => {
const node = createMockNode()
const first = useColorWidget()(node, colorSpec)
const second = useColorWidget()(node, colorSpec)
expect(second).toBe(first)
expect(node.widgets).toHaveLength(1)
})
})

View File

@@ -8,8 +8,14 @@ import type { ComfyWidgetConstructorV2 } from '@/scripts/widgets'
export const useColorWidget = (): ComfyWidgetConstructorV2 => {
return (node: LGraphNode, inputSpec: InputSpecV2): IColorWidget => {
const { name, options } = inputSpec as ColorInputSpec
const defaultValue = options?.default || '#000000'
const colorSpec = inputSpec as ColorInputSpec
const { name, options } = colorSpec
const defaultValue = colorSpec.default ?? options?.default ?? '#000000'
const existing = node.widgets?.find(
(w): w is IColorWidget => w.name === name && w.type === 'color'
)
if (existing) return existing
const widget = node.addWidget('color', name, defaultValue, () => {}, {
serialize: true