mirror of
https://github.com/Comfy-Org/ComfyUI_frontend.git
synced 2026-02-24 08:44:06 +00:00
## Summary Replace the Proxy-based proxy widget system with a store-driven architecture where `promotionStore` and `widgetValueStore` are the single sources of truth for subgraph widget promotion and widget values, and `SubgraphNode.widgets` is a synthetic getter composing lightweight `PromotedWidgetView` objects from store state. ## Motivation The subgraph widget promotion system previously scattered state across multiple unsynchronized layers: - **Persistence**: `node.properties.proxyWidgets` (tuples on the LiteGraph node) - **Runtime**: Proxy-based `proxyWidget.ts` with `Overlay` objects, `DisconnectedWidget` singleton, and `isProxyWidget` type guards - **UI**: Each Vue component independently calling `parseProxyWidgets()` via `customRef` hacks - **Mutation flags**: Imperative `widget.promoted = true/false` set on `subgraph-opened` events This led to 4+ independent parsings of the same data, complex cache invalidation, and no reactive contract between the promotion state and the rendering layer. Widget values were similarly owned by LiteGraph with no Vue-reactive backing. The core principle driving these changes: **Vue owns truth**. Pinia stores are the canonical source; LiteGraph objects delegate to stores via getters/setters; Vue components react to store state directly. ## Changes ### New stores (single sources of truth) - **`promotionStore`** — Reactive `Map<NodeId, PromotionEntry[]>` tracking which interior widgets are promoted on which SubgraphNode instances. Graph-scoped by root graph ID to prevent cross-workflow state collision. Replaces `properties.proxyWidgets` parsing, `customRef` hacks, `widget.promoted` mutation, and the `subgraph-opened` event listener. - **`widgetValueStore`** — Graph-scoped `Map<WidgetKey, WidgetState>` that is the canonical owner of widget values. `BaseWidget.value` delegates to this store via getter/setter when a node ID is assigned. Eliminates the need for Proxy-based value forwarding. ### Synthetic widgets getter (SubgraphNode) `SubgraphNode.widgets` is now a getter that reads `promotionStore.getPromotions(rootGraphId, nodeId)` and returns cached `PromotedWidgetView` objects. No stubs, no Proxies, no fake widgets persisted in the array. The setter is a no-op — mutations go through `promotionStore`. ### PromotedWidgetView A class behind a `createPromotedWidgetView` factory, implementing the `PromotedWidgetView` interface. Delegates value/type/options/drawing to the resolved interior widget and stores. Owns positional state (`y`, `computedHeight`) for canvas layout. Cached by `PromotedWidgetViewManager` for object-identity stability across frames. ### DOM widget promotion Promoted DOM widgets (textarea, image upload, etc.) render on the SubgraphNode surface via `positionOverride` in `domWidgetStore`. `DomWidgets.vue` checks for overrides and uses the SubgraphNode's coordinates instead of the interior node's. ### Promoted previews New `usePromotedPreviews` composable resolves image/audio/video preview widgets from promoted entries, enabling SubgraphNodes to display previews of interior preview nodes. ### Deleted - `proxyWidget.ts` (257 lines) — Proxy handler, `Overlay`, `newProxyWidget`, `isProxyWidget` - `DisconnectedWidget.ts` (39 lines) — Singleton Proxy target - `useValueTransform.ts` (32 lines) — Replaced by store delegation ### Key architectural changes - `BaseWidget.value` getter/setter delegates to `widgetValueStore` when node ID is set - `LGraph.add()` reordered: `node.graph` assigned before widget `setNodeId` (enables store registration) - `LGraph.clear()` cleans up graph-scoped stores to prevent stale entries across workflow switches - `promotionStore` and `widgetValueStore` state nested under root graph UUID for multi-workflow isolation - `SubgraphNode.serialize()` writes promotions back to `properties.proxyWidgets` for persistence compatibility - Legacy `-1` promotion entries resolved and migrated on first load with dev warning ## Test coverage - **3,700+ lines of new/updated tests** across 36 test files - **Unit**: `promotionStore.test.ts`, `widgetValueStore.test.ts`, `promotedWidgetView.test.ts` (921 lines), `subgraphNodePromotion.test.ts`, `proxyWidgetUtils.test.ts`, `DomWidgets.test.ts`, `PromotedWidgetViewManager.test.ts`, `usePromotedPreviews.test.ts`, `resolvePromotedWidget.test.ts`, `subgraphPseudoWidgetCache.test.ts` - **E2E**: `subgraphPromotion.spec.ts` (622 lines) — promote/demote, manual/auto promotion, paste preservation, seed control augmentation, image preview promotion; `imagePreview.spec.ts` extended with multi-promoted-preview coverage - **Fixtures**: 2 new subgraph workflow fixtures for preview promotion scenarios ## Review focus - Graph-scoped store keying (`rootGraphId`) — verify isolation across workflows/tabs and cleanup on `LGraph.clear()` - `PromotedWidgetView` positional stability — `_arrangeWidgets` writes to `y`/`computedHeight` on cached objects; getter returns fresh array but stable object references - DOM widget position override lifecycle — overrides set on promote, cleared on demote/removal/subgraph navigation - Legacy `-1` entry migration — resolved and written back on first load; unresolvable entries dropped with dev warning - Serialization round-trip — `promotionStore` state → `properties.proxyWidgets` on serialize, hydrated back on configure ## Diff breakdown (excluding lockfile) - 153 files changed, ~7,500 insertions, ~1,900 deletions (excluding pnpm-lock.yaml churn) - ~3,700 lines are tests - ~300 lines deleted (proxyWidget.ts, DisconnectedWidget.ts, useValueTransform.ts) <!-- Fixes #ISSUE_NUMBER --> ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-8856-feat-synthetic-widgets-getter-for-SubgraphNode-proxy-widget-v2-3076d73d365081c7b517f5ec7cb514f3) by [Unito](https://www.unito.io) --------- Co-authored-by: Amp <amp@ampcode.com> Co-authored-by: github-actions <github-actions@github.com> Co-authored-by: GitHub Action <action@github.com>
439 lines
14 KiB
TypeScript
439 lines
14 KiB
TypeScript
import type { Locator } from '@playwright/test'
|
|
import { expect } from '@playwright/test'
|
|
|
|
import type { Keybinding } from '../../src/platform/keybindings/types'
|
|
import { comfyPageFixture as test } from '../fixtures/ComfyPage'
|
|
import { DefaultGraphPositions } from '../fixtures/constants/defaultGraphPositions'
|
|
import { TestIds } from '../fixtures/selectors'
|
|
|
|
test.beforeEach(async ({ comfyPage }) => {
|
|
await comfyPage.settings.setSetting('Comfy.UseNewMenu', 'Disabled')
|
|
})
|
|
|
|
test.describe('Load workflow warning', { tag: '@ui' }, () => {
|
|
test('Should display a warning when loading a workflow with missing nodes', async ({
|
|
comfyPage
|
|
}) => {
|
|
await comfyPage.workflow.loadWorkflow('missing/missing_nodes')
|
|
|
|
const missingNodesWarning = comfyPage.page.getByTestId(
|
|
TestIds.dialogs.missingNodes
|
|
)
|
|
await expect(missingNodesWarning).toBeVisible()
|
|
})
|
|
|
|
test('Should display a warning when loading a workflow with missing nodes in subgraphs', async ({
|
|
comfyPage
|
|
}) => {
|
|
await comfyPage.workflow.loadWorkflow('missing/missing_nodes_in_subgraph')
|
|
|
|
const missingNodesWarning = comfyPage.page.getByTestId(
|
|
TestIds.dialogs.missingNodes
|
|
)
|
|
await expect(missingNodesWarning).toBeVisible()
|
|
|
|
// Verify the missing node text includes subgraph context
|
|
const warningText = await missingNodesWarning.textContent()
|
|
expect(warningText).toContain('MISSING_NODE_TYPE_IN_SUBGRAPH')
|
|
expect(warningText).toContain('in subgraph')
|
|
})
|
|
})
|
|
|
|
test('Does not report warning on undo/redo', async ({ comfyPage }) => {
|
|
await comfyPage.settings.setSetting('Comfy.NodeSearchBoxImpl', 'v1 (legacy)')
|
|
const missingNodesWarning = comfyPage.page.getByTestId(
|
|
TestIds.dialogs.missingNodes
|
|
)
|
|
|
|
await comfyPage.workflow.loadWorkflow('missing/missing_nodes')
|
|
await expect(missingNodesWarning).toBeVisible()
|
|
await comfyPage.page.keyboard.press('Escape')
|
|
await expect(missingNodesWarning).not.toBeVisible()
|
|
|
|
// Wait for any async operations to complete after dialog closes
|
|
await comfyPage.nextFrame()
|
|
|
|
// Make a change to the graph
|
|
await comfyPage.canvasOps.doubleClick()
|
|
await comfyPage.searchBox.fillAndSelectFirstNode('KSampler')
|
|
|
|
// Undo and redo the change
|
|
await comfyPage.keyboard.undo()
|
|
await expect(async () => {
|
|
await expect(missingNodesWarning).not.toBeVisible()
|
|
}).toPass({ timeout: 5000 })
|
|
|
|
await comfyPage.keyboard.redo()
|
|
await expect(async () => {
|
|
await expect(missingNodesWarning).not.toBeVisible()
|
|
}).toPass({ timeout: 5000 })
|
|
})
|
|
|
|
test.describe('Execution error', () => {
|
|
test.beforeEach(async ({ comfyPage }) => {
|
|
await comfyPage.settings.setSetting('Comfy.UseNewMenu', 'Top')
|
|
await comfyPage.setup()
|
|
})
|
|
|
|
test('Should display an error message when an execution error occurs', async ({
|
|
comfyPage
|
|
}) => {
|
|
await comfyPage.workflow.loadWorkflow('nodes/execution_error')
|
|
await comfyPage.command.executeCommand('Comfy.QueuePrompt')
|
|
await comfyPage.nextFrame()
|
|
|
|
// Wait for the error overlay to be visible
|
|
const errorOverlay = comfyPage.page.locator('[data-testid="error-overlay"]')
|
|
await expect(errorOverlay).toBeVisible()
|
|
})
|
|
})
|
|
|
|
test.describe('Missing models warning', () => {
|
|
test.beforeEach(async ({ comfyPage }) => {
|
|
await comfyPage.settings.setSetting(
|
|
'Comfy.Workflow.ShowMissingModelsWarning',
|
|
true
|
|
)
|
|
await comfyPage.page.evaluate((url: string) => {
|
|
return fetch(`${url}/api/devtools/cleanup_fake_model`)
|
|
}, comfyPage.url)
|
|
})
|
|
|
|
test('Should display a warning when missing models are found', async ({
|
|
comfyPage
|
|
}) => {
|
|
await comfyPage.workflow.loadWorkflow('missing/missing_models')
|
|
|
|
const missingModelsWarning = comfyPage.page.locator('.comfy-missing-models')
|
|
await expect(missingModelsWarning).toBeVisible()
|
|
|
|
const downloadButton = missingModelsWarning.getByText('Download')
|
|
await expect(downloadButton).toBeVisible()
|
|
|
|
// Check that the copy URL button is also visible for Desktop environment
|
|
const copyUrlButton = missingModelsWarning.getByText('Copy URL')
|
|
await expect(copyUrlButton).toBeVisible()
|
|
})
|
|
|
|
test('Should display a warning when missing models are found in node properties', async ({
|
|
comfyPage
|
|
}) => {
|
|
// Load workflow that has a node with models metadata at the node level
|
|
await comfyPage.workflow.loadWorkflow(
|
|
'missing/missing_models_from_node_properties'
|
|
)
|
|
|
|
const missingModelsWarning = comfyPage.page.locator('.comfy-missing-models')
|
|
await expect(missingModelsWarning).toBeVisible()
|
|
|
|
const downloadButton = missingModelsWarning.getByText('Download')
|
|
await expect(downloadButton).toBeVisible()
|
|
|
|
// Check that the copy URL button is also visible for Desktop environment
|
|
const copyUrlButton = missingModelsWarning.getByText('Copy URL')
|
|
await expect(copyUrlButton).toBeVisible()
|
|
})
|
|
|
|
test('Should not display a warning when no missing models are found', async ({
|
|
comfyPage
|
|
}) => {
|
|
const modelFoldersRes = {
|
|
status: 200,
|
|
body: JSON.stringify([
|
|
{
|
|
name: 'text_encoders',
|
|
folders: ['ComfyUI/models/text_encoders']
|
|
}
|
|
])
|
|
}
|
|
await comfyPage.page.route(
|
|
'**/api/experiment/models',
|
|
(route) => route.fulfill(modelFoldersRes),
|
|
{ times: 1 }
|
|
)
|
|
|
|
// Reload page to trigger indexing of model folders
|
|
await comfyPage.setup()
|
|
|
|
const clipModelsRes = {
|
|
status: 200,
|
|
body: JSON.stringify([
|
|
{
|
|
name: 'fake_model.safetensors',
|
|
pathIndex: 0
|
|
}
|
|
])
|
|
}
|
|
await comfyPage.page.route(
|
|
'**/api/experiment/models/text_encoders',
|
|
(route) => route.fulfill(clipModelsRes),
|
|
{ times: 1 }
|
|
)
|
|
|
|
await comfyPage.workflow.loadWorkflow('missing/missing_models')
|
|
|
|
const missingModelsWarning = comfyPage.page.locator('.comfy-missing-models')
|
|
await expect(missingModelsWarning).not.toBeVisible()
|
|
})
|
|
|
|
test('Should not display warning when model metadata exists but widget values have changed', async ({
|
|
comfyPage
|
|
}) => {
|
|
// This tests the scenario where outdated model metadata exists in the workflow
|
|
// but the actual selected models (widget values) have changed
|
|
await comfyPage.workflow.loadWorkflow(
|
|
'missing/model_metadata_widget_mismatch'
|
|
)
|
|
|
|
// The missing models warning should NOT appear
|
|
const missingModelsWarning = comfyPage.page.locator('.comfy-missing-models')
|
|
await expect(missingModelsWarning).not.toBeVisible()
|
|
})
|
|
|
|
// Flaky test after parallelization
|
|
// https://github.com/Comfy-Org/ComfyUI_frontend/pull/1400
|
|
test.skip('Should download missing model when clicking download button', async ({
|
|
comfyPage
|
|
}) => {
|
|
// The fake_model.safetensors is served by
|
|
// https://github.com/Comfy-Org/ComfyUI_devtools/blob/main/__init__.py
|
|
await comfyPage.workflow.loadWorkflow('missing/missing_models')
|
|
|
|
const missingModelsWarning = comfyPage.page.locator('.comfy-missing-models')
|
|
await expect(missingModelsWarning).toBeVisible()
|
|
|
|
const downloadButton = comfyPage.page.getByText('Download')
|
|
await expect(downloadButton).toBeVisible()
|
|
const downloadPromise = comfyPage.page.waitForEvent('download')
|
|
await downloadButton.click()
|
|
|
|
const download = await downloadPromise
|
|
expect(download.suggestedFilename()).toBe('fake_model.safetensors')
|
|
})
|
|
|
|
test.describe('Do not show again checkbox', () => {
|
|
let checkbox: Locator
|
|
let closeButton: Locator
|
|
|
|
test.beforeEach(async ({ comfyPage }) => {
|
|
await comfyPage.settings.setSetting(
|
|
'Comfy.Workflow.ShowMissingModelsWarning',
|
|
true
|
|
)
|
|
await comfyPage.workflow.loadWorkflow('missing/missing_models')
|
|
|
|
checkbox = comfyPage.page.getByLabel("Don't show this again")
|
|
closeButton = comfyPage.page.getByLabel('Close')
|
|
})
|
|
|
|
test('Should disable warning dialog when checkbox is checked', async ({
|
|
comfyPage
|
|
}) => {
|
|
await checkbox.click()
|
|
const changeSettingPromise = comfyPage.page.waitForRequest(
|
|
'**/api/settings/Comfy.Workflow.ShowMissingModelsWarning'
|
|
)
|
|
await closeButton.click()
|
|
await changeSettingPromise
|
|
|
|
const settingValue = await comfyPage.settings.getSetting(
|
|
'Comfy.Workflow.ShowMissingModelsWarning'
|
|
)
|
|
expect(settingValue).toBe(false)
|
|
})
|
|
|
|
test('Should keep warning dialog enabled when checkbox is unchecked', async ({
|
|
comfyPage
|
|
}) => {
|
|
await closeButton.click()
|
|
|
|
const settingValue = await comfyPage.settings.getSetting(
|
|
'Comfy.Workflow.ShowMissingModelsWarning'
|
|
)
|
|
expect(settingValue).toBe(true)
|
|
})
|
|
})
|
|
})
|
|
|
|
test.describe('Settings', () => {
|
|
test('@mobile Should be visible on mobile', async ({ comfyPage }) => {
|
|
await comfyPage.page.keyboard.press('Control+,')
|
|
const settingsDialog = comfyPage.page.locator(
|
|
'[data-testid="settings-dialog"]'
|
|
)
|
|
await expect(settingsDialog).toBeVisible()
|
|
const contentArea = settingsDialog.locator('main')
|
|
await expect(contentArea).toBeVisible()
|
|
const isUsableHeight = await contentArea.evaluate(
|
|
(el) => el.clientHeight > 30
|
|
)
|
|
expect(isUsableHeight).toBeTruthy()
|
|
})
|
|
|
|
test('Can open settings with hotkey', async ({ comfyPage }) => {
|
|
await comfyPage.page.keyboard.down('ControlOrMeta')
|
|
await comfyPage.page.keyboard.press(',')
|
|
await comfyPage.page.keyboard.up('ControlOrMeta')
|
|
const settingsLocator = comfyPage.page.locator(
|
|
'[data-testid="settings-dialog"]'
|
|
)
|
|
await expect(settingsLocator).toBeVisible()
|
|
await comfyPage.page.keyboard.press('Escape')
|
|
await expect(settingsLocator).not.toBeVisible()
|
|
})
|
|
|
|
test('Can change canvas zoom speed setting', async ({ comfyPage }) => {
|
|
const maxSpeed = 2.5
|
|
await comfyPage.settings.setSetting('Comfy.Graph.ZoomSpeed', maxSpeed)
|
|
await test.step('Setting should persist', async () => {
|
|
expect(await comfyPage.settings.getSetting('Comfy.Graph.ZoomSpeed')).toBe(
|
|
maxSpeed
|
|
)
|
|
})
|
|
})
|
|
|
|
test('Should persist keybinding setting', async ({ comfyPage }) => {
|
|
// Open the settings dialog
|
|
await comfyPage.page.keyboard.press('Control+,')
|
|
await comfyPage.page.waitForSelector('[data-testid="settings-dialog"]')
|
|
|
|
// Open the keybinding tab
|
|
const settingsDialog = comfyPage.page.locator(
|
|
'[data-testid="settings-dialog"]'
|
|
)
|
|
await settingsDialog
|
|
.locator('nav [role="button"]', { hasText: 'Keybinding' })
|
|
.click()
|
|
await comfyPage.page.waitForSelector(
|
|
'[placeholder="Search Keybindings..."]'
|
|
)
|
|
|
|
// Focus the 'New Blank Workflow' row
|
|
const newBlankWorkflowRow = comfyPage.page.locator('tr', {
|
|
has: comfyPage.page.getByRole('cell', { name: 'New Blank Workflow' })
|
|
})
|
|
await newBlankWorkflowRow.click()
|
|
|
|
// Click edit button
|
|
const editKeybindingButton = newBlankWorkflowRow.locator('.pi-pencil')
|
|
await editKeybindingButton.click()
|
|
|
|
// Set new keybinding
|
|
const input = comfyPage.page.getByPlaceholder('Press keys for new binding')
|
|
await input.press('Alt+n')
|
|
|
|
const requestPromise = comfyPage.page.waitForRequest(
|
|
(req) =>
|
|
req.url().includes('/api/settings') &&
|
|
!req.url().includes('/api/settings/') &&
|
|
req.method() === 'POST'
|
|
)
|
|
|
|
// Save keybinding
|
|
const saveButton = comfyPage.page
|
|
.getByLabel('New Blank Workflow')
|
|
.getByText('Save')
|
|
await saveButton.click()
|
|
|
|
const request = await requestPromise
|
|
const expectedSetting: Keybinding = {
|
|
commandId: 'Comfy.NewBlankWorkflow',
|
|
combo: {
|
|
key: 'n',
|
|
ctrl: false,
|
|
alt: true,
|
|
shift: false
|
|
}
|
|
}
|
|
expect(request.postData()).toContain(JSON.stringify(expectedSetting))
|
|
})
|
|
})
|
|
|
|
test.describe('Support', () => {
|
|
test('Should open external zendesk link with OSS tag', async ({
|
|
comfyPage
|
|
}) => {
|
|
await comfyPage.settings.setSetting('Comfy.UseNewMenu', 'Top')
|
|
|
|
// Prevent loading the external page
|
|
await comfyPage.page
|
|
.context()
|
|
.route('https://support.comfy.org/**', (route) =>
|
|
route.fulfill({ body: '<html></html>', contentType: 'text/html' })
|
|
)
|
|
|
|
const popupPromise = comfyPage.page.waitForEvent('popup')
|
|
await comfyPage.menu.topbar.triggerTopbarCommand(['Help', 'Support'])
|
|
const popup = await popupPromise
|
|
|
|
const url = new URL(popup.url())
|
|
expect(url.hostname).toBe('support.comfy.org')
|
|
expect(url.searchParams.get('tf_42243568391700')).toBe('oss')
|
|
|
|
await popup.close()
|
|
})
|
|
})
|
|
|
|
test.describe('Error dialog', () => {
|
|
test('Should display an error dialog when graph configure fails', async ({
|
|
comfyPage
|
|
}) => {
|
|
await comfyPage.page.evaluate(() => {
|
|
const graph = window.graph!
|
|
;(graph as { configure: () => void }).configure = () => {
|
|
throw new Error('Error on configure!')
|
|
}
|
|
})
|
|
|
|
await comfyPage.workflow.loadWorkflow('default')
|
|
|
|
const errorDialog = comfyPage.page.locator('.comfy-error-report')
|
|
await expect(errorDialog).toBeVisible()
|
|
})
|
|
|
|
test('Should display an error dialog when prompt execution fails', async ({
|
|
comfyPage
|
|
}) => {
|
|
await comfyPage.page.evaluate(async () => {
|
|
const app = window.app!
|
|
app.api.queuePrompt = () => {
|
|
throw new Error('Error on queuePrompt!')
|
|
}
|
|
await app.queuePrompt(0)
|
|
})
|
|
const errorDialog = comfyPage.page.locator('.comfy-error-report')
|
|
await expect(errorDialog).toBeVisible()
|
|
})
|
|
})
|
|
|
|
test.describe('Signin dialog', () => {
|
|
test('Paste content to signin dialog should not paste node on canvas', async ({
|
|
comfyPage
|
|
}) => {
|
|
const nodeNum = await comfyPage.nodeOps.getNodeCount()
|
|
await comfyPage.canvas.click({
|
|
position: DefaultGraphPositions.emptyLatentWidgetClick
|
|
})
|
|
await comfyPage.page.mouse.move(10, 10)
|
|
await comfyPage.nextFrame()
|
|
await comfyPage.clipboard.copy()
|
|
|
|
const textBox = comfyPage.widgetTextBox
|
|
await textBox.click()
|
|
await textBox.fill('test_password')
|
|
await textBox.press('Control+a')
|
|
await textBox.press('Control+c')
|
|
|
|
await comfyPage.page.evaluate(() => {
|
|
void window.app!.extensionManager.dialog.showSignInDialog()
|
|
})
|
|
|
|
const input = comfyPage.page.locator('#comfy-org-sign-in-password')
|
|
await input.waitFor({ state: 'visible' })
|
|
await input.press('Control+v')
|
|
await expect(input).toHaveValue('test_password')
|
|
|
|
expect(await comfyPage.nodeOps.getNodeCount()).toBe(nodeNum)
|
|
})
|
|
})
|