mirror of
https://github.com/Comfy-Org/ComfyUI_frontend.git
synced 2026-04-20 14:30:41 +00:00
feat: add proactive validation for missing required connections
- Create useRequiredConnectionValidator composable that validates on graphChanged events - Detect missing required connections by checking slot.link and widget values - Use debounced validation (200ms) for performance on large graphs - Register event listener with proper cleanup on unmount Completes COM-12907: Missing workflow connection now highlights BEFORE clicking Run Amp-Thread-ID: https://ampcode.com/threads/T-019c0c91-73e0-7188-9770-c315279fadd8 Co-authored-by: Amp <amp@ampcode.com>
This commit is contained in:
122
src/composables/graph/useRequiredConnectionValidator.test.ts
Normal file
122
src/composables/graph/useRequiredConnectionValidator.test.ts
Normal file
@@ -0,0 +1,122 @@
|
||||
import { createPinia, setActivePinia } from 'pinia'
|
||||
import { beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
|
||||
import { useGraphErrorStateStore } from '@/stores/graphErrorStateStore'
|
||||
|
||||
vi.mock('@/scripts/api', () => ({
|
||||
api: {
|
||||
addEventListener: vi.fn(),
|
||||
removeEventListener: vi.fn()
|
||||
}
|
||||
}))
|
||||
|
||||
vi.mock('@/scripts/app', () => ({
|
||||
app: {
|
||||
rootGraph: null
|
||||
}
|
||||
}))
|
||||
|
||||
vi.mock('@/stores/nodeDefStore', () => ({
|
||||
useNodeDefStore: vi.fn(() => ({
|
||||
nodeDefsByName: {}
|
||||
}))
|
||||
}))
|
||||
|
||||
vi.mock('@/utils/graphTraversalUtil', () => ({
|
||||
forEachNode: vi.fn()
|
||||
}))
|
||||
|
||||
describe('useRequiredConnectionValidator', () => {
|
||||
beforeEach(() => {
|
||||
setActivePinia(createPinia())
|
||||
vi.clearAllMocks()
|
||||
})
|
||||
|
||||
describe('store integration', () => {
|
||||
it('adds errors for missing required connections', () => {
|
||||
const store = useGraphErrorStateStore()
|
||||
|
||||
store.execute({
|
||||
type: 'REPLACE_SOURCE',
|
||||
source: 'frontend',
|
||||
errors: [
|
||||
{
|
||||
key: 'frontend:missing:1:model',
|
||||
source: 'frontend',
|
||||
target: { kind: 'slot', nodeId: '1', slotName: 'model' },
|
||||
code: 'MISSING_REQUIRED_INPUT'
|
||||
}
|
||||
]
|
||||
})
|
||||
|
||||
expect(store.hasSlotError('1', 'model')).toBe(true)
|
||||
expect(store.hasErrorsForNode('1')).toBe(true)
|
||||
})
|
||||
|
||||
it('clears errors when connections are made', () => {
|
||||
const store = useGraphErrorStateStore()
|
||||
|
||||
store.execute({
|
||||
type: 'REPLACE_SOURCE',
|
||||
source: 'frontend',
|
||||
errors: [
|
||||
{
|
||||
key: 'frontend:missing:1:model',
|
||||
source: 'frontend',
|
||||
target: { kind: 'slot', nodeId: '1', slotName: 'model' }
|
||||
}
|
||||
]
|
||||
})
|
||||
|
||||
expect(store.hasSlotError('1', 'model')).toBe(true)
|
||||
|
||||
store.execute({
|
||||
type: 'REPLACE_SOURCE',
|
||||
source: 'frontend',
|
||||
errors: []
|
||||
})
|
||||
|
||||
expect(store.hasSlotError('1', 'model')).toBe(false)
|
||||
})
|
||||
|
||||
it('preserves backend errors when frontend errors change', () => {
|
||||
const store = useGraphErrorStateStore()
|
||||
|
||||
store.execute({
|
||||
type: 'REPLACE_SOURCE',
|
||||
source: 'backend',
|
||||
errors: [
|
||||
{
|
||||
key: 'backend:node:2',
|
||||
source: 'backend',
|
||||
target: { kind: 'node', nodeId: '2' }
|
||||
}
|
||||
]
|
||||
})
|
||||
|
||||
store.execute({
|
||||
type: 'REPLACE_SOURCE',
|
||||
source: 'frontend',
|
||||
errors: [
|
||||
{
|
||||
key: 'frontend:missing:1:model',
|
||||
source: 'frontend',
|
||||
target: { kind: 'slot', nodeId: '1', slotName: 'model' }
|
||||
}
|
||||
]
|
||||
})
|
||||
|
||||
expect(store.hasErrorsForNode('1')).toBe(true)
|
||||
expect(store.hasErrorsForNode('2')).toBe(true)
|
||||
|
||||
store.execute({
|
||||
type: 'REPLACE_SOURCE',
|
||||
source: 'frontend',
|
||||
errors: []
|
||||
})
|
||||
|
||||
expect(store.hasErrorsForNode('1')).toBe(false)
|
||||
expect(store.hasErrorsForNode('2')).toBe(true)
|
||||
})
|
||||
})
|
||||
})
|
||||
75
src/composables/graph/useRequiredConnectionValidator.ts
Normal file
75
src/composables/graph/useRequiredConnectionValidator.ts
Normal file
@@ -0,0 +1,75 @@
|
||||
import { useDebounceFn } from '@vueuse/core'
|
||||
import { onUnmounted } from 'vue'
|
||||
|
||||
import type { LGraphNode } from '@/lib/litegraph/src/litegraph'
|
||||
import { api } from '@/scripts/api'
|
||||
import { app } from '@/scripts/app'
|
||||
import { useGraphErrorStateStore } from '@/stores/graphErrorStateStore'
|
||||
import type { GraphError } from '@/stores/graphErrorStateStore'
|
||||
import { useNodeDefStore } from '@/stores/nodeDefStore'
|
||||
import { createNodeLocatorId } from '@/types/nodeIdentification'
|
||||
import { forEachNode } from '@/utils/graphTraversalUtil'
|
||||
|
||||
export function useRequiredConnectionValidator(): void {
|
||||
const errorStore = useGraphErrorStateStore()
|
||||
const nodeDefStore = useNodeDefStore()
|
||||
|
||||
function validate(): void {
|
||||
const rootGraph = app.rootGraph
|
||||
if (!rootGraph) return
|
||||
|
||||
const errors: GraphError[] = []
|
||||
|
||||
forEachNode(rootGraph, (node: LGraphNode) => {
|
||||
const nodeDef = nodeDefStore.nodeDefsByName[node.type ?? '']
|
||||
if (!nodeDef?.input?.required) return
|
||||
|
||||
const subgraphId =
|
||||
node.graph && !node.graph.isRootGraph ? node.graph.id : null
|
||||
const locatorId = subgraphId
|
||||
? createNodeLocatorId(subgraphId, node.id)
|
||||
: String(node.id)
|
||||
|
||||
for (const inputName of Object.keys(nodeDef.input.required)) {
|
||||
const slot = node.inputs?.find((s) => s.name === inputName)
|
||||
if (!slot) continue
|
||||
|
||||
const hasConnection = slot.link !== null && slot.link !== undefined
|
||||
const hasWidgetValue = hasWidgetValueForInput(node, inputName)
|
||||
|
||||
if (!hasConnection && !hasWidgetValue) {
|
||||
errors.push({
|
||||
key: `frontend:missing:${locatorId}:${inputName}`,
|
||||
source: 'frontend',
|
||||
target: { kind: 'slot', nodeId: locatorId, slotName: inputName },
|
||||
code: 'MISSING_REQUIRED_INPUT'
|
||||
})
|
||||
}
|
||||
}
|
||||
})
|
||||
|
||||
errorStore.execute({ type: 'REPLACE_SOURCE', source: 'frontend', errors })
|
||||
}
|
||||
|
||||
function hasWidgetValueForInput(
|
||||
node: LGraphNode,
|
||||
inputName: string
|
||||
): boolean {
|
||||
if (!node.widgets) return false
|
||||
const widget = node.widgets.find((w) => w.name === inputName)
|
||||
if (!widget) return false
|
||||
return (
|
||||
widget.value !== undefined && widget.value !== null && widget.value !== ''
|
||||
)
|
||||
}
|
||||
|
||||
const debouncedValidate = useDebounceFn(validate, 200)
|
||||
|
||||
api.addEventListener('graphChanged', debouncedValidate)
|
||||
|
||||
onUnmounted(() => {
|
||||
api.removeEventListener('graphChanged', debouncedValidate)
|
||||
})
|
||||
|
||||
validate()
|
||||
}
|
||||
@@ -46,6 +46,7 @@ import GraphCanvas from '@/components/graph/GraphCanvas.vue'
|
||||
import GlobalToast from '@/components/toast/GlobalToast.vue'
|
||||
import RerouteMigrationToast from '@/components/toast/RerouteMigrationToast.vue'
|
||||
import { useGraphErrorState } from '@/composables/graph/useGraphErrorState'
|
||||
import { useRequiredConnectionValidator } from '@/composables/graph/useRequiredConnectionValidator'
|
||||
import { useBrowserTabTitle } from '@/composables/useBrowserTabTitle'
|
||||
import { useCoreCommands } from '@/composables/useCoreCommands'
|
||||
import { useErrorHandling } from '@/composables/useErrorHandling'
|
||||
@@ -87,6 +88,7 @@ setupAutoQueueHandler()
|
||||
useProgressFavicon()
|
||||
useBrowserTabTitle()
|
||||
useGraphErrorState()
|
||||
useRequiredConnectionValidator()
|
||||
|
||||
const { t } = useI18n()
|
||||
const toast = useToast()
|
||||
|
||||
Reference in New Issue
Block a user