mirror of
https://github.com/Comfy-Org/ComfyUI_frontend.git
synced 2026-02-10 10:00:08 +00:00
Show error state on Vue nodes that caused execution errors (#5541)
* add missing node error border * update vue node data after configure * provide locatorId of execution error node to vue nodes * [refactor] use execution store directly instead of provide/inject pattern - Add lastExecutionErrorNodeId computed property to execution store - Replace inject() with useExecutionStore() in LGraphNode component - Remove useExecutionErrorProvider composable and provider call - Clean up unused ExecutionErrorNodeIdKey injection key - Add explicit return type annotation to hasAnyError computed Addresses @DrJKL's architecture feedback and type safety suggestions. * simplify error styling to match main branch conventions Remove redundant dark-theme prefixes from border-error and outline-error classes since these CSS custom properties handle both themes automatically. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com> * address review feedback on hasAnyError computed function - Add explicit boolean return type - Destructure props with defaults for cleaner code - Use \!\! for proper boolean conversion to satisfy TypeScript Addresses @DrJKL review comment on error state computation. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com> * destructure props at top level as suggested in review Replace `props.nodeData` and `props.error` references with destructured variables for cleaner code and proper defaults. Addresses @DrJKL review comment about props destructuring. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com> * fix rebase issues: correct node ID comparison and border colors - Use lastExecutionErrorNodeId instead of lastExecutionErrorNodeLocatorId for proper comparison with nodeData.id (both are local node IDs) - Restore border-blue-100 colors that were incorrectly changed during rebase - These were unrelated changes that snuck in during conflict resolution 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com> * remove unused lastExecutionErrorNodeLocatorId from exports The computed property is defined but not used by any external modules. Only lastExecutionErrorNodeId is actually consumed by components. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com>
This commit is contained in:
@@ -223,7 +223,6 @@ export const useGraphNodeManager = (graph: LGraph): GraphNodeManager => {
|
||||
selected: node.selected || false,
|
||||
executing: false, // Will be updated separately based on execution state
|
||||
subgraphId,
|
||||
|
||||
hasErrors: !!node.has_errors,
|
||||
widgets: safeWidgets,
|
||||
inputs: node.inputs ? [...node.inputs] : undefined,
|
||||
|
||||
@@ -13,7 +13,7 @@
|
||||
// border
|
||||
'border border-solid border-sand-100 dark-theme:border-charcoal-300',
|
||||
!!executing && 'border-blue-100 dark-theme:border-blue-100',
|
||||
!!(error || nodeData.hasErrors) && 'border-error',
|
||||
hasAnyError && 'border-error',
|
||||
// hover
|
||||
'hover:ring-7 ring-gray-500/50 dark-theme:ring-gray-500/20',
|
||||
// Selected
|
||||
@@ -21,7 +21,7 @@
|
||||
!!isSelected && 'outline-black dark-theme:outline-white',
|
||||
!!(isSelected && executing) &&
|
||||
'outline-blue-100 dark-theme:outline-blue-100',
|
||||
!!(isSelected && (error || nodeData.hasErrors)) && 'outline-error',
|
||||
isSelected && hasAnyError && 'outline-error',
|
||||
{
|
||||
'animate-pulse': executing,
|
||||
'opacity-50': nodeData.mode === 4,
|
||||
@@ -146,6 +146,7 @@ import { useNodeLayout } from '@/renderer/extensions/vueNodes/layout/useNodeLayo
|
||||
import { LODLevel, useLOD } from '@/renderer/extensions/vueNodes/lod/useLOD'
|
||||
import { ExecutedWsMessage } from '@/schemas/apiSchema'
|
||||
import { app } from '@/scripts/app'
|
||||
import { useExecutionStore } from '@/stores/executionStore'
|
||||
import { useNodeOutputStore } from '@/stores/imagePreviewStore'
|
||||
import { getNodeByLocatorId } from '@/utils/graphTraversalUtil'
|
||||
import { cn } from '@/utils/tailwindUtil'
|
||||
@@ -167,7 +168,12 @@ interface LGraphNodeProps {
|
||||
zoomLevel?: number
|
||||
}
|
||||
|
||||
const props = defineProps<LGraphNodeProps>()
|
||||
const {
|
||||
nodeData,
|
||||
error = null,
|
||||
readonly = false,
|
||||
zoomLevel = 1
|
||||
} = defineProps<LGraphNodeProps>()
|
||||
|
||||
const emit = defineEmits<{
|
||||
'node-click': [
|
||||
@@ -186,7 +192,7 @@ const emit = defineEmits<{
|
||||
'update:title': [nodeId: string, newTitle: string]
|
||||
}>()
|
||||
|
||||
useVueElementTracking(props.nodeData.id, 'node')
|
||||
useVueElementTracking(nodeData.id, 'node')
|
||||
|
||||
// Inject selection state from parent
|
||||
const selectedNodeIds = inject(SelectedNodeIdsKey)
|
||||
@@ -198,14 +204,25 @@ if (!selectedNodeIds) {
|
||||
|
||||
// Computed selection state - only this node re-evaluates when its selection changes
|
||||
const isSelected = computed(() => {
|
||||
return selectedNodeIds.value.has(props.nodeData.id)
|
||||
return selectedNodeIds.value.has(nodeData.id)
|
||||
})
|
||||
|
||||
// Use execution state composable
|
||||
const { executing, progress } = useNodeExecutionState(props.nodeData.id)
|
||||
const { executing, progress } = useNodeExecutionState(nodeData.id)
|
||||
|
||||
// Direct access to execution store for error state
|
||||
const executionStore = useExecutionStore()
|
||||
const hasExecutionError = computed(
|
||||
() => executionStore.lastExecutionErrorNodeId === nodeData.id
|
||||
)
|
||||
|
||||
// Computed error states for styling
|
||||
const hasAnyError = computed(
|
||||
(): boolean => !!(hasExecutionError.value || nodeData.hasErrors || error)
|
||||
)
|
||||
|
||||
// LOD (Level of Detail) system based on zoom level
|
||||
const zoomRef = toRef(() => props.zoomLevel ?? 1)
|
||||
const zoomRef = toRef(() => zoomLevel)
|
||||
const {
|
||||
lodLevel,
|
||||
shouldRenderWidgets,
|
||||
@@ -234,7 +251,7 @@ const {
|
||||
startDrag,
|
||||
handleDrag: handleLayoutDrag,
|
||||
endDrag
|
||||
} = useNodeLayout(props.nodeData.id)
|
||||
} = useNodeLayout(nodeData.id)
|
||||
|
||||
// Drag state for styling
|
||||
const isDragging = ref(false)
|
||||
@@ -247,11 +264,11 @@ const lastX = ref(0)
|
||||
const DRAG_THRESHOLD_PX = 4
|
||||
|
||||
// Track collapsed state
|
||||
const isCollapsed = ref(props.nodeData.flags?.collapsed ?? false)
|
||||
const isCollapsed = ref(nodeData.flags?.collapsed ?? false)
|
||||
|
||||
// Watch for external changes to the collapsed state
|
||||
watch(
|
||||
() => props.nodeData.flags?.collapsed,
|
||||
() => nodeData.flags?.collapsed,
|
||||
(newCollapsed: boolean | undefined) => {
|
||||
if (newCollapsed !== undefined && newCollapsed !== isCollapsed.value) {
|
||||
isCollapsed.value = newCollapsed
|
||||
@@ -272,7 +289,7 @@ const progressClasses = 'h-2 bg-primary-500 transition-all duration-300'
|
||||
|
||||
// Common condition computations to avoid repetition
|
||||
const shouldShowWidgets = computed(
|
||||
() => shouldRenderWidgets.value && props.nodeData.widgets?.length
|
||||
() => shouldRenderWidgets.value && nodeData.widgets?.length
|
||||
)
|
||||
|
||||
const shouldShowContent = computed(
|
||||
@@ -281,7 +298,7 @@ const shouldShowContent = computed(
|
||||
|
||||
// Event handlers
|
||||
const handlePointerDown = (event: PointerEvent) => {
|
||||
if (!props.nodeData) {
|
||||
if (!nodeData) {
|
||||
console.warn('LGraphNode: nodeData is null/undefined in handlePointerDown')
|
||||
return
|
||||
}
|
||||
@@ -308,13 +325,13 @@ const handlePointerUp = (event: PointerEvent) => {
|
||||
const dx = event.clientX - lastX.value
|
||||
const dy = event.clientY - lastY.value
|
||||
const wasDragging = Math.hypot(dx, dy) > DRAG_THRESHOLD_PX
|
||||
emit('node-click', event, props.nodeData, wasDragging)
|
||||
emit('node-click', event, nodeData, wasDragging)
|
||||
}
|
||||
|
||||
const handleCollapse = () => {
|
||||
isCollapsed.value = !isCollapsed.value
|
||||
// Emit event so parent can sync with LiteGraph if needed
|
||||
emit('update:collapsed', props.nodeData.id, isCollapsed.value)
|
||||
emit('update:collapsed', nodeData.id, isCollapsed.value)
|
||||
}
|
||||
|
||||
const handleSlotClick = (
|
||||
@@ -322,15 +339,15 @@ const handleSlotClick = (
|
||||
slotIndex: number,
|
||||
isInput: boolean
|
||||
) => {
|
||||
if (!props.nodeData) {
|
||||
if (!nodeData) {
|
||||
console.warn('LGraphNode: nodeData is null/undefined in handleSlotClick')
|
||||
return
|
||||
}
|
||||
emit('slot-click', event, props.nodeData, slotIndex, isInput)
|
||||
emit('slot-click', event, nodeData, slotIndex, isInput)
|
||||
}
|
||||
|
||||
const handleTitleUpdate = (newTitle: string) => {
|
||||
emit('update:title', props.nodeData.id, newTitle)
|
||||
emit('update:title', nodeData.id, newTitle)
|
||||
}
|
||||
|
||||
const nodeOutputs = useNodeOutputStore()
|
||||
@@ -338,9 +355,9 @@ const nodeOutputs = useNodeOutputStore()
|
||||
const nodeImageUrls = ref<string[]>([])
|
||||
const onNodeOutputsUpdate = (newOutputs: ExecutedWsMessage['output']) => {
|
||||
// Construct proper locator ID using subgraph ID from VueNodeData
|
||||
const locatorId = props.nodeData.subgraphId
|
||||
? `${props.nodeData.subgraphId}:${props.nodeData.id}`
|
||||
: props.nodeData.id
|
||||
const locatorId = nodeData.subgraphId
|
||||
? `${nodeData.subgraphId}:${nodeData.id}`
|
||||
: nodeData.id
|
||||
|
||||
// Use root graph for getNodeByLocatorId since it needs to traverse from root
|
||||
const rootGraph = app.graph?.rootGraph || app.graph
|
||||
@@ -363,9 +380,7 @@ const onNodeOutputsUpdate = (newOutputs: ExecutedWsMessage['output']) => {
|
||||
}
|
||||
|
||||
const nodeOutputLocatorId = computed(() =>
|
||||
props.nodeData.subgraphId
|
||||
? `${props.nodeData.subgraphId}:${props.nodeData.id}`
|
||||
: props.nodeData.id
|
||||
nodeData.subgraphId ? `${nodeData.subgraphId}:${nodeData.id}` : nodeData.id
|
||||
)
|
||||
|
||||
watch(
|
||||
|
||||
@@ -36,7 +36,7 @@
|
||||
</template>
|
||||
|
||||
<script setup lang="ts">
|
||||
import { computed, onErrorCaptured, ref } from 'vue'
|
||||
import { computed, onErrorCaptured, ref, watch } from 'vue'
|
||||
|
||||
import EditableText from '@/components/common/EditableText.vue'
|
||||
import type { VueNodeData } from '@/composables/graph/useGraphNodeManager'
|
||||
@@ -74,18 +74,26 @@ const isEditing = ref(false)
|
||||
|
||||
const nodeInfo = computed(() => props.nodeData || props.node)
|
||||
|
||||
const EMPTY_STRING = ''
|
||||
const DEFAULT_TITLE = 'Untitled'
|
||||
|
||||
const resolveTitle = (info: LGraphNode | VueNodeData | undefined) => {
|
||||
const title = (info?.title ?? EMPTY_STRING).trim()
|
||||
const title = (info?.title ?? '').trim()
|
||||
if (title.length > 0) return title
|
||||
const type = (info?.type ?? EMPTY_STRING).trim()
|
||||
return type.length > 0 ? type : DEFAULT_TITLE
|
||||
const type = (info?.type ?? '').trim()
|
||||
return type.length > 0 ? type : 'Untitled'
|
||||
}
|
||||
|
||||
// Computed title that provides reactive updates
|
||||
const displayTitle = computed(() => resolveTitle(nodeInfo.value))
|
||||
// Local state for title to provide immediate feedback
|
||||
const displayTitle = ref(resolveTitle(nodeInfo.value))
|
||||
|
||||
// Watch for external changes to the node title or type
|
||||
watch(
|
||||
() => [nodeInfo.value?.title, nodeInfo.value?.type] as const,
|
||||
() => {
|
||||
const next = resolveTitle(nodeInfo.value)
|
||||
if (next !== displayTitle.value) {
|
||||
displayTitle.value = next
|
||||
}
|
||||
}
|
||||
)
|
||||
|
||||
// Event handlers
|
||||
const handleCollapse = () => {
|
||||
|
||||
@@ -220,6 +220,19 @@ export const useExecutionStore = defineStore('execution', () => {
|
||||
return total > 0 ? done / total : 0
|
||||
})
|
||||
|
||||
const lastExecutionErrorNodeLocatorId = computed(() => {
|
||||
const err = lastExecutionError.value
|
||||
if (!err) return null
|
||||
return executionIdToNodeLocatorId(String(err.node_id))
|
||||
})
|
||||
|
||||
const lastExecutionErrorNodeId = computed(() => {
|
||||
const locator = lastExecutionErrorNodeLocatorId.value
|
||||
if (!locator) return null
|
||||
const localId = workflowStore.nodeLocatorIdToNodeId(locator)
|
||||
return localId != null ? String(localId) : null
|
||||
})
|
||||
|
||||
function bindExecutionEvents() {
|
||||
api.addEventListener('execution_start', handleExecutionStart)
|
||||
api.addEventListener('execution_cached', handleExecutionCached)
|
||||
@@ -426,6 +439,10 @@ export const useExecutionStore = defineStore('execution', () => {
|
||||
* The error from the previous execution.
|
||||
*/
|
||||
lastExecutionError,
|
||||
/**
|
||||
* Local node ID for the most recent execution error.
|
||||
*/
|
||||
lastExecutionErrorNodeId,
|
||||
/**
|
||||
* The id of the node that is currently being executed (backward compatibility)
|
||||
*/
|
||||
|
||||
Reference in New Issue
Block a user