refactor: improve type safety patterns from @ts-expect-error cleanup

Amp-Thread-ID: https://ampcode.com/threads/T-019bb3bd-f607-735a-b1a8-fce5fe4f0125
Co-authored-by: Amp <amp@ampcode.com>

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');
+    }
+  };
+`

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<string, unknown>)['foo'] = true
+      window.foo = true
     })
 
     await comfyPage.executeCommand('TestCommand')
-    expect(
-      await comfyPage.page.evaluate(
-        () => (window as unknown as Record<string, unknown>)['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<void>((resolve) =>
         setTimeout(() => {
-          ;(window as unknown as Record<string, unknown>)['foo'] = true
+          window.foo = true
           resolve()
         }, 5)
       )
     })
 
     await comfyPage.executeCommand('TestCommand')
-    expect(
-      await comfyPage.page.evaluate(
-        () => (window as unknown as Record<string, unknown>)['foo']
-      )
-    ).toBe(true)
+    expect(await comfyPage.page.evaluate(() => window.foo)).toBe(true)
   })
 
   test('Should handle command errors', async ({ comfyPage }) => {

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<string, unknown>)['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<string, unknown>)['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<string, unknown>)['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<string, unknown>)['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<string, unknown>
-                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<string, unknown>)['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<string, unknown>)['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<string, unknown>
-                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<string, unknown>)['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<string, unknown>)['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<string, unknown>)['value'] = value
+            window.value = value
           })
       })
 
       await comfyPage.fillPromptDialog('Hello, world!')
-      expect(
-        await comfyPage.page.evaluate(
-          () => (window as unknown as Record<string, unknown>)['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<string, unknown>)['value'] = value
+            window.value = value
           })
       })
 
       await comfyPage.confirmDialog.click('confirm')
-      expect(
-        await comfyPage.page.evaluate(
-          () => (window as unknown as Record<string, unknown>)['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<string, unknown>)['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<string, unknown>)['value'] = value
+            window.value = value
           })
       })
 
       await comfyPage.confirmDialog.click('reject')
-      expect(
-        await comfyPage.page.evaluate(
-          () => (window as unknown as Record<string, unknown>)['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<string, unknown>)[
-                  '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<string, unknown>)[
-              'selectionCommandExecuted'
-            ]
-        )
+        await comfyPage.page.evaluate(() => window.selectionCommandExecuted)
       ).toBe(true)
     })
   })

browser_tests/tests/keybindings.spec.ts

@@ -11,25 +11,23 @@ test.describe('Keybindings', () => {
     comfyPage
   }) => {
     await comfyPage.registerKeybinding({ key: 'k' }, () => {
-      ;(window as unknown as Record<string, unknown>)['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<string, unknown>)['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<string, unknown>)['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<string, unknown>)['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<string, unknown>)['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<string, unknown>)['TestCommand']
-      )
-    ).toBe(undefined)
+    expect(await comfyPage.page.evaluate(() => window.TestCommand)).toBe(
+      undefined
+    )
   })
 })

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

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<string, unknown>)['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<string, unknown>)['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<string, unknown>)['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<string, unknown>)['widgetValue']
-      )
+      await comfyPage.page.evaluate(() => window.widgetValue)
     ).toBeDefined()
   })
 })

browser_tests/tsconfig.json

@@ -9,5 +9,6 @@
   },
   "include": [
     "**/*.ts",
+    "types/**/*.d.ts"
   ]
 }

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 {}

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<typeof this.rootGraph.configure>[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 =

src/scripts/domWidget.ts

@@ -197,19 +197,23 @@ abstract class BaseDOMWidgetImpl<V extends object | string>
     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<string, unknown> {
+    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<string, unknown>) => 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<T extends HTMLElement, V extends object | string>
     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<string, unknown> {
+    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<string, unknown> {
+    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?.()

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