mirror of
https://github.com/Comfy-Org/ComfyUI_frontend.git
synced 2026-04-19 22:09:37 +00:00
fix: address code review feedback on node replacement
- Add error toast in replaceNodesInPlace for user-visible failure feedback, returning empty array on error instead of throwing - Guard removeMissingNodesByType behind replacement success check (replaced.length > 0) to prevent stale error list updates - Sort buildMissingNodeGroups by priority for deterministic UI order (Swap Nodes 0 → Missing Node Packs 1 → Execution Errors) - Add aux_id fallback and cnr_id precedence tests for getCnrIdFromNode - Split replaceAllWarning from replaceWarning to fix i18n key mismatch between TabErrors tooltip and MissingNodesContent dialog
This commit is contained in:
@@ -142,7 +142,9 @@ function handleLocateNode(nodeType: MissingNodeType) {
|
||||
}
|
||||
|
||||
function handleReplaceNode() {
|
||||
replaceNodesInPlace(props.group.nodeTypes)
|
||||
executionErrorStore.removeMissingNodesByType([props.group.type])
|
||||
const replaced = replaceNodesInPlace(props.group.nodeTypes)
|
||||
if (replaced.length > 0) {
|
||||
executionErrorStore.removeMissingNodesByType([props.group.type])
|
||||
}
|
||||
}
|
||||
</script>
|
||||
|
||||
@@ -267,9 +267,11 @@ function handleOpenManagerInfo(packId: string) {
|
||||
|
||||
function handleReplaceAll(swapNodeGroups: SwapNodeGroup[]) {
|
||||
const allNodeTypes = swapNodeGroups.flatMap((g) => g.nodeTypes)
|
||||
replaceNodesInPlace(allNodeTypes)
|
||||
const typesToRemove = swapNodeGroups.map((g) => g.type)
|
||||
executionErrorStore.removeMissingNodesByType(typesToRemove)
|
||||
const replaced = replaceNodesInPlace(allNodeTypes)
|
||||
if (replaced.length > 0) {
|
||||
const typesToRemove = swapNodeGroups.map((g) => g.type)
|
||||
executionErrorStore.removeMissingNodesByType(typesToRemove)
|
||||
}
|
||||
}
|
||||
|
||||
function handleEnterSubgraph(nodeId: string) {
|
||||
|
||||
@@ -537,7 +537,7 @@ export function useErrorGroups(
|
||||
groups.push({
|
||||
type: 'swap_nodes' as const,
|
||||
title: 'Swap Nodes',
|
||||
priority: 1
|
||||
priority: 0
|
||||
})
|
||||
}
|
||||
|
||||
@@ -545,11 +545,11 @@ export function useErrorGroups(
|
||||
groups.push({
|
||||
type: 'missing_node' as const,
|
||||
title: error.message,
|
||||
priority: 0
|
||||
priority: 1
|
||||
})
|
||||
}
|
||||
|
||||
return groups
|
||||
return groups.sort((a, b) => a.priority - b.priority)
|
||||
}
|
||||
|
||||
const allErrorGroups = computed<ErrorGroup[]>(() => {
|
||||
|
||||
@@ -283,6 +283,15 @@ export function useNodeReplacement() {
|
||||
life: 3000
|
||||
})
|
||||
}
|
||||
} catch (error) {
|
||||
console.error('Failed to replace nodes:', error)
|
||||
toastStore.add({
|
||||
severity: 'error',
|
||||
summary: t('g.error', 'Error'),
|
||||
detail: t('nodeReplacement.replaceFailed', 'Failed to replace nodes'),
|
||||
life: 5000
|
||||
})
|
||||
return []
|
||||
} finally {
|
||||
changeTracker?.afterChange()
|
||||
}
|
||||
|
||||
@@ -49,6 +49,20 @@ describe('getCnrIdFromNode', () => {
|
||||
expect(getCnrIdFromNode(node)).toBe('node-pack')
|
||||
})
|
||||
|
||||
it('returns aux_id when cnr_id is absent', () => {
|
||||
const node = {
|
||||
properties: { aux_id: 'node-aux-pack' }
|
||||
} as unknown as LGraphNode
|
||||
expect(getCnrIdFromNode(node)).toBe('node-aux-pack')
|
||||
})
|
||||
|
||||
it('prefers cnr_id over aux_id in node properties', () => {
|
||||
const node = {
|
||||
properties: { cnr_id: 'primary', aux_id: 'secondary' }
|
||||
} as unknown as LGraphNode
|
||||
expect(getCnrIdFromNode(node)).toBe('primary')
|
||||
})
|
||||
|
||||
it('returns undefined when node has no cnr_id or aux_id', () => {
|
||||
const node = { properties: {} } as unknown as LGraphNode
|
||||
expect(getCnrIdFromNode(node)).toBeUndefined()
|
||||
|
||||
Reference in New Issue
Block a user