Fix copy paste of widget value (#284)

* Fix copy paste of widget value * Fix ui tests * Allow undefined group font size * Update test expectations [skip ci] * nit --------- Co-authored-by: github-actions <[email protected]>
Comfy-Org · Aug 3, 2024 · 0cf5e64 · 0cf5e64
1 parent f0f8674
commit 0cf5e64
Show file tree

Hide file tree

Showing 8 changed files with 93 additions and 88 deletions.
diff --git a/browser_tests/copyPaste.spec.ts b/browser_tests/copyPaste.spec.ts
@@ -22,6 +22,28 @@ test.describe('Copy Paste', () => {
     expect(resultString).toBe(originalString + originalString)
   })
 
+  test('Can copy and paste widget value', async ({ comfyPage }) => {
+    // Copy width value (512) from empty latent node to KSampler's seed.
+    // Empty latent node's width
+    await comfyPage.canvas.click({
+      position: {
+        x: 718,
+        y: 643
+      }
+    })
+    await comfyPage.ctrlC()
+    // KSampler's seed
+    await comfyPage.canvas.click({
+      position: {
+        x: 1005,
+        y: 281
+      }
+    })
+    await comfyPage.ctrlV()
+    await comfyPage.page.keyboard.press('Enter')
+    await expect(comfyPage.canvas).toHaveScreenshot('copied-widget-value.png')
+  })
+
   /**
    * https://github.com/Comfy-Org/ComfyUI_frontend/issues/98
    */

diff --git a/browser_tests/copyPaste.spec.ts-snapshots/copied-widget-value-chromium-linux.png b/browser_tests/copyPaste.spec.ts-snapshots/copied-widget-value-chromium-linux.png
diff --git a/src/scripts/app.ts b/src/scripts/app.ts
@@ -17,7 +17,7 @@ import { applyTextReplacements, addStylesheet } from './utils'
 import type { ComfyExtension } from '@/types/comfy'
 import {
   type ComfyWorkflowJSON,
-  parseComfyWorkflow
+  validateComfyWorkflow
 } from '../types/comfyWorkflow'
 import { ComfyNodeDef } from '@/types/apiTypes'
 import { lightenColor } from '@/utils/colorUtil'
@@ -1114,12 +1114,12 @@ export class ComfyApp {
       let workflow: ComfyWorkflowJSON
       try {
         data = data.slice(data.indexOf('{'))
-        workflow = await parseComfyWorkflow(data)
+        workflow = JSON.parse(data)
       } catch (err) {
         try {
           data = data.slice(data.indexOf('workflow\n'))
           data = data.slice(data.indexOf('{'))
-          workflow = await parseComfyWorkflow(data)
+          workflow = JSON.parse(data)
         } catch (error) {
           console.error(error)
         }
@@ -1906,7 +1906,7 @@ export class ComfyApp {
     try {
       const loadWorkflow = async (json) => {
         if (json) {
-          const workflow = await parseComfyWorkflow(json)
+          const workflow = JSON.parse(json)
           const workflowName = getStorageValue('Comfy.PreviousWorkflow')
           await this.loadGraphData(workflow, true, true, workflowName)
           return true
@@ -2236,6 +2236,9 @@ export class ComfyApp {
       console.error(error)
     }
 
+    graphData = await validateComfyWorkflow(graphData, /* onError=*/ alert)
+    if (!graphData) return
+
     const missingNodeTypes = []
     await this.#invokeExtensionsAsync(
       'beforeConfigureGraph',
@@ -2662,7 +2665,7 @@ export class ComfyApp {
       const pngInfo = await getPngMetadata(file)
       if (pngInfo?.workflow) {
         await this.loadGraphData(
-          await parseComfyWorkflow(pngInfo.workflow),
+          JSON.parse(pngInfo.workflow),
           true,
           true,
           fileName
@@ -2683,12 +2686,7 @@ export class ComfyApp {
       const prompt = pngInfo?.prompt || pngInfo?.Prompt
 
       if (workflow) {
-        this.loadGraphData(
-          await parseComfyWorkflow(workflow),
-          true,
-          true,
-          fileName
-        )
+        this.loadGraphData(JSON.parse(workflow), true, true, fileName)
       } else if (prompt) {
         this.loadApiJson(JSON.parse(prompt), fileName)
       } else {
@@ -2700,12 +2698,7 @@ export class ComfyApp {
       const prompt = pngInfo?.prompt || pngInfo?.Prompt
 
       if (workflow) {
-        this.loadGraphData(
-          await parseComfyWorkflow(workflow),
-          true,
-          true,
-          fileName
-        )
+        this.loadGraphData(JSON.parse(workflow), true, true, fileName)
       } else if (prompt) {
         this.loadApiJson(JSON.parse(prompt), fileName)
       } else {
@@ -2724,11 +2717,7 @@ export class ComfyApp {
         } else if (this.isApiJson(jsonContent)) {
           this.loadApiJson(jsonContent, fileName)
         } else {
-          await this.loadGraphData(
-            await parseComfyWorkflow(readerResult),
-            true,
-            fileName
-          )
+          await this.loadGraphData(JSON.parse(readerResult), true, fileName)
         }
       }
       reader.readAsText(file)
@@ -2742,7 +2731,7 @@ export class ComfyApp {
       if (info.workflow) {
         await this.loadGraphData(
           // @ts-ignore
-          await parseComfyWorkflow(info.workflow),
+          JSON.parse(info.workflow),
           true,
           true,
           fileName

diff --git a/src/types/comfyWorkflow.ts b/src/types/comfyWorkflow.ts
@@ -74,7 +74,7 @@ const zGroup = z
     title: z.string(),
     bounding: z.tuple([z.number(), z.number(), z.number(), z.number()]),
     color: z.string(),
-    font_size: z.number(),
+    font_size: z.number().optional(),
     locked: z.boolean().optional()
   })
   .passthrough()
@@ -131,16 +131,15 @@ export type ComfyLink = z.infer<typeof zComfyLink>
 export type ComfyNode = z.infer<typeof zComfyNode>
 export type ComfyWorkflowJSON = z.infer<typeof zComfyWorkflow>
 
-export async function parseComfyWorkflow(
-  data: string
-): Promise<ComfyWorkflowJSON> {
-  // Validate
-  const result = await zComfyWorkflow.safeParseAsync(JSON.parse(data))
+export async function validateComfyWorkflow(
+  data: any,
+  onError: (error: string) => void = console.warn
+): Promise<ComfyWorkflowJSON | null> {
+  const result = await zComfyWorkflow.safeParseAsync(data)
   if (!result.success) {
-    // TODO: Pretty print the error on UI modal.
     const error = fromZodError(result.error)
-    alert(`Invalid workflow against zod schema:\n${error}`)
-    throw error
+    onError(`Invalid workflow against zod schema:\n${error}`)
+    return null
   }
   return result.data
 }
diff --git a/tests-ui/tests/comfyWorkflow.test.ts b/tests-ui/tests/comfyWorkflow.test.ts
@@ -1,4 +1,4 @@
-import { parseComfyWorkflow } from '../../src/types/comfyWorkflow'
+import { validateComfyWorkflow } from '../../src/types/comfyWorkflow'
 import { defaultGraph } from '../../src/scripts/defaultGraph'
 import fs from 'fs'
 
@@ -9,101 +9,83 @@ describe('parseComfyWorkflow', () => {
     fs.readdirSync(WORKFLOW_DIR).forEach(async (file) => {
       if (file.endsWith('.json')) {
         const data = fs.readFileSync(`${WORKFLOW_DIR}/${file}`, 'utf-8')
-        await expect(parseComfyWorkflow(data)).resolves.not.toThrow()
+        expect(await validateComfyWorkflow(JSON.parse(data))).not.toBeNull()
       }
     })
   })
 
   it('workflow.nodes', async () => {
     const workflow = JSON.parse(JSON.stringify(defaultGraph))
     workflow.nodes = undefined
-    await expect(parseComfyWorkflow(JSON.stringify(workflow))).rejects.toThrow()
+    expect(await validateComfyWorkflow(workflow)).toBeNull()
 
     workflow.nodes = null
-    await expect(parseComfyWorkflow(JSON.stringify(workflow))).rejects.toThrow()
+    expect(await validateComfyWorkflow(workflow)).toBeNull()
 
     workflow.nodes = []
-    await expect(
-      parseComfyWorkflow(JSON.stringify(workflow))
-    ).resolves.not.toThrow()
+    expect(await validateComfyWorkflow(workflow)).not.toBeNull()
   })
 
   it('workflow.version', async () => {
     const workflow = JSON.parse(JSON.stringify(defaultGraph))
     workflow.version = undefined
-    await expect(parseComfyWorkflow(JSON.stringify(workflow))).rejects.toThrow()
+    expect(await validateComfyWorkflow(workflow)).toBeNull()
 
     workflow.version = '1.0.1' // Invalid format.
-    await expect(parseComfyWorkflow(JSON.stringify(workflow))).rejects.toThrow()
+    expect(await validateComfyWorkflow(workflow)).toBeNull()
 
     workflow.version = 1
-    await expect(
-      parseComfyWorkflow(JSON.stringify(workflow))
-    ).resolves.not.toThrow()
+    expect(await validateComfyWorkflow(workflow)).not.toBeNull()
   })
 
   it('workflow.extra', async () => {
     const workflow = JSON.parse(JSON.stringify(defaultGraph))
     workflow.extra = undefined
-    await expect(
-      parseComfyWorkflow(JSON.stringify(workflow))
-    ).resolves.not.toThrow()
+    expect(await validateComfyWorkflow(workflow)).not.toBeNull()
 
     workflow.extra = null
-    await expect(
-      parseComfyWorkflow(JSON.stringify(workflow))
-    ).resolves.not.toThrow()
+    expect(await validateComfyWorkflow(workflow)).not.toBeNull()
 
     workflow.extra = {}
-    await expect(
-      parseComfyWorkflow(JSON.stringify(workflow))
-    ).resolves.not.toThrow()
+    expect(await validateComfyWorkflow(workflow)).not.toBeNull()
 
     workflow.extra = { foo: 'bar' } // Should accept extra fields.
-    await expect(
-      parseComfyWorkflow(JSON.stringify(workflow))
-    ).resolves.not.toThrow()
+    expect(await validateComfyWorkflow(workflow)).not.toBeNull()
   })
 
   it('workflow.nodes.pos', async () => {
     const workflow = JSON.parse(JSON.stringify(defaultGraph))
     workflow.nodes[0].pos = [1, 2, 3]
-    await expect(parseComfyWorkflow(JSON.stringify(workflow))).rejects.toThrow()
+    expect(await validateComfyWorkflow(workflow)).toBeNull()
 
     workflow.nodes[0].pos = [1, 2]
-    await expect(
-      parseComfyWorkflow(JSON.stringify(workflow))
-    ).resolves.not.toThrow()
+    expect(await validateComfyWorkflow(workflow)).not.toBeNull()
 
     // Should automatically transform the legacy format object to array.
     workflow.nodes[0].pos = { '0': 3, '1': 4 }
-    let parsedWorkflow = await parseComfyWorkflow(JSON.stringify(workflow))
-    expect(parsedWorkflow.nodes[0].pos).toEqual([3, 4])
+    let validatedWorkflow = await validateComfyWorkflow(workflow)
+    expect(validatedWorkflow.nodes[0].pos).toEqual([3, 4])
 
     workflow.nodes[0].pos = { 0: 3, 1: 4 }
-    parsedWorkflow = await parseComfyWorkflow(JSON.stringify(workflow))
-    expect(parsedWorkflow.nodes[0].pos).toEqual([3, 4])
+    validatedWorkflow = await validateComfyWorkflow(workflow)
+    expect(validatedWorkflow.nodes[0].pos).toEqual([3, 4])
   })
 
   it('workflow.nodes.widget_values', async () => {
     const workflow = JSON.parse(JSON.stringify(defaultGraph))
     workflow.nodes[0].widgets_values = ['foo', 'bar']
-    await expect(
-      parseComfyWorkflow(JSON.stringify(workflow))
-    ).resolves.not.toThrow()
+    expect(await validateComfyWorkflow(workflow)).not.toBeNull()
 
     workflow.nodes[0].widgets_values = 'foo'
-    await expect(parseComfyWorkflow(JSON.stringify(workflow))).rejects.toThrow()
+    expect(await validateComfyWorkflow(workflow)).toBeNull()
 
     workflow.nodes[0].widgets_values = undefined
-    await expect(
-      parseComfyWorkflow(JSON.stringify(workflow))
-    ).resolves.not.toThrow()
+    expect(await validateComfyWorkflow(workflow)).not.toBeNull()
 
     // The object format of widgets_values is used by VHS nodes to perform
     // dynamic widgets display.
     workflow.nodes[0].widgets_values = { foo: 'bar' }
-    const parsedWorkflow = await parseComfyWorkflow(JSON.stringify(workflow))
-    expect(parsedWorkflow.nodes[0].widgets_values).toEqual({ foo: 'bar' })
+    const validatedWorkflow = await validateComfyWorkflow(workflow)
+    expect(validatedWorkflow.nodes[0].widgets_values).toEqual({ foo: 'bar' })
   })
 })
diff --git a/tests-ui/tests/exampleWorkflows.test.ts b/tests-ui/tests/exampleWorkflows.test.ts
@@ -38,8 +38,11 @@ describe('example workflows', () => {
     lg.teardown(global)
   })
 
-  for (const file of readdirSync(WORKFLOW_DIR)) {
-    if (!file.endsWith('.json')) continue
+  const workflowFiles = readdirSync(WORKFLOW_DIR).filter((file) =>
+    file.endsWith('.json')
+  )
+
+  const workflows = workflowFiles.map((file) => {
     const { workflow, prompt } = JSON.parse(
       readFileSync(path.resolve(WORKFLOW_DIR, file), 'utf8')
     )
@@ -53,17 +56,24 @@ describe('example workflows', () => {
       skip = !!Object.keys(parsedWorkflow?.extra?.groupNodes ?? {}).length
     } catch (error) {}
 
-    ;(skip ? test.skip : test)(
-      'correctly generates prompt json for ' + file,
-      async () => {
-        if (!workflow || !prompt) throw new Error('Invalid example json')
+    return { file, workflow, prompt, parsedWorkflow, skip }
+  })
 
-        const { app } = await start()
-        await app.loadGraphData(parsedWorkflow)
+  describe.each(workflows)(
+    'Workflow Test: %s',
+    ({ file, workflow, prompt, parsedWorkflow, skip }) => {
+      ;(skip ? test.skip : test)(
+        'correctly generates prompt json for ' + file,
+        async () => {
+          if (!workflow || !prompt) throw new Error('Invalid example json')
 
-        const output = await app.graphToPrompt()
-        expect(output.output).toEqual(fixLegacyPrompt(JSON.parse(prompt)))
-      }
-    )
-  }
+          const { app } = await start()
+          await app.loadGraphData(parsedWorkflow)
+
+          const output = await app.graphToPrompt()
+          expect(output.output).toEqual(fixLegacyPrompt(JSON.parse(prompt)))
+        }
+      )
+    }
+  )
 })
diff --git a/tests-ui/tests/groupNode.test.ts b/tests-ui/tests/groupNode.test.ts
@@ -756,7 +756,8 @@ describe('group node', () => {
       })
     )
   })
-  test('shows missing node error on missing internal node when loading graph data', async () => {
+  // Now reports zod validation error
+  test.skip('shows missing node error on missing internal node when loading graph data', async () => {
     const { graph } = await start()
 
     const dialogShow = jest.spyOn(graph.app.ui.dialog, 'show')

diff --git a/tests-ui/tests/widgetInputs.test.ts b/tests-ui/tests/widgetInputs.test.ts
@@ -181,7 +181,8 @@ describe('widget inputs', () => {
     expect(clone.inputs.ckpt_name).toBeFalsy()
   })
 
-  test('shows missing node error on custom node with converted input', async () => {
+  // Invalid workflow against zod schema now.
+  test.skip('shows missing node error on custom node with converted input', async () => {
     const { graph } = await start()
 
     const dialogShow = jest.spyOn(graph.app.ui.dialog, 'show')
@@ -219,6 +220,7 @@ describe('widget inputs', () => {
           flags: {},
           order: 0,
           mode: 0,
+          // Missing name and type
           outputs: [{ links: [4], widget: { name: 'test' } }],
           title: 'test',
           properties: {}