mirror of
https://github.com/Comfy-Org/ComfyUI_frontend.git
synced 2026-06-05 20:54:56 +00:00
## Summary Introduces **Subgraph Link Only Promotion** (ADR 0009) — a new model for surfacing inner subgraph widgets on the parent SubgraphNode by *promoting through links* rather than by duplicating widget state on the host. Ships with the hygiene/refactor pass on the migration, store, and event layers that the new model depends on. ## What changes ### Subgraph Link Only Promotion (ADR 0009) Promoted widgets are defined by the link from a SubgraphNode input to the interior node, not by a duplicated widget instance on the host. Consequences: - A SubgraphNode renders inner widgets purely as a **projection** of the interior widgets and links — no host-side state to drift. - **Per-host independence**: multiple instances of the same SubgraphNode render and edit their own values without cross-talk. - **Reversible promote/demote**: structural link operation, so demote preserves host slots and external connections (#12278). ### Supporting refactors - **Migration** — Planner/classifier/repair/quarantine helpers collapsed into a single `proxyWidgetMigration` entry point with black-box round-trip coverage. Honors the source-node-id disambiguator on `proxyWidgets`, so deduplicated names (e.g. `text`, `text_1`) resolve to the right interior widget. - **Widget identity** — `appMode` unified on `WidgetEntityId`; promoted widget state is keyed by entityId across the store, DOM, and migration paths. - **SubgraphNode** — 3-key promoted-view cache replaced with a single version counter + explicit `invalidatePromotedViews()` at mutation sites; `id === -1` sentinel removed. - **Events** — `LGraph.trigger()` now dispatches node trigger payloads through `this.events`, replacing a leaky `onTrigger` monkey-patch. `SubgraphEditor` reactivity is driven from subgraph events instead of imperative refresh. - **Stores** — `appModeStore` migration helpers collapsed into `upgradeAndValidateInput`; `nodeOutputStore.*ByExecutionId` derived from the locator index; `previewExposureStore` cleanup and cycle-detection double-warn fix. - **Misc** — `Outcome` types consolidated; mutable accumulators replaced with `flatMap`; new ESLint rule forbids litegraph imports under `src/world/`. ### Tests - Browser tests for promoted widgets retagged `@vue-nodes` and rewritten to assert against the rendered Vue node DOM (via `getNodeLocator` / `getByRole('textbox')` / `enterSubgraph`) instead of `page.evaluate` graph introspection. - Per-host widget independence asserted via DOM. - Migration coverage moved to black-box round-trip tests. - Added coverage for duplicate-named promoted widget identity (ADR 0009) and the per-parent demote branch in `WidgetActions`. ## Review focus - ADR 0009 conformance of the link-only promotion model. - Disambiguator resolution path in `proxyWidgetMigration`. - Single-version-counter promoted-view cache and its `invalidatePromotedViews()` call sites. - `LGraph.trigger()` event dispatch and the `AppModeWidgetList.vue` migration off `onTrigger` (FE-667 tracks the remaining `useGraphNodeManager` conversion). ## Breaking changes None for users. Internal subgraph promotion APIs changed — see ADR 0009. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-12197-feat-subgraph-link-only-widget-promotion-migration-store-hygiene-35e6d73d365081fd882cf3a69bc09956) by [Unito](https://www.unito.io) --------- Co-authored-by: Amp <amp@ampcode.com> Co-authored-by: GitHub Action <action@github.com> Co-authored-by: github-actions <github-actions@github.com> Co-authored-by: Glary-Bot <glary-bot@users.noreply.github.com> Co-authored-by: AustinMroz <austin@comfy.org>
266 lines
9.1 KiB
TypeScript
266 lines
9.1 KiB
TypeScript
import { render, screen, within } from '@testing-library/vue'
|
|
import userEvent from '@testing-library/user-event'
|
|
import { createTestingPinia } from '@pinia/testing'
|
|
import { setActivePinia } from 'pinia'
|
|
import { beforeEach, describe, expect, it, vi } from 'vitest'
|
|
import { nextTick } from 'vue'
|
|
import { createI18n } from 'vue-i18n'
|
|
|
|
import {
|
|
createTestSubgraph,
|
|
createTestSubgraphNode
|
|
} from '@/lib/litegraph/src/subgraph/__fixtures__/subgraphHelpers'
|
|
import { LGraphNode } from '@/lib/litegraph/src/litegraph'
|
|
import { useCanvasStore } from '@/renderer/core/canvas/canvasStore'
|
|
import { usePreviewExposureStore } from '@/stores/previewExposureStore'
|
|
|
|
import { isPromotedWidgetView } from '@/core/graph/subgraph/promotedWidgetTypes'
|
|
import { promoteValueWidgetViaSubgraphInput } from '@/core/graph/subgraph/promotionUtils'
|
|
import SubgraphEditor from './SubgraphEditor.vue'
|
|
import type { ComponentProps } from 'vue-component-type-helpers'
|
|
import type DraggableList from '@/components/common/DraggableList.vue'
|
|
|
|
type DraggableListProps = ComponentProps<typeof DraggableList>
|
|
type PromotedRow =
|
|
DraggableListProps['modelValue'] extends Array<infer T> ? T : never
|
|
|
|
vi.mock('@/services/litegraphService', () => ({
|
|
useLitegraphService: () => ({ updatePreviews: vi.fn() })
|
|
}))
|
|
|
|
const i18n = createI18n({
|
|
legacy: false,
|
|
locale: 'en',
|
|
messages: {
|
|
en: {
|
|
subgraphStore: {
|
|
shown: 'Shown',
|
|
hidden: 'Hidden',
|
|
hideAll: 'Hide all',
|
|
showAll: 'Show all',
|
|
addRecommended: 'Add recommended'
|
|
},
|
|
rightSidePanel: {
|
|
noneSearchDesc: 'No results'
|
|
},
|
|
g: {
|
|
search: 'Search',
|
|
searchPlaceholder: 'Search'
|
|
}
|
|
}
|
|
}
|
|
})
|
|
|
|
describe('SubgraphEditor', () => {
|
|
beforeEach(() => {
|
|
setActivePinia(createTestingPinia({ stubActions: false }))
|
|
vi.clearAllMocks()
|
|
})
|
|
|
|
it('renders preview exposures after promoted inputs without drag handles', () => {
|
|
const subgraph = createTestSubgraph()
|
|
const host = createTestSubgraphNode(subgraph)
|
|
const firstNode = new LGraphNode('FirstNode')
|
|
const secondNode = new LGraphNode('SecondNode')
|
|
const previewNode = new LGraphNode('PreviewImage')
|
|
previewNode.type = 'PreviewImage'
|
|
subgraph.add(firstNode)
|
|
subgraph.add(secondNode)
|
|
subgraph.add(previewNode)
|
|
|
|
const firstInput = firstNode.addInput('first', 'STRING')
|
|
const firstWidget = firstNode.addWidget('text', 'first', '', () => {})
|
|
firstInput.widget = { name: firstWidget.name }
|
|
const secondInput = secondNode.addInput('second', 'STRING')
|
|
const secondWidget = secondNode.addWidget('text', 'second', '', () => {})
|
|
secondInput.widget = { name: secondWidget.name }
|
|
promoteValueWidgetViaSubgraphInput(host, firstNode, firstWidget)
|
|
promoteValueWidgetViaSubgraphInput(host, secondNode, secondWidget)
|
|
usePreviewExposureStore().addExposure(
|
|
subgraph.rootGraph.id,
|
|
String(host.id),
|
|
{
|
|
sourceNodeId: String(previewNode.id),
|
|
sourcePreviewName: '$$canvas-image-preview'
|
|
}
|
|
)
|
|
useCanvasStore().selectedItems = [host]
|
|
|
|
render(SubgraphEditor, {
|
|
container: document.body.appendChild(document.createElement('div')),
|
|
global: {
|
|
plugins: [i18n],
|
|
stubs: {
|
|
DraggableList: {
|
|
template:
|
|
'<div data-testid="draggable-list"><slot drag-class="draggable-item" /></div>'
|
|
}
|
|
}
|
|
}
|
|
})
|
|
|
|
const shown = screen.getByTestId('subgraph-editor-shown-section')
|
|
expect(
|
|
within(shown)
|
|
.getAllByTestId('subgraph-widget-label')
|
|
.map((el) => el.textContent?.trim())
|
|
).toEqual(['first', 'second', '$$canvas-image-preview'])
|
|
expect(
|
|
within(screen.getByTestId('draggable-list'))
|
|
.getAllByTestId('subgraph-widget-label')
|
|
.map((el) => el.textContent?.trim())
|
|
).toEqual(['first', 'second'])
|
|
expect(
|
|
within(shown).getAllByTestId('subgraph-widget-drag-handle')
|
|
).toHaveLength(2)
|
|
})
|
|
|
|
it('updates rendered order when promoted widgets are reordered', async () => {
|
|
const subgraph = createTestSubgraph()
|
|
const host = createTestSubgraphNode(subgraph)
|
|
const firstNode = new LGraphNode('FirstNode')
|
|
const secondNode = new LGraphNode('SecondNode')
|
|
subgraph.add(firstNode)
|
|
subgraph.add(secondNode)
|
|
|
|
const firstInput = firstNode.addInput('first', 'STRING')
|
|
const firstWidget = firstNode.addWidget('text', 'first', '', () => {})
|
|
firstInput.widget = { name: firstWidget.name }
|
|
const secondInput = secondNode.addInput('second', 'STRING')
|
|
const secondWidget = secondNode.addWidget('text', 'second', '', () => {})
|
|
secondInput.widget = { name: secondWidget.name }
|
|
promoteValueWidgetViaSubgraphInput(host, firstNode, firstWidget)
|
|
promoteValueWidgetViaSubgraphInput(host, secondNode, secondWidget)
|
|
useCanvasStore().selectedItems = [host]
|
|
|
|
let listSetter: ((value: PromotedRow[]) => void) | undefined
|
|
const draggableListStub = {
|
|
props: ['modelValue'],
|
|
emits: ['update:modelValue'],
|
|
setup(
|
|
_: unknown,
|
|
{
|
|
emit,
|
|
slots
|
|
}: {
|
|
emit: (event: string, ...args: unknown[]) => void
|
|
slots: { default?: (props: { dragClass: string }) => unknown }
|
|
}
|
|
) {
|
|
listSetter = (value) => emit('update:modelValue', value)
|
|
return () => slots.default?.({ dragClass: 'draggable-item' })
|
|
}
|
|
}
|
|
render(SubgraphEditor, {
|
|
container: document.body.appendChild(document.createElement('div')),
|
|
global: {
|
|
plugins: [i18n],
|
|
stubs: { DraggableList: draggableListStub }
|
|
}
|
|
})
|
|
await nextTick()
|
|
|
|
const shown = screen.getByTestId('subgraph-editor-shown-section')
|
|
expect(
|
|
within(shown)
|
|
.getAllByTestId('subgraph-widget-label')
|
|
.map((el) => el.textContent?.trim())
|
|
).toEqual(['first', 'second'])
|
|
|
|
const promotedWidgets = host.widgets.filter(isPromotedWidgetView)
|
|
const reversed = [
|
|
{ kind: 'promoted', node: secondNode, widget: promotedWidgets[1] },
|
|
{ kind: 'promoted', node: firstNode, widget: promotedWidgets[0] }
|
|
] as PromotedRow[]
|
|
listSetter?.(reversed)
|
|
await nextTick()
|
|
|
|
expect(
|
|
within(shown)
|
|
.getAllByTestId('subgraph-widget-label')
|
|
.map((el) => el.textContent?.trim())
|
|
).toEqual(['second', 'first'])
|
|
})
|
|
|
|
it('demotes linked promoted widgets when "Hide all" is clicked', async () => {
|
|
const subgraph = createTestSubgraph()
|
|
const host = createTestSubgraphNode(subgraph)
|
|
const firstNode = new LGraphNode('FirstNode')
|
|
const secondNode = new LGraphNode('SecondNode')
|
|
subgraph.add(firstNode)
|
|
subgraph.add(secondNode)
|
|
|
|
const firstInput = firstNode.addInput('first', 'STRING')
|
|
const firstWidget = firstNode.addWidget('text', 'first', '', () => {})
|
|
firstInput.widget = { name: firstWidget.name }
|
|
const secondInput = secondNode.addInput('second', 'STRING')
|
|
const secondWidget = secondNode.addWidget('text', 'second', '', () => {})
|
|
secondInput.widget = { name: secondWidget.name }
|
|
promoteValueWidgetViaSubgraphInput(host, firstNode, firstWidget)
|
|
promoteValueWidgetViaSubgraphInput(host, secondNode, secondWidget)
|
|
useCanvasStore().selectedItems = [host]
|
|
|
|
render(SubgraphEditor, {
|
|
container: document.body.appendChild(document.createElement('div')),
|
|
global: {
|
|
plugins: [i18n],
|
|
stubs: {
|
|
DraggableList: {
|
|
template:
|
|
'<div data-testid="draggable-list"><slot drag-class="draggable-item" /></div>'
|
|
}
|
|
}
|
|
}
|
|
})
|
|
|
|
expect(host.widgets.filter(isPromotedWidgetView)).toHaveLength(2)
|
|
|
|
const shown = screen.getByTestId('subgraph-editor-shown-section')
|
|
const hideAllLink = within(shown).getByText('Hide all')
|
|
await userEvent.click(hideAllLink)
|
|
|
|
expect(host.widgets.filter(isPromotedWidgetView)).toHaveLength(0)
|
|
})
|
|
|
|
it('removes the exposure when a preview row without a real source widget is demoted', async () => {
|
|
const subgraph = createTestSubgraph()
|
|
const host = createTestSubgraphNode(subgraph)
|
|
const orphanedSourceNode = new LGraphNode('OrphanedNode')
|
|
orphanedSourceNode.type = 'OrphanedNode'
|
|
subgraph.add(orphanedSourceNode)
|
|
|
|
const previewStore = usePreviewExposureStore()
|
|
previewStore.addExposure(subgraph.rootGraph.id, String(host.id), {
|
|
sourceNodeId: String(orphanedSourceNode.id),
|
|
sourcePreviewName: '$$canvas-image-preview'
|
|
})
|
|
|
|
useCanvasStore().selectedItems = [host]
|
|
|
|
render(SubgraphEditor, {
|
|
container: document.body.appendChild(document.createElement('div')),
|
|
global: {
|
|
plugins: [i18n],
|
|
stubs: {
|
|
DraggableList: {
|
|
template:
|
|
'<div data-testid="draggable-list"><slot drag-class="draggable-item" /></div>'
|
|
}
|
|
}
|
|
}
|
|
})
|
|
|
|
expect(
|
|
previewStore.getExposures(subgraph.rootGraph.id, String(host.id))
|
|
).toHaveLength(1)
|
|
|
|
const shown = screen.getByTestId('subgraph-editor-shown-section')
|
|
const toggleButton = within(shown).getByTestId('subgraph-widget-toggle')
|
|
await userEvent.click(toggleButton)
|
|
|
|
expect(
|
|
previewStore.getExposures(subgraph.rootGraph.id, String(host.id))
|
|
).toHaveLength(0)
|
|
})
|
|
})
|