diff --git a/browser_tests/fixtures/utils/subgraphUtils.ts b/browser_tests/fixtures/utils/subgraphUtils.ts new file mode 100644 index 000000000..c488d5f08 --- /dev/null +++ b/browser_tests/fixtures/utils/subgraphUtils.ts @@ -0,0 +1,58 @@ +/** + * Represents a subgraph's graph object with inputs and outputs slots. + */ +export interface SubgraphGraph { + inputs: SubgraphSlot[] + outputs: SubgraphSlot[] +} + +export interface SubgraphSlot { + label?: string + name?: string + displayName?: string +} + +/** + * Type guard to check if a graph object is a subgraph. + */ +export function isSubgraph(graph: unknown): graph is SubgraphGraph { + return ( + graph !== null && + typeof graph === 'object' && + 'inputs' in graph && + 'outputs' in graph && + Array.isArray((graph as SubgraphGraph).inputs) && + Array.isArray((graph as SubgraphGraph).outputs) + ) +} + +/** + * Assertion function that throws if the graph is not a subgraph. + */ +export function assertSubgraph( + graph: unknown, + message = 'Not in subgraph' +): asserts graph is SubgraphGraph { + if (!isSubgraph(graph)) { + throw new Error(message) + } +} + +/** + * Inline assertion for use inside page.evaluate() browser context. + * Returns a string that can be used with Function constructor or eval. + */ +export const SUBGRAPH_ASSERT_INLINE = ` + const assertSubgraph = (graph) => { + if ( + graph === null || + typeof graph !== 'object' || + !('inputs' in graph) || + !('outputs' in graph) || + !Array.isArray(graph.inputs) || + !Array.isArray(graph.outputs) + ) { + throw new Error('Not in subgraph'); + } + }; +` diff --git a/browser_tests/tests/commands.spec.ts b/browser_tests/tests/commands.spec.ts index 0a355cf54..78f289954 100644 --- a/browser_tests/tests/commands.spec.ts +++ b/browser_tests/tests/commands.spec.ts @@ -9,33 +9,25 @@ test.beforeEach(async ({ comfyPage }) => { test.describe('Keybindings', () => { test('Should execute command', async ({ comfyPage }) => { await comfyPage.registerCommand('TestCommand', () => { - ;(window as unknown as Record)['foo'] = true + window.foo = true }) await comfyPage.executeCommand('TestCommand') - expect( - await comfyPage.page.evaluate( - () => (window as unknown as Record)['foo'] - ) - ).toBe(true) + expect(await comfyPage.page.evaluate(() => window.foo)).toBe(true) }) test('Should execute async command', async ({ comfyPage }) => { await comfyPage.registerCommand('TestCommand', async () => { await new Promise((resolve) => setTimeout(() => { - ;(window as unknown as Record)['foo'] = true + window.foo = true resolve() }, 5) ) }) await comfyPage.executeCommand('TestCommand') - expect( - await comfyPage.page.evaluate( - () => (window as unknown as Record)['foo'] - ) - ).toBe(true) + expect(await comfyPage.page.evaluate(() => window.foo)).toBe(true) }) test('Should handle command errors', async ({ comfyPage }) => { diff --git a/browser_tests/tests/extensionAPI.spec.ts b/browser_tests/tests/extensionAPI.spec.ts index da878ae65..beef8c4b4 100644 --- a/browser_tests/tests/extensionAPI.spec.ts +++ b/browser_tests/tests/extensionAPI.spec.ts @@ -19,7 +19,7 @@ test.describe('Topbar commands', () => { id: 'foo', label: 'foo-command', function: () => { - ;(window as unknown as Record)['foo'] = true + window.foo = true } } ], @@ -33,11 +33,7 @@ test.describe('Topbar commands', () => { }) await comfyPage.menu.topbar.triggerTopbarCommand(['ext', 'foo-command']) - expect( - await comfyPage.page.evaluate( - () => (window as unknown as Record)['foo'] - ) - ).toBe(true) + expect(await comfyPage.page.evaluate(() => window.foo)).toBe(true) }) test('Should not allow register command defined in other extension', async ({ @@ -72,8 +68,7 @@ test.describe('Topbar commands', () => { { id: 'TestCommand', function: () => { - ;(window as unknown as Record)['TestCommand'] = - true + window.TestCommand = true } } ], @@ -87,11 +82,7 @@ test.describe('Topbar commands', () => { }) await comfyPage.page.keyboard.press('k') - expect( - await comfyPage.page.evaluate( - () => (window as unknown as Record)['TestCommand'] - ) - ).toBe(true) + expect(await comfyPage.page.evaluate(() => window.TestCommand)).toBe(true) }) test.describe('Settings', () => { @@ -108,19 +99,14 @@ test.describe('Topbar commands', () => { type: 'text', defaultValue: 'Hello, world!', onChange: () => { - const win = window as unknown as Record - win['changeCount'] = ((win['changeCount'] as number) ?? 0) + 1 + window.changeCount = (window.changeCount ?? 0) + 1 } } as unknown as SettingParams ] }) }) // onChange is called when the setting is first added - expect( - await comfyPage.page.evaluate( - () => (window as unknown as Record)['changeCount'] - ) - ).toBe(1) + expect(await comfyPage.page.evaluate(() => window.changeCount)).toBe(1) expect(await comfyPage.getSetting('TestSetting' as string)).toBe( 'Hello, world!' ) @@ -129,11 +115,7 @@ test.describe('Topbar commands', () => { expect(await comfyPage.getSetting('TestSetting' as string)).toBe( 'Hello, universe!' ) - expect( - await comfyPage.page.evaluate( - () => (window as unknown as Record)['changeCount'] - ) - ).toBe(2) + expect(await comfyPage.page.evaluate(() => window.changeCount)).toBe(2) }) test('Should allow setting boolean settings', async ({ comfyPage }) => { @@ -149,8 +131,7 @@ test.describe('Topbar commands', () => { type: 'boolean', defaultValue: false, onChange: () => { - const win = window as unknown as Record - win['changeCount'] = ((win['changeCount'] as number) ?? 0) + 1 + window.changeCount = (window.changeCount ?? 0) + 1 } } as unknown as SettingParams ] @@ -160,11 +141,7 @@ test.describe('Topbar commands', () => { expect(await comfyPage.getSetting('Comfy.TestSetting' as string)).toBe( false ) - expect( - await comfyPage.page.evaluate( - () => (window as unknown as Record)['changeCount'] - ) - ).toBe(1) + expect(await comfyPage.page.evaluate(() => window.changeCount)).toBe(1) await comfyPage.settingDialog.open() await comfyPage.settingDialog.toggleBooleanSetting( @@ -173,11 +150,7 @@ test.describe('Topbar commands', () => { expect(await comfyPage.getSetting('Comfy.TestSetting' as string)).toBe( true ) - expect( - await comfyPage.page.evaluate( - () => (window as unknown as Record)['changeCount'] - ) - ).toBe(2) + expect(await comfyPage.page.evaluate(() => window.changeCount)).toBe(2) }) test.describe('Passing through attrs to setting components', () => { @@ -303,16 +276,14 @@ test.describe('Topbar commands', () => { message: 'Test Prompt Message' }) .then((value: string | null) => { - ;(window as unknown as Record)['value'] = value + window.value = value }) }) await comfyPage.fillPromptDialog('Hello, world!') - expect( - await comfyPage.page.evaluate( - () => (window as unknown as Record)['value'] - ) - ).toBe('Hello, world!') + expect(await comfyPage.page.evaluate(() => window.value)).toBe( + 'Hello, world!' + ) }) test('Should allow showing a confirmation dialog', async ({ @@ -327,39 +298,31 @@ test.describe('Topbar commands', () => { message: 'Test Confirm Message' }) .then((value: boolean | null) => { - ;(window as unknown as Record)['value'] = value + window.value = value }) }) await comfyPage.confirmDialog.click('confirm') - expect( - await comfyPage.page.evaluate( - () => (window as unknown as Record)['value'] - ) - ).toBe(true) + expect(await comfyPage.page.evaluate(() => window.value)).toBe(true) }) test('Should allow dismissing a dialog', async ({ comfyPage }) => { await comfyPage.page.evaluate(() => { const app = window['app'] if (!app) throw new Error('App not initialized') - ;(window as unknown as Record)['value'] = 'foo' + window.value = 'foo' void app.extensionManager.dialog .confirm({ title: 'Test Confirm', message: 'Test Confirm Message' }) .then((value: boolean | null) => { - ;(window as unknown as Record)['value'] = value + window.value = value }) }) await comfyPage.confirmDialog.click('reject') - expect( - await comfyPage.page.evaluate( - () => (window as unknown as Record)['value'] - ) - ).toBeNull() + expect(await comfyPage.page.evaluate(() => window.value)).toBeNull() }) }) @@ -383,9 +346,7 @@ test.describe('Topbar commands', () => { label: 'Test Command', icon: 'pi pi-star', function: () => { - ;(window as unknown as Record)[ - 'selectionCommandExecuted' - ] = true + window.selectionCommandExecuted = true } } ], @@ -403,12 +364,7 @@ test.describe('Topbar commands', () => { // Verify the command was executed expect( - await comfyPage.page.evaluate( - () => - (window as unknown as Record)[ - 'selectionCommandExecuted' - ] - ) + await comfyPage.page.evaluate(() => window.selectionCommandExecuted) ).toBe(true) }) }) diff --git a/browser_tests/tests/keybindings.spec.ts b/browser_tests/tests/keybindings.spec.ts index 81e6e19f4..dd03ca590 100644 --- a/browser_tests/tests/keybindings.spec.ts +++ b/browser_tests/tests/keybindings.spec.ts @@ -11,25 +11,23 @@ test.describe('Keybindings', () => { comfyPage }) => { await comfyPage.registerKeybinding({ key: 'k' }, () => { - ;(window as unknown as Record)['TestCommand'] = true + window.TestCommand = true }) const textBox = comfyPage.widgetTextBox await textBox.click() await textBox.fill('k') await expect(textBox).toHaveValue('k') - expect( - await comfyPage.page.evaluate( - () => (window as unknown as Record)['TestCommand'] - ) - ).toBe(undefined) + expect(await comfyPage.page.evaluate(() => window.TestCommand)).toBe( + undefined + ) }) test('Should not trigger modifier keybinding when typing in input fields', async ({ comfyPage }) => { await comfyPage.registerKeybinding({ key: 'k', ctrl: true }, () => { - ;(window as unknown as Record)['TestCommand'] = true + window.TestCommand = true }) const textBox = comfyPage.widgetTextBox @@ -37,28 +35,22 @@ test.describe('Keybindings', () => { await textBox.fill('q') await textBox.press('Control+k') await expect(textBox).toHaveValue('q') - expect( - await comfyPage.page.evaluate( - () => (window as unknown as Record)['TestCommand'] - ) - ).toBe(true) + expect(await comfyPage.page.evaluate(() => window.TestCommand)).toBe(true) }) test('Should not trigger keybinding reserved by text input when typing in input fields', async ({ comfyPage }) => { await comfyPage.registerKeybinding({ key: 'Ctrl+v' }, () => { - ;(window as unknown as Record)['TestCommand'] = true + window.TestCommand = true }) const textBox = comfyPage.widgetTextBox await textBox.click() await textBox.press('Control+v') await expect(textBox).toBeFocused() - expect( - await comfyPage.page.evaluate( - () => (window as unknown as Record)['TestCommand'] - ) - ).toBe(undefined) + expect(await comfyPage.page.evaluate(() => window.TestCommand)).toBe( + undefined + ) }) }) diff --git a/browser_tests/tests/subgraph-rename-dialog.spec.ts b/browser_tests/tests/subgraph-rename-dialog.spec.ts index 5d566c077..277aab7e1 100644 --- a/browser_tests/tests/subgraph-rename-dialog.spec.ts +++ b/browser_tests/tests/subgraph-rename-dialog.spec.ts @@ -26,14 +26,30 @@ test.describe('Subgraph Slot Rename Dialog', () => { // Get initial slot label const initialInputLabel = await comfyPage.page.evaluate(() => { + const assertSubgraph = ( + graph: unknown + ): asserts graph is { + inputs: { label?: string; name?: string }[] + outputs: unknown[] + } => { + if ( + graph === null || + typeof graph !== 'object' || + !('inputs' in graph) || + !('outputs' in graph) || + !Array.isArray((graph as { inputs: unknown }).inputs) || + !Array.isArray((graph as { outputs: unknown }).outputs) + ) { + throw new Error('Not in subgraph') + } + } const app = window['app'] if (!app) throw new Error('App not available') const canvas = app.canvas if (!canvas) throw new Error('Canvas not available') const graph = canvas.graph - if (!graph || !('inputs' in graph)) throw new Error('Not in subgraph') - const inputs = graph.inputs as { label?: string; name?: string }[] - return inputs?.[0]?.label || inputs?.[0]?.name || null + assertSubgraph(graph) + return graph.inputs[0]?.label || graph.inputs[0]?.name || null }) // First rename @@ -60,18 +76,30 @@ test.describe('Subgraph Slot Rename Dialog', () => { // Verify the rename worked const afterFirstRename = await comfyPage.page.evaluate(() => { + const assertSubgraph = ( + graph: unknown + ): asserts graph is { + inputs: { label?: string; name?: string; displayName?: string }[] + outputs: unknown[] + } => { + if ( + graph === null || + typeof graph !== 'object' || + !('inputs' in graph) || + !('outputs' in graph) || + !Array.isArray((graph as { inputs: unknown }).inputs) || + !Array.isArray((graph as { outputs: unknown }).outputs) + ) { + throw new Error('Not in subgraph') + } + } const app = window['app'] if (!app) throw new Error('App not available') const canvas = app.canvas if (!canvas) throw new Error('Canvas not available') const graph = canvas.graph - if (!graph || !('inputs' in graph)) throw new Error('Not in subgraph') - const inputs = graph.inputs as { - label?: string - name?: string - displayName?: string - }[] - const slot = inputs?.[0] + assertSubgraph(graph) + const slot = graph.inputs[0] return { label: slot?.label || null, name: slot?.name || null, @@ -112,14 +140,30 @@ test.describe('Subgraph Slot Rename Dialog', () => { // Verify the second rename worked const afterSecondRename = await comfyPage.page.evaluate(() => { + const assertSubgraph = ( + graph: unknown + ): asserts graph is { + inputs: { label?: string }[] + outputs: unknown[] + } => { + if ( + graph === null || + typeof graph !== 'object' || + !('inputs' in graph) || + !('outputs' in graph) || + !Array.isArray((graph as { inputs: unknown }).inputs) || + !Array.isArray((graph as { outputs: unknown }).outputs) + ) { + throw new Error('Not in subgraph') + } + } const app = window['app'] if (!app) throw new Error('App not available') const canvas = app.canvas if (!canvas) throw new Error('Canvas not available') const graph = canvas.graph - if (!graph || !('inputs' in graph)) throw new Error('Not in subgraph') - const inputs = graph.inputs as { label?: string }[] - return inputs?.[0]?.label || null + assertSubgraph(graph) + return graph.inputs[0]?.label || null }) expect(afterSecondRename).toBe(SECOND_RENAMED_NAME) }) @@ -134,14 +178,30 @@ test.describe('Subgraph Slot Rename Dialog', () => { // Get initial output slot label const initialOutputLabel = await comfyPage.page.evaluate(() => { + const assertSubgraph = ( + graph: unknown + ): asserts graph is { + inputs: unknown[] + outputs: { label?: string; name?: string }[] + } => { + if ( + graph === null || + typeof graph !== 'object' || + !('inputs' in graph) || + !('outputs' in graph) || + !Array.isArray((graph as { inputs: unknown }).inputs) || + !Array.isArray((graph as { outputs: unknown }).outputs) + ) { + throw new Error('Not in subgraph') + } + } const app = window['app'] if (!app) throw new Error('App not available') const canvas = app.canvas if (!canvas) throw new Error('Canvas not available') const graph = canvas.graph - if (!graph || !('outputs' in graph)) throw new Error('Not in subgraph') - const outputs = graph.outputs as { label?: string; name?: string }[] - return outputs?.[0]?.label || outputs?.[0]?.name || null + assertSubgraph(graph) + return graph.outputs[0]?.label || graph.outputs[0]?.name || null }) // First rename diff --git a/browser_tests/tests/widget.spec.ts b/browser_tests/tests/widget.spec.ts index 23c804f73..6d461e79f 100644 --- a/browser_tests/tests/widget.spec.ts +++ b/browser_tests/tests/widget.spec.ts @@ -112,16 +112,14 @@ test.describe('Slider widget', () => { if (!app?.graph?.nodes?.[0]?.widgets?.[0]) return const widget = app.graph.nodes[0].widgets[0] widget.callback = (value: number) => { - ;(window as unknown as Record)['widgetValue'] = value + window.widgetValue = value } }) await widget.dragHorizontal(50) await expect(comfyPage.canvas).toHaveScreenshot('slider_widget_dragged.png') expect( - await comfyPage.page.evaluate( - () => (window as unknown as Record)['widgetValue'] - ) + await comfyPage.page.evaluate(() => window.widgetValue) ).toBeDefined() }) }) @@ -137,16 +135,14 @@ test.describe('Number widget', () => { if (!app?.graph?.nodes?.[0]?.widgets?.[0]) return const widget = app.graph.nodes[0].widgets[0] widget.callback = (value: number) => { - ;(window as unknown as Record)['widgetValue'] = value + window.widgetValue = value } }) await widget.dragHorizontal(50) await expect(comfyPage.canvas).toHaveScreenshot('seed_widget_dragged.png') expect( - await comfyPage.page.evaluate( - () => (window as unknown as Record)['widgetValue'] - ) + await comfyPage.page.evaluate(() => window.widgetValue) ).toBeDefined() }) }) diff --git a/browser_tests/tsconfig.json b/browser_tests/tsconfig.json index 391298333..334841eec 100644 --- a/browser_tests/tsconfig.json +++ b/browser_tests/tsconfig.json @@ -9,5 +9,6 @@ }, "include": [ "**/*.ts", + "types/**/*.d.ts" ] } diff --git a/browser_tests/types/global.d.ts b/browser_tests/types/global.d.ts new file mode 100644 index 000000000..8ca92ba7a --- /dev/null +++ b/browser_tests/types/global.d.ts @@ -0,0 +1,21 @@ +import type { ComfyApp } from '@/scripts/app' +import type { LGraphBadge, LiteGraph } from '@/lib/litegraph/src/litegraph' + +declare global { + interface Window { + // Application globals + app?: ComfyApp + LiteGraph?: typeof LiteGraph + LGraphBadge?: typeof LGraphBadge + + // Test-only properties used in browser tests + TestCommand?: boolean + foo?: unknown + value?: unknown + widgetValue?: unknown + selectionCommandExecuted?: boolean + changeCount?: number + } +} + +export {} diff --git a/src/scripts/app.ts b/src/scripts/app.ts index da0af6f44..1a258f6a4 100644 --- a/src/scripts/app.ts +++ b/src/scripts/app.ts @@ -37,6 +37,7 @@ import type { NodeExecutionOutput, ResultItem } from '@/schemas/apiSchema' +import type { WorkflowAsGraph } from '@/types/workflowSchemaTypes' import { type ComfyNodeDef as ComfyNodeDefV1, isComboInputSpecV1, @@ -1194,12 +1195,8 @@ export class ComfyApp { } try { - // TODO: Align ComfyWorkflowJSON (Zod schema) with ISerialisedGraph (litegraph types) - // The schemas are structurally compatible at runtime but TypeScript can't verify this. - // See: https://github.com/Comfy-Org/ComfyUI_frontend/issues/XXXX - this.rootGraph.configure( - graphData as Parameters[0] - ) + // Type cast: ComfyWorkflowJSON → WorkflowAsGraph (see WorkflowAsGraph docs) + this.rootGraph.configure(graphData as WorkflowAsGraph) // Save original renderer version before scaling (it gets modified during scaling) const originalMainGraphRenderer = diff --git a/src/scripts/domWidget.ts b/src/scripts/domWidget.ts index 10463fefe..aa0b8dd14 100644 --- a/src/scripts/domWidget.ts +++ b/src/scripts/domWidget.ts @@ -197,19 +197,23 @@ abstract class BaseDOMWidgetImpl useDomWidgetStore().unregisterWidget(this.id) } - override createCopyForNode(node: LGraphNode): this { - const constructorArg = { - node: node, + /** + * Creates the constructor arguments for cloning this widget. + * Subclasses should override to add their specific properties. + */ + protected getCloneArgs(node: LGraphNode): Record { + return { + node, name: this.name, type: this.type, options: this.options } - const cloned: this = new (this.constructor as new ( - obj: typeof constructorArg - ) => this)(constructorArg) + } + + override createCopyForNode(node: LGraphNode): this { + const Ctor = this.constructor as new (args: Record) => this + const cloned = new Ctor(this.getCloneArgs(node)) cloned.value = this.value - // Preserve the Y position from the original widget to maintain proper positioning - // when widgets are promoted through subgraph nesting cloned.y = this.y return cloned } @@ -232,22 +236,11 @@ export class DOMWidgetImpl this.element = obj.element } - override createCopyForNode(node: LGraphNode): this { - const constructorArg = { - node: node, - name: this.name, - type: this.type, - element: this.element, - options: this.options + protected override getCloneArgs(node: LGraphNode): Record { + return { + ...super.getCloneArgs(node), + element: this.element } - const cloned: this = new (this.constructor as new ( - obj: typeof constructorArg - ) => this)(constructorArg) - cloned.value = this.value - // Preserve the Y position from the original widget to maintain proper positioning - // when widgets are promoted through subgraph nesting - cloned.y = this.y - return cloned } /** Extract DOM widget size info */ @@ -322,6 +315,15 @@ export class ComponentWidgetImpl< this.props = obj.props } + protected override getCloneArgs(node: LGraphNode): Record { + return { + ...super.getCloneArgs(node), + component: this.component, + inputSpec: this.inputSpec, + props: this.props + } + } + override computeLayoutSize() { const minHeight = this.options.getMinHeight?.() ?? 50 const maxHeight = this.options.getMaxHeight?.() diff --git a/src/types/workflowSchemaTypes.ts b/src/types/workflowSchemaTypes.ts new file mode 100644 index 000000000..9f28307d5 --- /dev/null +++ b/src/types/workflowSchemaTypes.ts @@ -0,0 +1,14 @@ +import type { ISerialisedGraph } from '@/lib/litegraph/src/types/serialisation' +import type { ComfyWorkflowJSON } from '@/platform/workflow/validation/schemas/workflowSchema' + +/** + * ComfyWorkflowJSON and ISerialisedGraph are structurally compatible + * at runtime. This type alias documents the intentional cast between them. + * + * ComfyWorkflowJSON is the Zod-validated workflow schema from the frontend. + * ISerialisedGraph is the LiteGraph serialization format. + * + * TODO: Align these schemas to eliminate the need for this cast. + * @see https://github.com/Comfy-Org/ComfyUI_frontend/issues/XXXX + */ +export type WorkflowAsGraph = ComfyWorkflowJSON & ISerialisedGraph