Merge branch 'main' into test/cov-app

Addresses review feedback:
https://github.com/Comfy-Org/ComfyUI_frontend/pull/11400#discussion_r3114705436

src/scripts/app.ts

@@ -153,19 +153,18 @@ import {
 
 export const ANIM_PREVIEW_WIDGET = '$$comfy_animation_preview'
 
-export function sanitizeNodeName(string: string) {
-  let entityMap = {
-    '&': '',
-    '<': '',
-    '>': '',
-    '"': '',
-    "'": '',
-    '`': '',
-    '=': ''
-  }
-  return String(string).replace(/[&<>"'`=]/g, function fromEntityMap(s) {
-    return entityMap[s as keyof typeof entityMap]
-  })
+import {
+  isApiJson,
+  positionBatchLayout,
+  sanitizeNodeName,
+  stackNodesVertically
+} from './appUtil'
+
+export {
+  isApiJson,
+  positionBatchLayout,
+  sanitizeNodeName,
+  stackNodesVertically
 }
 
 type Clipspace = {
@@ -1904,61 +1903,20 @@ export class ComfyApp {
     this.canvas.selectItems(videoNodes)
   }
 
-  /**
-   * Positions batched nodes in drag and drop
-   * @param nodes
-   * @param batchNode
-   */
   positionNodes(nodes: LGraphNode[]): void {
-    if (nodes.length <= 1) return
-
-    const [x, y] = nodes[0].getBounding()
-    const nodeHeight = 150
-
-    nodes.forEach((node, index) => {
-      if (index > 0) {
-        node.pos = [x, y + nodeHeight * index + 25 * (index + 1)]
-      }
-    })
-
-    this.canvas.graph?.change()
+    if (stackNodesVertically(nodes)) {
+      this.canvas.graph?.change()
+    }
   }
 
   positionBatchNodes(nodes: LGraphNode[], batchNode: LGraphNode): void {
-    const [x, y, width] = nodes[0].getBounding()
-    batchNode.pos = [x + width + 100, y + 30]
-
-    // Retrieving Node Height is inconsistent
-    let height = 0
-    if (nodes[0].type === 'LoadImage') {
-      height = 344
-    }
-
-    nodes.forEach((node, index) => {
-      if (index > 0) {
-        node.pos = [x, y + height * index + 25 * (index + 1)]
-      }
-    })
-
+    positionBatchLayout(nodes, batchNode)
     this.canvas.graph?.change()
   }
 
-  // @deprecated
+  /** @deprecated Use {@link isApiJson} from @/scripts/appUtil instead */
   isApiJson(data: unknown): data is ComfyApiWorkflow {
-    if (!_.isObject(data) || Array.isArray(data)) {
-      return false
-    }
-    if (Object.keys(data).length === 0) return false
-
-    return Object.values(data).every((node) => {
-      if (!node || typeof node !== 'object' || Array.isArray(node)) {
-        return false
-      }
-
-      const { class_type: classType, inputs } = node as Record<string, unknown>
-      const inputsIsRecord = _.isObject(inputs) && !Array.isArray(inputs)
-      return typeof classType === 'string' && inputsIsRecord
-    })
+    return isApiJson(data)
   }
 
   loadApiJson(apiData: ComfyApiWorkflow, fileName: string) {

src/scripts/appUtil.test.ts

@@ -0,0 +1,130 @@
+import { describe, expect, it } from 'vitest'
+
+import {
+  isApiJson,
+  positionBatchLayout,
+  sanitizeNodeName,
+  stackNodesVertically
+} from './appUtil'
+
+function createNodeLike(
+  pos: [number, number],
+  bounding: number[],
+  type = 'LoadImage'
+) {
+  return {
+    pos,
+    type,
+    getBounding: () => new Float64Array(bounding)
+  }
+}
+
+describe('sanitizeNodeName', () => {
+  it('strips dangerous HTML entity characters', () => {
+    expect(sanitizeNodeName('a&b<c>d"e\'f`g=h')).toBe('abcdefgh')
+  })
+
+  it('returns the string unchanged when no entities are present', () => {
+    expect(sanitizeNodeName('KSampler')).toBe('KSampler')
+  })
+
+  it('handles empty string', () => {
+    expect(sanitizeNodeName('')).toBe('')
+  })
+})
+
+describe('isApiJson', () => {
+  it('accepts valid API workflow data', () => {
+    const data = {
+      '1': { class_type: 'KSampler', inputs: { seed: 42 } },
+      '2': { class_type: 'CLIPTextEncode', inputs: { text: 'hello' } }
+    }
+    expect(isApiJson(data)).toBe(true)
+  })
+
+  it('rejects empty object', () => {
+    expect(isApiJson({})).toBe(false)
+  })
+
+  it('rejects arrays', () => {
+    expect(isApiJson([1, 2, 3])).toBe(false)
+  })
+
+  it('rejects non-objects', () => {
+    expect(isApiJson('string')).toBe(false)
+    expect(isApiJson(42)).toBe(false)
+    expect(isApiJson(null)).toBe(false)
+  })
+
+  it('rejects when a node lacks class_type', () => {
+    expect(isApiJson({ '1': { inputs: { seed: 42 } } })).toBe(false)
+  })
+
+  it('rejects when inputs is an array instead of object', () => {
+    expect(isApiJson({ '1': { class_type: 'KSampler', inputs: [1, 2] } })).toBe(
+      false
+    )
+  })
+})
+
+describe('stackNodesVertically', () => {
+  it('returns false for a single node', () => {
+    const node = createNodeLike([100, 200], [100, 200, 300, 400])
+    expect(stackNodesVertically([node])).toBe(false)
+    expect(node.pos).toEqual([100, 200])
+  })
+
+  it('stacks multiple nodes below the first', () => {
+    const node1 = createNodeLike([100, 200], [100, 200, 300, 400])
+    const node2 = createNodeLike([0, 0], [0, 0, 200, 100])
+    const node3 = createNodeLike([0, 0], [0, 0, 200, 100])
+
+    expect(stackNodesVertically([node1, node2, node3])).toBe(true)
+
+    expect(node1.pos).toEqual([100, 200])
+    expect(node2.pos).toEqual([100, 400])
+    expect(node3.pos).toEqual([100, 575])
+  })
+
+  it('returns false for empty array', () => {
+    expect(stackNodesVertically([])).toBe(false)
+  })
+})
+
+describe('positionBatchLayout', () => {
+  it('places batch node to the right of the first node', () => {
+    const node1 = createNodeLike([100, 200], [100, 200, 300, 400])
+    const batchNode = createNodeLike([0, 0], [0, 0, 0, 0])
+
+    positionBatchLayout([node1], batchNode)
+
+    expect(batchNode.pos).toEqual([500, 230])
+  })
+
+  it('stacks LoadImage nodes at 344px height intervals', () => {
+    const node1 = createNodeLike([100, 200], [100, 200, 300, 400], 'LoadImage')
+    const node2 = createNodeLike([0, 0], [0, 0, 200, 100], 'LoadImage')
+    const batchNode = createNodeLike([0, 0], [0, 0, 0, 0])
+
+    positionBatchLayout([node1, node2], batchNode)
+
+    expect(node1.pos).toEqual([100, 200])
+    expect(node2.pos).toEqual([100, 544 + 50])
+  })
+
+  it('handles empty nodes array without throwing', () => {
+    const batchNode = createNodeLike([50, 50], [0, 0, 0, 0])
+    positionBatchLayout([], batchNode)
+    expect(batchNode.pos).toEqual([50, 50])
+  })
+
+  it('uses zero height for non-LoadImage nodes', () => {
+    const node1 = createNodeLike([100, 200], [100, 200, 300, 400], 'Other')
+    const node2 = createNodeLike([0, 0], [0, 0, 200, 100], 'Other')
+    const batchNode = createNodeLike([0, 0], [0, 0, 0, 0])
+
+    positionBatchLayout([node1, node2], batchNode)
+
+    expect(node2.pos).toEqual([100, 200 + 50])
+  })
+})

src/scripts/appUtil.ts

@@ -0,0 +1,97 @@
+import _ from 'es-toolkit/compat'
+
+import type { Rect } from '@/lib/litegraph/src/interfaces'
+import type { ComfyApiWorkflow } from '@/platform/workflow/validation/schemas/workflowSchema'
+
+/**
+ * Strips dangerous HTML entity characters from node names.
+ */
+export function sanitizeNodeName(string: string) {
+  let entityMap = {
+    '&': '',
+    '<': '',
+    '>': '',
+    '"': '',
+    "'": '',
+    '`': '',
+    '=': ''
+  }
+  return String(string).replace(/[&<>"'`=]/g, function fromEntityMap(s) {
+    return entityMap[s as keyof typeof entityMap]
+  })
+}
+
+/**
+ * Checks whether the given data conforms to the ComfyUI API workflow format.
+ * Each top-level value must have a string `class_type` and an object `inputs`.
+ *
+ * @deprecated
+ */
+export function isApiJson(data: unknown): data is ComfyApiWorkflow {
+  if (!_.isObject(data) || Array.isArray(data)) {
+    return false
+  }
+  if (Object.keys(data).length === 0) return false
+
+  return Object.values(data).every((node) => {
+    if (!node || typeof node !== 'object' || Array.isArray(node)) {
+      return false
+    }
+
+    const { class_type: classType, inputs } = node as Record<string, unknown>
+    const inputsIsRecord = _.isObject(inputs) && !Array.isArray(inputs)
+    return typeof classType === 'string' && inputsIsRecord
+  })
+}
+
+/**
+ * Vertically stacks nodes below the first node's position.
+ * Returns `true` if any positions were changed.
+ */
+export function stackNodesVertically(
+  nodes: { pos: [number, number]; getBounding: () => Rect }[]
+): boolean {
+  if (nodes.length <= 1) return false
+
+  const [x, y] = nodes[0].getBounding()
+  const nodeHeight = 150
+
+  nodes.forEach((node, index) => {
+    if (index > 0) {
+      node.pos = [x, y + nodeHeight * index + 25 * (index + 1)]
+    }
+  })
+
+  return true
+}
+
+/**
+ * Positions image nodes vertically and places a batch node to their right.
+ */
+export function positionBatchLayout(
+  nodes: {
+    pos: [number, number]
+    type: string
+    getBounding: () => Rect
+  }[],
+  batchNode: { pos: [number, number] }
+): void {
+  if (nodes.length === 0) {
+    return
+  }
+
+  const [x, y, width] = nodes[0].getBounding()
+  batchNode.pos = [x + width + 100, y + 30]
+
+  // Retrieving Node Height is inconsistent
+  let height = 0
+  if (nodes[0].type === 'LoadImage') {
+    height = 344
+  }
+
+  nodes.forEach((node, index) => {
+    if (index > 0) {
+      node.pos = [x, y + height * index + 25 * (index + 1)]
+    }
+  })
+}