mirror of
https://github.com/Comfy-Org/ComfyUI_frontend.git
synced 2026-04-19 22:09:37 +00:00
## Summary Refactors the error system to improve separation of concerns, fix DDD layer violations, and address code quality issues. - Extract `missingNodesErrorStore` from `executionErrorStore`, removing the delegation pattern that coupled missing-node logic into the execution error store - Extract `useNodeErrorFlagSync` composable for node error flag reconciliation (previously inlined) - Extract `useErrorClearingHooks` composable with explicit callback cleanup on node removal - Extract `useErrorActions` composable to deduplicate telemetry+command patterns across error card components - Move `getCnrIdFromNode`/`getCnrIdFromProperties` to `platform/nodeReplacement` layer (DDD fix) - Move `missingNodesErrorStore` to `platform/nodeReplacement` (DDD alignment) - Add unmount cancellation guard to `useErrorReport` async `onMounted` - Return watch stop handle from `useNodeErrorFlagSync` - Add `asyncResolvedIds` eviction on `missingNodesError` reset - Add `console.warn` to silent catch blocks and empty array guard - Hoist `useCommandStore` to setup scope, fix floating promises - Add `data-testid` to error groups, image/video error spans, copy button - Update E2E tests to use scoped locators and testids - Add unit tests for `onNodeRemoved` restoration and double-install guard Fixes #9875, Fixes #10027, Fixes #10033, Fixes #10085 ## Test plan - [x] Existing unit tests pass with updated imports and mocks - [x] New unit tests for `useErrorClearingHooks` (callback restoration, double-install guard) - [x] E2E tests updated to use scoped locators and `data-testid` - [ ] Manual: verify error tab shows runtime errors and missing nodes correctly - [ ] Manual: verify "Find on GitHub", "Copy", and "Get Help" buttons work in error cards ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-10302-refactor-error-system-cleanup-store-separation-DDD-fix-test-improvements-3286d73d365081838279d045b8dd957a) by [Unito](https://www.unito.io) --------- Co-authored-by: GitHub Action <action@github.com>
187 lines
4.9 KiB
TypeScript
187 lines
4.9 KiB
TypeScript
import { createPinia, setActivePinia } from 'pinia'
|
|
import { nextTick, ref } from 'vue'
|
|
import { beforeEach, describe, expect, it, vi } from 'vitest'
|
|
|
|
import type { MissingNodeType } from '@/types/comfy'
|
|
|
|
vi.mock('@/scripts/app', () => ({
|
|
app: {
|
|
rootGraph: {
|
|
serialize: vi.fn(() => ({})),
|
|
getNodeById: vi.fn()
|
|
}
|
|
}
|
|
}))
|
|
|
|
vi.mock('@/utils/graphTraversalUtil', () => ({
|
|
getNodeByExecutionId: vi.fn(),
|
|
getExecutionIdByNode: vi.fn(),
|
|
getRootParentNode: vi.fn(() => null),
|
|
forEachNode: vi.fn(),
|
|
mapAllNodes: vi.fn(() => [])
|
|
}))
|
|
|
|
vi.mock('@/platform/distribution/types', () => ({
|
|
isCloud: false
|
|
}))
|
|
|
|
vi.mock('@/i18n', () => ({
|
|
st: vi.fn((_key: string, fallback: string) => fallback)
|
|
}))
|
|
|
|
vi.mock('@/stores/comfyRegistryStore', () => ({
|
|
useComfyRegistryStore: () => ({
|
|
inferPackFromNodeName: vi.fn()
|
|
})
|
|
}))
|
|
|
|
vi.mock('@/utils/nodeTitleUtil', () => ({
|
|
resolveNodeDisplayName: vi.fn(() => '')
|
|
}))
|
|
|
|
vi.mock('@/utils/litegraphUtil', () => ({
|
|
isLGraphNode: vi.fn(() => false)
|
|
}))
|
|
|
|
vi.mock('@/utils/executableGroupNodeDto', () => ({
|
|
isGroupNode: vi.fn(() => false)
|
|
}))
|
|
|
|
import { useMissingNodesErrorStore } from '@/platform/nodeReplacement/missingNodesErrorStore'
|
|
import { useErrorGroups } from './useErrorGroups'
|
|
|
|
function makeMissingNodeType(
|
|
type: string,
|
|
opts: {
|
|
nodeId?: string
|
|
isReplaceable?: boolean
|
|
replacement?: { new_node_id: string }
|
|
} = {}
|
|
): MissingNodeType {
|
|
return {
|
|
type,
|
|
nodeId: opts.nodeId ?? '1',
|
|
isReplaceable: opts.isReplaceable ?? false,
|
|
replacement: opts.replacement
|
|
? {
|
|
old_node_id: type,
|
|
new_node_id: opts.replacement.new_node_id,
|
|
old_widget_ids: null,
|
|
input_mapping: null,
|
|
output_mapping: null
|
|
}
|
|
: undefined
|
|
}
|
|
}
|
|
|
|
describe('swapNodeGroups computed', () => {
|
|
beforeEach(() => {
|
|
setActivePinia(createPinia())
|
|
})
|
|
|
|
function getSwapNodeGroups(nodeTypes: MissingNodeType[]) {
|
|
useMissingNodesErrorStore().surfaceMissingNodes(nodeTypes)
|
|
|
|
const searchQuery = ref('')
|
|
const t = (key: string) => key
|
|
const { swapNodeGroups } = useErrorGroups(searchQuery, t)
|
|
return swapNodeGroups
|
|
}
|
|
|
|
it('returns empty array when no missing nodes', () => {
|
|
const swap = getSwapNodeGroups([])
|
|
expect(swap.value).toEqual([])
|
|
})
|
|
|
|
it('returns empty array when no nodes are replaceable', () => {
|
|
const swap = getSwapNodeGroups([
|
|
makeMissingNodeType('NodeA', { isReplaceable: false }),
|
|
makeMissingNodeType('NodeB', { isReplaceable: false })
|
|
])
|
|
expect(swap.value).toEqual([])
|
|
})
|
|
|
|
it('groups replaceable nodes by type', async () => {
|
|
const swap = getSwapNodeGroups([
|
|
makeMissingNodeType('OldNode', {
|
|
nodeId: '1',
|
|
isReplaceable: true,
|
|
replacement: { new_node_id: 'NewNode' }
|
|
}),
|
|
makeMissingNodeType('OldNode', {
|
|
nodeId: '2',
|
|
isReplaceable: true,
|
|
replacement: { new_node_id: 'NewNode' }
|
|
})
|
|
])
|
|
await nextTick()
|
|
expect(swap.value).toHaveLength(1)
|
|
expect(swap.value[0].type).toBe('OldNode')
|
|
expect(swap.value[0].newNodeId).toBe('NewNode')
|
|
expect(swap.value[0].nodeTypes).toHaveLength(2)
|
|
})
|
|
|
|
it('creates separate groups for different types', async () => {
|
|
const swap = getSwapNodeGroups([
|
|
makeMissingNodeType('TypeA', {
|
|
nodeId: '1',
|
|
isReplaceable: true,
|
|
replacement: { new_node_id: 'NewA' }
|
|
}),
|
|
makeMissingNodeType('TypeB', {
|
|
nodeId: '2',
|
|
isReplaceable: true,
|
|
replacement: { new_node_id: 'NewB' }
|
|
})
|
|
])
|
|
await nextTick()
|
|
expect(swap.value).toHaveLength(2)
|
|
expect(swap.value.map((g) => g.type)).toEqual(['TypeA', 'TypeB'])
|
|
})
|
|
|
|
it('sorts groups alphabetically by type', async () => {
|
|
const swap = getSwapNodeGroups([
|
|
makeMissingNodeType('Zebra', {
|
|
nodeId: '1',
|
|
isReplaceable: true,
|
|
replacement: { new_node_id: 'NewZ' }
|
|
}),
|
|
makeMissingNodeType('Alpha', {
|
|
nodeId: '2',
|
|
isReplaceable: true,
|
|
replacement: { new_node_id: 'NewA' }
|
|
})
|
|
])
|
|
await nextTick()
|
|
expect(swap.value[0].type).toBe('Alpha')
|
|
expect(swap.value[1].type).toBe('Zebra')
|
|
})
|
|
|
|
it('excludes string nodeType entries', async () => {
|
|
const swap = getSwapNodeGroups([
|
|
'StringGroupNode' as unknown as MissingNodeType,
|
|
makeMissingNodeType('OldNode', {
|
|
nodeId: '1',
|
|
isReplaceable: true,
|
|
replacement: { new_node_id: 'NewNode' }
|
|
})
|
|
])
|
|
await nextTick()
|
|
expect(swap.value).toHaveLength(1)
|
|
expect(swap.value[0].type).toBe('OldNode')
|
|
})
|
|
|
|
it('sets newNodeId to undefined when replacement is missing', async () => {
|
|
const swap = getSwapNodeGroups([
|
|
makeMissingNodeType('OldNode', {
|
|
nodeId: '1',
|
|
isReplaceable: true
|
|
// no replacement
|
|
})
|
|
])
|
|
await nextTick()
|
|
expect(swap.value).toHaveLength(1)
|
|
expect(swap.value[0].newNodeId).toBeUndefined()
|
|
})
|
|
})
|