mirror of
https://github.com/Comfy-Org/ComfyUI_frontend.git
synced 2026-05-05 13:41:59 +00:00
fix: stabilize subgraph promoted widget identity and rendering (#9896)
## Summary Fix subgraph promoted widget identity/rendering so on-node widgets stay correct through configure/hydration churn, duplicate names, and linked+independent coexistence. ## Changes - **Subgraph promotion reconciliation**: stabilize linked-entry identity by subgraph slot id, preserve deterministic linked representative selection, and prune stale alias/fallback entries without dropping legitimate independent promotions. - **Promoted view resolution**: bind slot mapping by promoted view object identity (`getSlotFromWidget` / `getWidgetFromSlot`) to avoid same-name collisions. - **On-node widget rendering**: harden `NodeWidgets` identity and dedup to avoid visual aliasing, prefer visible duplicates over hidden stale entries, include type/source execution identity, and avoid collapsing transient unresolved entries. - **Mapping correctness**: update `useGraphNodeManager` promoted source mapping to resolve by input target only when the promoted view is actually bound to that input. - **Subgraph input uniqueness**: ensure empty-slot promotion creates unique input names (`seed`, `seed_1`, etc.) for same-name multi-source promotions. - **Safety fix**: guard against undefined canvas in slot-link interaction. - **Tests/fixtures**: add focused regressions for fixture path `subgraph_complex_promotion_1`, linked+independent same-name cases, duplicate-name identity mapping, dedup behavior, and input-name uniqueness. ## Review Focus Validate behavior around transient configure/hydration states (`-1` id to concrete id), duplicate-name promotions, linked representative recovery, and that dedup never hides legitimate widgets while still removing true duplicates. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-9896-fix-stabilize-subgraph-promoted-widget-identity-and-rendering-3226d73d365081c8a1e8d0a5a22e826d) by [Unito](https://www.unito.io) --------- Co-authored-by: Amp <amp@ampcode.com>
This commit is contained in:
Alexander Brown
@@ -1,14 +1,29 @@
|
||||
import { createTestingPinia } from '@pinia/testing'
|
||||
import { mount } from '@vue/test-utils'
|
||||
import { describe, expect, it } from 'vitest'
|
||||
import { setActivePinia } from 'pinia'
|
||||
import { nextTick } from 'vue'
|
||||
import { describe, expect, it, vi } from 'vitest'
|
||||
|
||||
import type {
|
||||
SafeWidgetData,
|
||||
VueNodeData
|
||||
} from '@/composables/graph/useGraphNodeManager'
|
||||
import { useWidgetValueStore } from '@/stores/widgetValueStore'
|
||||
|
||||
import NodeWidgets from '@/renderer/extensions/vueNodes/components/NodeWidgets.vue'
|
||||
|
||||
vi.mock('@/renderer/core/canvas/canvasStore', () => ({
|
||||
useCanvasStore: () => ({
|
||||
canvas: {
|
||||
graph: {
|
||||
rootGraph: {
|
||||
id: 'graph-test'
|
||||
}
|
||||
}
|
||||
}
|
||||
})
|
||||
}))
|
||||
|
||||
describe('NodeWidgets', () => {
|
||||
const createMockWidget = (
|
||||
overrides: Partial<SafeWidgetData> = {}
|
||||
@@ -40,12 +55,15 @@ describe('NodeWidgets', () => {
|
||||
})
|
||||
|
||||
const mountComponent = (nodeData?: VueNodeData) => {
|
||||
const pinia = createTestingPinia({ stubActions: false })
|
||||
setActivePinia(pinia)
|
||||
|
||||
return mount(NodeWidgets, {
|
||||
props: {
|
||||
nodeData
|
||||
},
|
||||
global: {
|
||||
plugins: [createTestingPinia()],
|
||||
plugins: [pinia],
|
||||
stubs: {
|
||||
// Stub InputSlot to avoid complex slot registration dependencies
|
||||
InputSlot: true
|
||||
@@ -117,4 +135,165 @@ describe('NodeWidgets', () => {
|
||||
}
|
||||
)
|
||||
})
|
||||
|
||||
it('deduplicates widgets with identical render identity while keeping distinct promoted sources', () => {
|
||||
const duplicateA = createMockWidget({
|
||||
name: 'string_a',
|
||||
type: 'text',
|
||||
nodeId: '5e0670b8-ea2c-4fb6-8b73-a1100a2d4f8f:19',
|
||||
storeNodeId: '5e0670b8-ea2c-4fb6-8b73-a1100a2d4f8f:19',
|
||||
storeName: 'string_a',
|
||||
slotName: 'string_a'
|
||||
})
|
||||
const duplicateB = createMockWidget({
|
||||
name: 'string_a',
|
||||
type: 'text',
|
||||
nodeId: '5e0670b8-ea2c-4fb6-8b73-a1100a2d4f8f:19',
|
||||
storeNodeId: '5e0670b8-ea2c-4fb6-8b73-a1100a2d4f8f:19',
|
||||
storeName: 'string_a',
|
||||
slotName: 'string_a'
|
||||
})
|
||||
const distinct = createMockWidget({
|
||||
name: 'string_a',
|
||||
type: 'text',
|
||||
nodeId: '5e0670b8-ea2c-4fb6-8b73-a1100a2d4f8f:20',
|
||||
storeNodeId: '5e0670b8-ea2c-4fb6-8b73-a1100a2d4f8f:20',
|
||||
storeName: 'string_a',
|
||||
slotName: 'string_a'
|
||||
})
|
||||
const nodeData = createMockNodeData('SubgraphNode', [
|
||||
duplicateA,
|
||||
duplicateB,
|
||||
distinct
|
||||
])
|
||||
|
||||
const wrapper = mountComponent(nodeData)
|
||||
|
||||
expect(wrapper.findAll('.lg-node-widget')).toHaveLength(2)
|
||||
})
|
||||
|
||||
it('prefers a visible duplicate over a hidden duplicate when identities collide', () => {
|
||||
const hiddenDuplicate = createMockWidget({
|
||||
name: 'string_a',
|
||||
type: 'text',
|
||||
nodeId: '5e0670b8-ea2c-4fb6-8b73-a1100a2d4f8f:19',
|
||||
storeNodeId: '5e0670b8-ea2c-4fb6-8b73-a1100a2d4f8f:19',
|
||||
storeName: 'string_a',
|
||||
slotName: 'string_a',
|
||||
options: { hidden: true }
|
||||
})
|
||||
const visibleDuplicate = createMockWidget({
|
||||
name: 'string_a',
|
||||
type: 'text',
|
||||
nodeId: '5e0670b8-ea2c-4fb6-8b73-a1100a2d4f8f:19',
|
||||
storeNodeId: '5e0670b8-ea2c-4fb6-8b73-a1100a2d4f8f:19',
|
||||
storeName: 'string_a',
|
||||
slotName: 'string_a',
|
||||
options: { hidden: false }
|
||||
})
|
||||
const nodeData = createMockNodeData('SubgraphNode', [
|
||||
hiddenDuplicate,
|
||||
visibleDuplicate
|
||||
])
|
||||
|
||||
const wrapper = mountComponent(nodeData)
|
||||
|
||||
expect(wrapper.findAll('.lg-node-widget')).toHaveLength(1)
|
||||
})
|
||||
|
||||
it('does not deduplicate entries that share names but have different widget types', () => {
|
||||
const textWidget = createMockWidget({
|
||||
name: 'string_a',
|
||||
type: 'text',
|
||||
nodeId: '5e0670b8-ea2c-4fb6-8b73-a1100a2d4f8f:19',
|
||||
storeNodeId: '5e0670b8-ea2c-4fb6-8b73-a1100a2d4f8f:19',
|
||||
storeName: 'string_a',
|
||||
slotName: 'string_a'
|
||||
})
|
||||
const comboWidget = createMockWidget({
|
||||
name: 'string_a',
|
||||
type: 'combo',
|
||||
nodeId: '5e0670b8-ea2c-4fb6-8b73-a1100a2d4f8f:19',
|
||||
storeNodeId: '5e0670b8-ea2c-4fb6-8b73-a1100a2d4f8f:19',
|
||||
storeName: 'string_a',
|
||||
slotName: 'string_a'
|
||||
})
|
||||
const nodeData = createMockNodeData('SubgraphNode', [
|
||||
textWidget,
|
||||
comboWidget
|
||||
])
|
||||
|
||||
const wrapper = mountComponent(nodeData)
|
||||
|
||||
expect(wrapper.findAll('.lg-node-widget')).toHaveLength(2)
|
||||
})
|
||||
|
||||
it('keeps unresolved same-name promoted entries distinct by source execution identity', () => {
|
||||
const firstTransientEntry = createMockWidget({
|
||||
nodeId: undefined,
|
||||
storeNodeId: undefined,
|
||||
name: 'string_a',
|
||||
storeName: 'string_a',
|
||||
slotName: 'string_a',
|
||||
type: 'text',
|
||||
sourceExecutionId: '65:18'
|
||||
})
|
||||
const secondTransientEntry = createMockWidget({
|
||||
nodeId: undefined,
|
||||
storeNodeId: undefined,
|
||||
name: 'string_a',
|
||||
storeName: 'string_a',
|
||||
slotName: 'string_a',
|
||||
type: 'text',
|
||||
sourceExecutionId: '65:19'
|
||||
})
|
||||
const nodeData = createMockNodeData('SubgraphNode', [
|
||||
firstTransientEntry,
|
||||
secondTransientEntry
|
||||
])
|
||||
|
||||
const wrapper = mountComponent(nodeData)
|
||||
|
||||
expect(wrapper.findAll('.lg-node-widget')).toHaveLength(2)
|
||||
})
|
||||
|
||||
it('hides widgets when merged store options mark them hidden', async () => {
|
||||
const nodeData = createMockNodeData('TestNode', [
|
||||
createMockWidget({
|
||||
nodeId: 'test_node',
|
||||
name: 'test_widget',
|
||||
options: { hidden: false }
|
||||
})
|
||||
])
|
||||
|
||||
const wrapper = mountComponent(nodeData)
|
||||
const widgetValueStore = useWidgetValueStore()
|
||||
widgetValueStore.registerWidget('graph-test', {
|
||||
nodeId: 'test_node',
|
||||
name: 'test_widget',
|
||||
type: 'combo',
|
||||
value: 'value',
|
||||
options: { hidden: true },
|
||||
label: undefined,
|
||||
serialize: true,
|
||||
disabled: false
|
||||
})
|
||||
|
||||
await nextTick()
|
||||
|
||||
expect(wrapper.findAll('.lg-node-widget')).toHaveLength(0)
|
||||
})
|
||||
|
||||
it('keeps AppInput ids mapped to node identity for selection', () => {
|
||||
const nodeData = createMockNodeData('TestNode', [
|
||||
createMockWidget({ nodeId: 'test_node', name: 'seed_a', type: 'text' }),
|
||||
createMockWidget({ nodeId: 'test_node', name: 'seed_b', type: 'text' })
|
||||
])
|
||||
|
||||
const wrapper = mountComponent(nodeData)
|
||||
const appInputWrappers = wrapper.findAllComponents({ name: 'AppInput' })
|
||||
const ids = appInputWrappers.map((component) => component.props('id'))
|
||||
|
||||
expect(ids).toStrictEqual(['test_node', 'test_node'])
|
||||
})
|
||||
})
|
||||
|
||||
Reference in New Issue
Block a user