mirror of
https://github.com/Comfy-Org/ComfyUI_frontend.git
synced 2026-03-05 13:10:24 +00:00
fix: Prune invalid builder mappings on load (#9376)
## Summary - extract resolveNode to reusable util - remove mid builder pruning - handle missing widgets with label ## Review Focus `resolveNode` was simplified for subgraphs by calling getNodeById on each of the subgraphs instead of searching their inner nodes manually. ## Screenshots (if applicable) "Widget not visible" <img width="657" height="822" alt="image" src="https://github.com/user-attachments/assets/ab7d1e87-3210-4e54-876a-07881974b5c7" /> <img width="674" height="375" alt="image" src="https://github.com/user-attachments/assets/c50ec871-d423-43d6-8e1e-7b1a362f621c" /> ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-9376-fix-Prune-invalid-builder-mappings-on-load-3196d73d3650811280c2d459ed0271af) by [Unito](https://www.unito.io)
This commit is contained in:
@@ -1,6 +1,6 @@
|
||||
<script setup lang="ts">
|
||||
import { remove } from 'es-toolkit'
|
||||
import { computed, provide, ref, toValue, watchEffect } from 'vue'
|
||||
import { computed, provide, ref, toValue } from 'vue'
|
||||
import type { MaybeRef } from 'vue'
|
||||
import { useI18n } from 'vue-i18n'
|
||||
|
||||
@@ -25,6 +25,7 @@ import { DOMWidgetImpl } from '@/scripts/domWidget'
|
||||
import { useDialogService } from '@/services/dialogService'
|
||||
import { useAppMode } from '@/composables/useAppMode'
|
||||
import { useAppModeStore } from '@/stores/appModeStore'
|
||||
import { resolveNode } from '@/utils/litegraphUtil'
|
||||
import { cn } from '@/utils/tailwindUtil'
|
||||
import { HideLayoutFieldKey } from '@/types/widgetTypes'
|
||||
|
||||
@@ -45,32 +46,12 @@ provide(HideLayoutFieldKey, true)
|
||||
|
||||
workflowStore.activeWorkflow?.changeTracker?.reset()
|
||||
|
||||
function resolveNode(nodeId: NodeId) {
|
||||
return (
|
||||
app.rootGraph.getNodeById(nodeId) ??
|
||||
[...app.rootGraph.subgraphs.values()]
|
||||
.flatMap((sg) => sg.nodes)
|
||||
.find((n) => n.id == nodeId)
|
||||
)
|
||||
}
|
||||
|
||||
// Prune stale entries whose node/widget no longer exists, so the
|
||||
// DraggableList model always matches the rendered items.
|
||||
watchEffect(() => {
|
||||
const valid = appModeStore.selectedInputs.filter(([nodeId, widgetName]) =>
|
||||
resolveNode(nodeId)?.widgets?.some((w) => w.name === widgetName)
|
||||
)
|
||||
if (valid.length < appModeStore.selectedInputs.length) {
|
||||
appModeStore.selectedInputs = valid
|
||||
}
|
||||
})
|
||||
|
||||
const arrangeInputs = computed(() =>
|
||||
appModeStore.selectedInputs
|
||||
.map(([nodeId, widgetName]) => {
|
||||
const node = resolveNode(nodeId)
|
||||
const widget = node?.widgets?.find((w) => w.name === widgetName)
|
||||
if (!node || !widget) return null
|
||||
if (!node) return null
|
||||
const widget = node.widgets?.find((w) => w.name === widgetName)
|
||||
return { nodeId, widgetName, node, widget }
|
||||
})
|
||||
.filter((item): item is NonNullable<typeof item> => item !== null)
|
||||
@@ -80,7 +61,13 @@ const inputsWithState = computed(() =>
|
||||
appModeStore.selectedInputs.map(([nodeId, widgetName]) => {
|
||||
const node = resolveNode(nodeId)
|
||||
const widget = node?.widgets?.find((w) => w.name === widgetName)
|
||||
if (!node || !widget) return { nodeId, widgetName }
|
||||
if (!node || !widget) {
|
||||
return {
|
||||
nodeId,
|
||||
widgetName,
|
||||
subLabel: t('linearMode.builder.unknownWidget')
|
||||
}
|
||||
}
|
||||
|
||||
const input = node.inputs.find((i) => i.widget?.name === widget.name)
|
||||
const rename = input && (() => renameWidget(widget, input))
|
||||
@@ -228,9 +215,9 @@ const renderedInputs = computed<[string, MaybeRef<BoundStyle> | undefined][]>(
|
||||
v-for="{ nodeId, widgetName, node, widget } in arrangeInputs"
|
||||
:key="`${nodeId}: ${widgetName}`"
|
||||
:class="cn(dragClass, 'p-2 my-2 pointer-events-auto')"
|
||||
:aria-label="`${widget.label ?? widgetName} — ${node.title}`"
|
||||
:aria-label="`${widget?.label ?? widgetName} — ${node.title}`"
|
||||
>
|
||||
<div class="pointer-events-none" inert>
|
||||
<div v-if="widget" class="pointer-events-none" inert>
|
||||
<WidgetItem
|
||||
:widget="widget"
|
||||
:node="node"
|
||||
@@ -238,6 +225,12 @@ const renderedInputs = computed<[string, MaybeRef<BoundStyle> | undefined][]>(
|
||||
hidden-widget-actions
|
||||
/>
|
||||
</div>
|
||||
<div v-else class="text-muted-foreground text-sm p-1 pointer-events-none">
|
||||
{{ widgetName }}
|
||||
<p class="text-xs italic">
|
||||
({{ t('linearMode.builder.unknownWidget') }})
|
||||
</p>
|
||||
</div>
|
||||
</div>
|
||||
</DraggableList>
|
||||
<PropertiesAccordionItem
|
||||
|
||||
@@ -3067,7 +3067,8 @@
|
||||
"promptAddOutputs": "Click on output nodes to add them here. These will be the generated results.",
|
||||
"noOutputs": "No output nodes added yet",
|
||||
"outputsDesc": "Connect at least one output node so users can see results after running.",
|
||||
"outputsExample": "Examples: “Save Image” or “Save Video”"
|
||||
"outputsExample": "Examples: “Save Image” or “Save Video”",
|
||||
"unknownWidget": "Widget not visible"
|
||||
},
|
||||
"queue": {
|
||||
"clickToClear": "Click to clear queue",
|
||||
|
||||
@@ -26,6 +26,7 @@ import { useQueueSettingsStore } from '@/stores/queueStore'
|
||||
import { cn } from '@/utils/tailwindUtil'
|
||||
import { useAppMode } from '@/composables/useAppMode'
|
||||
import { useAppModeStore } from '@/stores/appModeStore'
|
||||
import { resolveNode } from '@/utils/litegraphUtil'
|
||||
const { t } = useI18n()
|
||||
const commandStore = useCommandStore()
|
||||
const executionErrorStore = useExecutionErrorStore()
|
||||
@@ -68,11 +69,7 @@ const mappedSelections = computed(() => {
|
||||
([id]) => id === nodeId
|
||||
).map(([, widgetName]) => widgetName)
|
||||
unprocessedInputs = unprocessedInputs.slice(inputGroup.length)
|
||||
const node =
|
||||
app.rootGraph.getNodeById(nodeId) ??
|
||||
[...app.rootGraph.subgraphs.values()]
|
||||
.flatMap((sg) => sg.nodes)
|
||||
.find((n) => n.id == nodeId)
|
||||
const node = resolveNode(nodeId)
|
||||
if (!node) continue
|
||||
|
||||
const nodeData = nodeToNodeData(node)
|
||||
|
||||
@@ -7,6 +7,8 @@ import type { LoadedComfyWorkflow } from '@/platform/workflow/management/stores/
|
||||
import { ComfyWorkflow as ComfyWorkflowClass } from '@/platform/workflow/management/stores/comfyWorkflow'
|
||||
import { useWorkflowStore } from '@/platform/workflow/management/stores/workflowStore'
|
||||
import { app } from '@/scripts/app'
|
||||
import type { LGraphNode, NodeId } from '@/lib/litegraph/src/LGraphNode'
|
||||
import type { ChangeTracker } from '@/scripts/changeTracker'
|
||||
import { createMockChangeTracker } from '@/utils/__tests__/litegraphTestUtils'
|
||||
|
||||
vi.mock('@/scripts/app', () => ({
|
||||
@@ -15,6 +17,14 @@ vi.mock('@/scripts/app', () => ({
|
||||
}
|
||||
}))
|
||||
|
||||
const mockResolveNode = vi.hoisted(() =>
|
||||
vi.fn<(id: NodeId) => LGraphNode | undefined>(() => undefined)
|
||||
)
|
||||
vi.mock('@/utils/litegraphUtil', async (importOriginal) => ({
|
||||
...(await importOriginal()),
|
||||
resolveNode: mockResolveNode
|
||||
}))
|
||||
|
||||
vi.mock('@/renderer/core/canvas/canvasStore', () => ({
|
||||
useCanvasStore: () => ({
|
||||
getCanvas: () => ({ read_only: false })
|
||||
@@ -43,6 +53,7 @@ describe('appModeStore', () => {
|
||||
setActivePinia(createTestingPinia({ stubActions: false }))
|
||||
vi.clearAllMocks()
|
||||
vi.mocked(app.rootGraph).extra = {}
|
||||
mockResolveNode.mockReturnValue(undefined)
|
||||
})
|
||||
|
||||
describe('enterBuilder', () => {
|
||||
@@ -93,6 +104,105 @@ describe('appModeStore', () => {
|
||||
})
|
||||
})
|
||||
|
||||
describe('loadSelections pruning', () => {
|
||||
function mockNode(id: number) {
|
||||
return { id }
|
||||
}
|
||||
|
||||
function workflowWithLinearData(
|
||||
inputs: [number, string][],
|
||||
outputs: number[]
|
||||
) {
|
||||
const workflow = createBuilderWorkflow('app')
|
||||
workflow.changeTracker = createMockChangeTracker({
|
||||
activeState: {
|
||||
last_node_id: 0,
|
||||
last_link_id: 0,
|
||||
nodes: [],
|
||||
links: [],
|
||||
groups: [],
|
||||
config: {},
|
||||
version: 0.4,
|
||||
extra: { linearData: { inputs, outputs } }
|
||||
}
|
||||
} as unknown as Partial<ChangeTracker>)
|
||||
return workflow
|
||||
}
|
||||
|
||||
it('removes inputs referencing deleted nodes on load', async () => {
|
||||
const node1 = mockNode(1)
|
||||
mockResolveNode.mockImplementation((id) =>
|
||||
id == 1 ? (node1 as unknown as LGraphNode) : undefined
|
||||
)
|
||||
|
||||
const workflowStore = useWorkflowStore()
|
||||
const store = useAppModeStore()
|
||||
|
||||
workflowStore.activeWorkflow = workflowWithLinearData(
|
||||
[
|
||||
[1, 'prompt'],
|
||||
[99, 'width']
|
||||
],
|
||||
[]
|
||||
)
|
||||
await nextTick()
|
||||
|
||||
expect(store.selectedInputs).toEqual([[1, 'prompt']])
|
||||
})
|
||||
|
||||
it('keeps inputs for existing nodes even if widget is missing', async () => {
|
||||
const node1 = mockNode(1)
|
||||
mockResolveNode.mockImplementation((id) =>
|
||||
id == 1 ? (node1 as unknown as LGraphNode) : undefined
|
||||
)
|
||||
|
||||
const workflowStore = useWorkflowStore()
|
||||
const store = useAppModeStore()
|
||||
|
||||
workflowStore.activeWorkflow = workflowWithLinearData(
|
||||
[
|
||||
[1, 'prompt'],
|
||||
[1, 'deleted_widget']
|
||||
],
|
||||
[]
|
||||
)
|
||||
await nextTick()
|
||||
|
||||
expect(store.selectedInputs).toEqual([
|
||||
[1, 'prompt'],
|
||||
[1, 'deleted_widget']
|
||||
])
|
||||
})
|
||||
|
||||
it('removes outputs referencing deleted nodes on load', async () => {
|
||||
const node1 = mockNode(1)
|
||||
mockResolveNode.mockImplementation((id) =>
|
||||
id == 1 ? (node1 as unknown as LGraphNode) : undefined
|
||||
)
|
||||
|
||||
const workflowStore = useWorkflowStore()
|
||||
const store = useAppModeStore()
|
||||
|
||||
workflowStore.activeWorkflow = workflowWithLinearData([], [1, 99])
|
||||
await nextTick()
|
||||
|
||||
expect(store.selectedOutputs).toEqual([1])
|
||||
})
|
||||
|
||||
it('hasOutputs is false when all output nodes are deleted', async () => {
|
||||
mockResolveNode.mockReturnValue(undefined)
|
||||
|
||||
const workflowStore = useWorkflowStore()
|
||||
const store = useAppModeStore()
|
||||
|
||||
workflowStore.activeWorkflow = workflowWithLinearData([], [10, 20])
|
||||
await nextTick()
|
||||
|
||||
expect(store.selectedOutputs).toEqual([])
|
||||
expect(store.hasOutputs).toBe(false)
|
||||
})
|
||||
})
|
||||
|
||||
describe('linearData sync watcher', () => {
|
||||
it('writes linearData to rootGraph.extra when in builder mode', async () => {
|
||||
const workflowStore = useWorkflowStore()
|
||||
|
||||
@@ -7,6 +7,7 @@ import type { LinearData } from '@/platform/workflow/management/stores/comfyWork
|
||||
import { useCanvasStore } from '@/renderer/core/canvas/canvasStore'
|
||||
import { useWorkflowStore } from '@/platform/workflow/management/stores/workflowStore'
|
||||
import { app } from '@/scripts/app'
|
||||
import { resolveNode } from '@/utils/litegraphUtil'
|
||||
|
||||
export const useAppModeStore = defineStore('appMode', () => {
|
||||
const { getCanvas } = useCanvasStore()
|
||||
@@ -18,8 +19,21 @@ export const useAppModeStore = defineStore('appMode', () => {
|
||||
const hasOutputs = computed(() => !!selectedOutputs.length)
|
||||
|
||||
function loadSelections(data: Partial<LinearData> | undefined) {
|
||||
selectedInputs.splice(0, selectedInputs.length, ...(data?.inputs ?? []))
|
||||
selectedOutputs.splice(0, selectedOutputs.length, ...(data?.outputs ?? []))
|
||||
const rawInputs = data?.inputs ?? []
|
||||
const rawOutputs = data?.outputs ?? []
|
||||
|
||||
// Prune entries referencing nodes deleted in workflow mode.
|
||||
// Only check node existence, not widgets — dynamic widgets can
|
||||
// hide/show other widgets so a missing widget does not mean stale data.
|
||||
const inputs = app.rootGraph
|
||||
? rawInputs.filter(([nodeId]) => resolveNode(nodeId))
|
||||
: rawInputs
|
||||
const outputs = app.rootGraph
|
||||
? rawOutputs.filter((nodeId) => resolveNode(nodeId))
|
||||
: rawOutputs
|
||||
|
||||
selectedInputs.splice(0, selectedInputs.length, ...inputs)
|
||||
selectedOutputs.splice(0, selectedOutputs.length, ...outputs)
|
||||
}
|
||||
|
||||
function resetSelectedToWorkflow() {
|
||||
|
||||
@@ -15,15 +15,21 @@ import {
|
||||
createNode,
|
||||
isAnimatedOutput,
|
||||
isVideoOutput,
|
||||
migrateWidgetsValues
|
||||
migrateWidgetsValues,
|
||||
resolveNode
|
||||
} from '@/utils/litegraphUtil'
|
||||
|
||||
vi.mock('@/lib/litegraph/src/litegraph', () => ({
|
||||
vi.mock('@/lib/litegraph/src/litegraph', async (importOriginal) => ({
|
||||
...(await importOriginal()),
|
||||
LiteGraph: {
|
||||
createNode: vi.fn()
|
||||
}
|
||||
}))
|
||||
|
||||
vi.mock('@/scripts/app', () => ({
|
||||
app: { rootGraph: null }
|
||||
}))
|
||||
|
||||
vi.mock('@/platform/updates/common/toastStore', () => ({
|
||||
useToastStore: vi.fn(() => ({
|
||||
addAlert: vi.fn(),
|
||||
@@ -384,3 +390,53 @@ describe('compressWidgetInputSlots', () => {
|
||||
expect(graph.links).toEqual([])
|
||||
})
|
||||
})
|
||||
|
||||
describe('resolveNode', () => {
|
||||
function mockGraph(
|
||||
nodeList: Partial<LGraphNode>[],
|
||||
subgraphs?: Map<string, LGraph>
|
||||
) {
|
||||
const nodesById: Record<string, LGraphNode> = {}
|
||||
for (const n of nodeList) {
|
||||
nodesById[String(n.id)] = n as LGraphNode
|
||||
}
|
||||
return {
|
||||
nodes: nodeList as LGraphNode[],
|
||||
getNodeById(id: unknown) {
|
||||
return id != null ? (nodesById[String(id)] ?? null) : null
|
||||
},
|
||||
subgraphs: subgraphs ?? new Map()
|
||||
} as unknown as LGraph
|
||||
}
|
||||
|
||||
it('returns undefined when graph is nullish', () => {
|
||||
expect(resolveNode(1, null)).toBeUndefined()
|
||||
expect(resolveNode(1, undefined)).toBeUndefined()
|
||||
})
|
||||
|
||||
it('finds a node in the main graph', () => {
|
||||
const node = { id: 5 } as LGraphNode
|
||||
const graph = mockGraph([node])
|
||||
expect(resolveNode(5, graph)).toBe(node)
|
||||
})
|
||||
|
||||
it('finds a node in a subgraph', () => {
|
||||
const subNode = { id: 10 } as LGraphNode
|
||||
const subgraph = mockGraph([subNode])
|
||||
const graph = mockGraph([], new Map([['sg-1', subgraph]]))
|
||||
expect(resolveNode(10, graph)).toBe(subNode)
|
||||
})
|
||||
|
||||
it('returns undefined when node is not found anywhere', () => {
|
||||
const graph = mockGraph([{ id: 1 } as LGraphNode])
|
||||
expect(resolveNode(999, graph)).toBeUndefined()
|
||||
})
|
||||
|
||||
it('prefers main graph over subgraph', () => {
|
||||
const mainNode = { id: 1, title: 'main' } as LGraphNode
|
||||
const subNode = { id: 1, title: 'sub' } as LGraphNode
|
||||
const subgraph = mockGraph([subNode])
|
||||
const graph = mockGraph([mainNode], new Map([['sg-1', subgraph]]))
|
||||
expect(resolveNode(1, graph)).toBe(mainNode)
|
||||
})
|
||||
})
|
||||
|
||||
@@ -20,8 +20,10 @@ import type {
|
||||
IComboWidget,
|
||||
WidgetCallbackOptions
|
||||
} from '@/lib/litegraph/src/types/widgets'
|
||||
import type { NodeId } from '@/lib/litegraph/src/LGraphNode'
|
||||
import type { InputSpec } from '@/schemas/nodeDef/nodeDefSchemaV2'
|
||||
import { useToastStore } from '@/platform/updates/common/toastStore'
|
||||
import { app } from '@/scripts/app'
|
||||
import { t } from '@/i18n'
|
||||
|
||||
type ImageNode = LGraphNode & { imgs: HTMLImageElement[] | undefined }
|
||||
@@ -304,6 +306,20 @@ export function getLinkTypeColor(typeName: string): string {
|
||||
return LGraphCanvas.link_type_colors[typeName] ?? LiteGraph.LINK_COLOR
|
||||
}
|
||||
|
||||
export function resolveNode(
|
||||
nodeId: NodeId,
|
||||
graph: LGraph | null | undefined = app.rootGraph
|
||||
): LGraphNode | undefined {
|
||||
if (!graph) return undefined
|
||||
const found = graph.getNodeById(nodeId)
|
||||
if (found) return found
|
||||
for (const sg of graph.subgraphs.values()) {
|
||||
const node = sg.getNodeById(nodeId)
|
||||
if (node) return node
|
||||
}
|
||||
return undefined
|
||||
}
|
||||
|
||||
export function isLoad3dNode(node: LGraphNode) {
|
||||
return (
|
||||
node &&
|
||||
|
||||
Reference in New Issue
Block a user