mirror of
https://github.com/Comfy-Org/ComfyUI_frontend.git
synced 2026-03-09 23:20:04 +00:00
[refactor] Extract manager composables and execution utils (#9163)
## Summary Extracts inline logic from manager components into dedicated composables and utilities, and adds a cyclic subgraph fix. ## Changes - **`usePackInstall`**: New composable extracted from `PackInstallButton.vue` — handles conflict detection, payload construction, and `Promise.allSettled`-based batch installation - **`useApplyChanges`**: New shared composable extracted from `ManagerProgressToast.vue` — manages ComfyUI restart flow with reconnect timeout and post-reconnect refresh - **`executionIdUtil`**: New utility (`getAncestorExecutionIds`, `getParentExecutionIds`, `buildSubgraphExecutionPaths`) with unit tests; fixes infinite recursion on cyclic subgraph definitions ## Review Focus - `useApplyChanges` reconnect timeout (2 min) and setting restore logic - `buildSubgraphExecutionPaths` visited-set guard for cyclic subgraph defs ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-9163-refactor-Extract-manager-composables-and-execution-utils-3116d73d365081f293d3d5484775ad48) by [Unito](https://www.unito.io)
This commit is contained in:
98
src/utils/executionIdUtil.test.ts
Normal file
98
src/utils/executionIdUtil.test.ts
Normal file
@@ -0,0 +1,98 @@
|
||||
import { describe, expect, it } from 'vitest'
|
||||
|
||||
import type { ComfyNode } from '@/platform/workflow/validation/schemas/workflowSchema'
|
||||
import {
|
||||
buildSubgraphExecutionPaths,
|
||||
getAncestorExecutionIds,
|
||||
getParentExecutionIds
|
||||
} from '@/utils/executionIdUtil'
|
||||
|
||||
function node(id: number, type: string): ComfyNode {
|
||||
return { id, type } as ComfyNode
|
||||
}
|
||||
|
||||
function subgraphDef(id: string, nodes: ComfyNode[]) {
|
||||
return { id, name: id, nodes, inputNode: {}, outputNode: {} }
|
||||
}
|
||||
|
||||
describe('getAncestorExecutionIds', () => {
|
||||
it('returns only itself for a root node', () => {
|
||||
expect(getAncestorExecutionIds('65')).toEqual(['65'])
|
||||
})
|
||||
|
||||
it('returns all ancestors including self for nested IDs', () => {
|
||||
expect(getAncestorExecutionIds('65:70')).toEqual(['65', '65:70'])
|
||||
expect(getAncestorExecutionIds('65:70:63')).toEqual([
|
||||
'65',
|
||||
'65:70',
|
||||
'65:70:63'
|
||||
])
|
||||
})
|
||||
})
|
||||
|
||||
describe('getParentExecutionIds', () => {
|
||||
it('returns empty for a root node', () => {
|
||||
expect(getParentExecutionIds('65')).toEqual([])
|
||||
})
|
||||
|
||||
it('returns all ancestors excluding self for nested IDs', () => {
|
||||
expect(getParentExecutionIds('65:70')).toEqual(['65'])
|
||||
expect(getParentExecutionIds('65:70:63')).toEqual(['65', '65:70'])
|
||||
})
|
||||
})
|
||||
|
||||
describe('buildSubgraphExecutionPaths', () => {
|
||||
it('returns empty map when there are no subgraph definitions', () => {
|
||||
expect(buildSubgraphExecutionPaths([node(5, 'SomeNode')], [])).toEqual(
|
||||
new Map()
|
||||
)
|
||||
})
|
||||
|
||||
it('returns empty map when no root node matches a subgraph type', () => {
|
||||
const def = subgraphDef('def-A', [])
|
||||
expect(
|
||||
buildSubgraphExecutionPaths([node(5, 'UnrelatedNode')], [def])
|
||||
).toEqual(new Map())
|
||||
})
|
||||
|
||||
it('maps a single subgraph instance to its execution path', () => {
|
||||
const def = subgraphDef('def-A', [])
|
||||
const result = buildSubgraphExecutionPaths([node(5, 'def-A')], [def])
|
||||
expect(result.get('def-A')).toEqual(['5'])
|
||||
})
|
||||
|
||||
it('collects multiple instances of the same subgraph type', () => {
|
||||
const def = subgraphDef('def-A', [])
|
||||
const result = buildSubgraphExecutionPaths(
|
||||
[node(5, 'def-A'), node(10, 'def-A')],
|
||||
[def]
|
||||
)
|
||||
expect(result.get('def-A')).toEqual(['5', '10'])
|
||||
})
|
||||
|
||||
it('builds nested execution paths for subgraphs within subgraphs', () => {
|
||||
const innerDef = subgraphDef('def-B', [])
|
||||
const outerDef = subgraphDef('def-A', [node(70, 'def-B')])
|
||||
const result = buildSubgraphExecutionPaths(
|
||||
[node(5, 'def-A')],
|
||||
[outerDef, innerDef]
|
||||
)
|
||||
expect(result.get('def-A')).toEqual(['5'])
|
||||
expect(result.get('def-B')).toEqual(['5:70'])
|
||||
})
|
||||
|
||||
it('does not recurse infinitely on self-referential subgraph definitions', () => {
|
||||
const cyclicDef = subgraphDef('def-A', [node(70, 'def-A')])
|
||||
expect(() =>
|
||||
buildSubgraphExecutionPaths([node(5, 'def-A')], [cyclicDef])
|
||||
).not.toThrow()
|
||||
})
|
||||
|
||||
it('does not recurse infinitely on mutually cyclic subgraph definitions', () => {
|
||||
const defA = subgraphDef('def-A', [node(70, 'def-B')])
|
||||
const defB = subgraphDef('def-B', [node(80, 'def-A')])
|
||||
expect(() =>
|
||||
buildSubgraphExecutionPaths([node(5, 'def-A')], [defA, defB])
|
||||
).not.toThrow()
|
||||
})
|
||||
})
|
||||
71
src/utils/executionIdUtil.ts
Normal file
71
src/utils/executionIdUtil.ts
Normal file
@@ -0,0 +1,71 @@
|
||||
import type { NodeExecutionId } from '@/types/nodeIdentification'
|
||||
import type { ComfyNode } from '@/platform/workflow/validation/schemas/workflowSchema'
|
||||
import { isSubgraphDefinition } from '@/platform/workflow/validation/schemas/workflowSchema'
|
||||
|
||||
/**
|
||||
* Returns all ancestor execution IDs for a given execution ID, including itself.
|
||||
*
|
||||
* Example: "65:70:63" → ["65", "65:70", "65:70:63"]
|
||||
* @knipIgnoreUsedByStackedPR
|
||||
*/
|
||||
export function getAncestorExecutionIds(
|
||||
executionId: string | NodeExecutionId
|
||||
): NodeExecutionId[] {
|
||||
const parts = executionId.split(':')
|
||||
return Array.from({ length: parts.length }, (_, i) =>
|
||||
parts.slice(0, i + 1).join(':')
|
||||
)
|
||||
}
|
||||
|
||||
/**
|
||||
* Returns all ancestor execution IDs for a given execution ID, excluding itself.
|
||||
*
|
||||
* Example: "65:70:63" → ["65", "65:70"]
|
||||
* @knipIgnoreUsedByStackedPR
|
||||
*/
|
||||
export function getParentExecutionIds(
|
||||
executionId: string | NodeExecutionId
|
||||
): NodeExecutionId[] {
|
||||
return getAncestorExecutionIds(executionId).slice(0, -1)
|
||||
}
|
||||
|
||||
/**
|
||||
* "def-A" → ["5", "10"] for each container node instantiating that subgraph definition.
|
||||
* @knipIgnoreUsedByStackedPR
|
||||
*/
|
||||
export function buildSubgraphExecutionPaths(
|
||||
rootNodes: ComfyNode[],
|
||||
allSubgraphDefs: unknown[]
|
||||
): Map<string, string[]> {
|
||||
const subgraphDefMap = new Map(
|
||||
allSubgraphDefs.filter(isSubgraphDefinition).map((s) => [s.id, s])
|
||||
)
|
||||
const pathMap = new Map<string, string[]>()
|
||||
const visited = new Set<string>()
|
||||
|
||||
const build = (nodes: ComfyNode[], parentPrefix: string) => {
|
||||
for (const n of nodes ?? []) {
|
||||
if (typeof n.type !== 'string' || !subgraphDefMap.has(n.type)) continue
|
||||
const path = parentPrefix ? `${parentPrefix}:${n.id}` : String(n.id)
|
||||
const existing = pathMap.get(n.type)
|
||||
if (existing) {
|
||||
existing.push(path)
|
||||
} else {
|
||||
pathMap.set(n.type, [path])
|
||||
}
|
||||
|
||||
if (visited.has(n.type)) continue
|
||||
visited.add(n.type)
|
||||
|
||||
const innerDef = subgraphDefMap.get(n.type)
|
||||
if (innerDef) {
|
||||
build(innerDef.nodes, path)
|
||||
}
|
||||
|
||||
visited.delete(n.type)
|
||||
}
|
||||
}
|
||||
|
||||
build(rootNodes, '')
|
||||
return pathMap
|
||||
}
|
||||
Reference in New Issue
Block a user