mirror of
https://github.com/Comfy-Org/ComfyUI_frontend.git
synced 2026-07-16 16:58:33 +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>
152 lines
4.7 KiB
TypeScript
152 lines
4.7 KiB
TypeScript
import { afterEach, describe, expect, it, vi } from 'vitest'
|
|
|
|
import type { IBaseWidget } from '@/lib/litegraph/src/types/widgets'
|
|
|
|
import { IS_CONTROL_WIDGET } from './controlWidgetMarker'
|
|
import {
|
|
computeNextControlledValue,
|
|
isValueControlWidget
|
|
} from './valueControl'
|
|
|
|
const makeNumberWidget = (
|
|
value: number,
|
|
options: Partial<IBaseWidget['options']> = {}
|
|
): IBaseWidget =>
|
|
({
|
|
type: 'number',
|
|
name: 'seed',
|
|
value,
|
|
options
|
|
}) as unknown as IBaseWidget
|
|
|
|
const makeComboWidget = (value: string, values: string[]): IBaseWidget =>
|
|
({
|
|
type: 'combo',
|
|
name: 'choice',
|
|
value,
|
|
options: { values }
|
|
}) as unknown as IBaseWidget
|
|
|
|
describe('isValueControlWidget', () => {
|
|
it('returns true for a marked widget with both lifecycle hooks', () => {
|
|
const widget = {
|
|
[IS_CONTROL_WIDGET]: true,
|
|
beforeQueued: () => {},
|
|
afterQueued: () => {}
|
|
} as unknown as IBaseWidget
|
|
expect(isValueControlWidget(widget)).toBe(true)
|
|
})
|
|
|
|
it('returns false when the marker symbol is missing', () => {
|
|
const widget = {
|
|
beforeQueued: () => {},
|
|
afterQueued: () => {}
|
|
} as unknown as IBaseWidget
|
|
expect(isValueControlWidget(widget)).toBe(false)
|
|
})
|
|
|
|
it('returns false when lifecycle hooks are missing', () => {
|
|
const widget = {
|
|
[IS_CONTROL_WIDGET]: true
|
|
} as unknown as IBaseWidget
|
|
expect(isValueControlWidget(widget)).toBe(false)
|
|
})
|
|
})
|
|
|
|
describe('computeNextControlledValue (number)', () => {
|
|
it('returns undefined for fixed mode', () => {
|
|
expect(
|
|
computeNextControlledValue(makeNumberWidget(5), 'fixed')
|
|
).toBeUndefined()
|
|
})
|
|
|
|
it('increments by step2', () => {
|
|
const widget = makeNumberWidget(5, { min: 0, max: 100, step2: 2 })
|
|
expect(computeNextControlledValue(widget, 'increment')).toBe(7)
|
|
})
|
|
|
|
it('decrements by step2', () => {
|
|
const widget = makeNumberWidget(5, { min: 0, max: 100, step2: 3 })
|
|
expect(computeNextControlledValue(widget, 'decrement')).toBe(2)
|
|
})
|
|
|
|
it('clamps to max on increment', () => {
|
|
const widget = makeNumberWidget(99, { min: 0, max: 100, step2: 5 })
|
|
expect(computeNextControlledValue(widget, 'increment')).toBe(100)
|
|
})
|
|
|
|
it('clamps to min on decrement', () => {
|
|
const widget = makeNumberWidget(1, { min: 0, max: 100, step2: 5 })
|
|
expect(computeNextControlledValue(widget, 'decrement')).toBe(0)
|
|
})
|
|
|
|
it('randomizes within range using a seeded random', () => {
|
|
const widget = makeNumberWidget(0, { min: 10, max: 20, step2: 1 })
|
|
vi.spyOn(Math, 'random').mockReturnValue(0.5)
|
|
expect(computeNextControlledValue(widget, 'randomize')).toBe(15)
|
|
})
|
|
|
|
it('returns undefined when target value is not numeric', () => {
|
|
const widget = {
|
|
type: 'number',
|
|
name: 'seed',
|
|
value: 'not a number',
|
|
options: {}
|
|
} as unknown as IBaseWidget
|
|
expect(computeNextControlledValue(widget, 'increment')).toBeUndefined()
|
|
})
|
|
})
|
|
|
|
describe('computeNextControlledValue (combo)', () => {
|
|
it('cycles to the next value on increment', () => {
|
|
const widget = makeComboWidget('a', ['a', 'b', 'c'])
|
|
expect(computeNextControlledValue(widget, 'increment')).toBe('b')
|
|
})
|
|
|
|
it('clamps at the end on increment without wrap', () => {
|
|
const widget = makeComboWidget('c', ['a', 'b', 'c'])
|
|
expect(computeNextControlledValue(widget, 'increment')).toBe('c')
|
|
})
|
|
|
|
it('wraps to first value on increment-wrap past the end', () => {
|
|
const widget = makeComboWidget('c', ['a', 'b', 'c'])
|
|
expect(computeNextControlledValue(widget, 'increment-wrap')).toBe('a')
|
|
})
|
|
|
|
it('cycles to the previous value on decrement', () => {
|
|
const widget = makeComboWidget('b', ['a', 'b', 'c'])
|
|
expect(computeNextControlledValue(widget, 'decrement')).toBe('a')
|
|
})
|
|
|
|
it('randomizes by index', () => {
|
|
const widget = makeComboWidget('a', ['a', 'b', 'c', 'd'])
|
|
vi.spyOn(Math, 'random').mockReturnValue(0.6)
|
|
expect(computeNextControlledValue(widget, 'randomize')).toBe('c')
|
|
})
|
|
|
|
it('applies a substring filter', () => {
|
|
const widget = makeComboWidget('apple', ['apple', 'banana', 'apricot'])
|
|
expect(
|
|
computeNextControlledValue(widget, 'increment', { comboFilter: 'ap' })
|
|
).toBe('apricot')
|
|
})
|
|
|
|
it('applies a regex filter when wrapped in slashes', () => {
|
|
const widget = makeComboWidget('foo1', ['foo1', 'bar', 'foo2'])
|
|
expect(
|
|
computeNextControlledValue(widget, 'increment', { comboFilter: '/foo/' })
|
|
).toBe('foo2')
|
|
})
|
|
|
|
it('returns undefined when the filter eliminates all values', () => {
|
|
const widget = makeComboWidget('a', ['a', 'b'])
|
|
expect(
|
|
computeNextControlledValue(widget, 'increment', { comboFilter: 'zzz' })
|
|
).toBeUndefined()
|
|
})
|
|
|
|
afterEach(() => {
|
|
vi.restoreAllMocks()
|
|
})
|
|
})
|