mirror of
https://github.com/Comfy-Org/ComfyUI_frontend.git
synced 2026-04-20 06:20:11 +00:00
refactor: centralize NodeExecutionOutput → ResultItemImpl parsing
Extract shared parseNodeOutput/parseTaskOutput utility to eliminate three independent copies of the same conversion with inconsistent validation: - flattenNodeOutput.ts (strict, required filename+subfolder) - jobOutputCache.ts (weak, any single field sufficient) - queueStore.ts (no validation, cast as ResultItem[]) All three now delegate to a single isResultItem guard that requires filename and subfolder as strings and validates type via the Zod resultItemType enum. Also excludes both 'animated' and 'text' metadata keys consistently. Addresses review feedback from DrJKL on PR #9622.
This commit is contained in:
@@ -1,38 +1,10 @@
|
||||
import type { NodeExecutionOutput, ResultItem } from '@/schemas/apiSchema'
|
||||
import { resultItemType } from '@/schemas/apiSchema'
|
||||
import { ResultItemImpl } from '@/stores/queueStore'
|
||||
|
||||
const EXCLUDED_KEYS = new Set(['animated'])
|
||||
|
||||
function isResultItemLike(item: unknown): item is ResultItem {
|
||||
if (!item || typeof item !== 'object' || Array.isArray(item)) {
|
||||
return false
|
||||
}
|
||||
|
||||
const candidate = item as Record<string, unknown>
|
||||
|
||||
if (typeof candidate.filename !== 'string') return false
|
||||
if (typeof candidate.subfolder !== 'string') return false
|
||||
|
||||
if (
|
||||
candidate.type !== undefined &&
|
||||
!resultItemType.safeParse(candidate.type).success
|
||||
) {
|
||||
return false
|
||||
}
|
||||
|
||||
return true
|
||||
}
|
||||
import type { NodeExecutionOutput } from '@/schemas/apiSchema'
|
||||
import { parseNodeOutput } from '@/stores/resultItemParsing'
|
||||
import type { ResultItemImpl } from '@/stores/queueStore'
|
||||
|
||||
export function flattenNodeOutput([nodeId, nodeOutput]: [
|
||||
string | number,
|
||||
NodeExecutionOutput
|
||||
]): ResultItemImpl[] {
|
||||
return Object.entries(nodeOutput)
|
||||
.filter(([key, value]) => !EXCLUDED_KEYS.has(key) && Array.isArray(value))
|
||||
.flatMap(([mediaType, items]) =>
|
||||
(items as unknown[])
|
||||
.filter(isResultItemLike)
|
||||
.map((item) => new ResultItemImpl({ ...item, mediaType, nodeId }))
|
||||
)
|
||||
return parseNodeOutput(nodeId, nodeOutput)
|
||||
}
|
||||
|
||||
@@ -11,11 +11,11 @@ import QuickLRU from '@alloc/quick-lru'
|
||||
import type { JobDetail } from '@/platform/remote/comfyui/jobs/jobTypes'
|
||||
import { extractWorkflow } from '@/platform/remote/comfyui/jobs/fetchJobs'
|
||||
import type { ComfyWorkflowJSON } from '@/platform/workflow/validation/schemas/workflowSchema'
|
||||
import { resultItemType } from '@/schemas/apiSchema'
|
||||
import type { ResultItem, TaskOutput } from '@/schemas/apiSchema'
|
||||
import type { TaskOutput } from '@/schemas/apiSchema'
|
||||
import { api } from '@/scripts/api'
|
||||
import { ResultItemImpl } from '@/stores/queueStore'
|
||||
import type { TaskItemImpl } from '@/stores/queueStore'
|
||||
import { parseTaskOutput } from '@/stores/resultItemParsing'
|
||||
|
||||
const MAX_TASK_CACHE_SIZE = 50
|
||||
const MAX_JOB_DETAIL_CACHE_SIZE = 50
|
||||
@@ -79,65 +79,7 @@ export async function getOutputsForTask(
|
||||
|
||||
function getPreviewableOutputs(outputs?: TaskOutput): ResultItemImpl[] {
|
||||
if (!outputs) return []
|
||||
const resultItems = Object.entries(outputs).flatMap(([nodeId, nodeOutputs]) =>
|
||||
Object.entries(nodeOutputs)
|
||||
.filter(([mediaType, _]) => mediaType !== 'animated')
|
||||
.flatMap(([mediaType, items]) => {
|
||||
if (!Array.isArray(items)) {
|
||||
return []
|
||||
}
|
||||
|
||||
return items.filter(isResultItemLike).map(
|
||||
(item) =>
|
||||
new ResultItemImpl({
|
||||
...item,
|
||||
nodeId,
|
||||
mediaType
|
||||
})
|
||||
)
|
||||
})
|
||||
)
|
||||
|
||||
return ResultItemImpl.filterPreviewable(resultItems)
|
||||
}
|
||||
|
||||
function isResultItemLike(item: unknown): item is ResultItem {
|
||||
if (!item || typeof item !== 'object' || Array.isArray(item)) {
|
||||
return false
|
||||
}
|
||||
|
||||
const candidate = item as Record<string, unknown>
|
||||
|
||||
if (
|
||||
candidate.filename !== undefined &&
|
||||
typeof candidate.filename !== 'string'
|
||||
) {
|
||||
return false
|
||||
}
|
||||
|
||||
if (
|
||||
candidate.subfolder !== undefined &&
|
||||
typeof candidate.subfolder !== 'string'
|
||||
) {
|
||||
return false
|
||||
}
|
||||
|
||||
if (
|
||||
candidate.type !== undefined &&
|
||||
!resultItemType.safeParse(candidate.type).success
|
||||
) {
|
||||
return false
|
||||
}
|
||||
|
||||
if (
|
||||
candidate.filename === undefined &&
|
||||
candidate.subfolder === undefined &&
|
||||
candidate.type === undefined
|
||||
) {
|
||||
return false
|
||||
}
|
||||
|
||||
return true
|
||||
return ResultItemImpl.filterPreviewable(parseTaskOutput(outputs))
|
||||
}
|
||||
|
||||
export function getPreviewableOutputsFromJobDetail(
|
||||
|
||||
@@ -16,6 +16,7 @@ import type {
|
||||
} from '@/schemas/apiSchema'
|
||||
import { appendCloudResParam } from '@/platform/distribution/cloudPreviewUtil'
|
||||
import { api } from '@/scripts/api'
|
||||
import { parseTaskOutput } from '@/stores/resultItemParsing'
|
||||
import type { ComfyApp } from '@/scripts/app'
|
||||
import { useExtensionService } from '@/services/extensionService'
|
||||
import { getJobDetail } from '@/services/jobOutputCache'
|
||||
@@ -267,18 +268,7 @@ export class TaskItemImpl {
|
||||
if (!this.outputs) {
|
||||
return []
|
||||
}
|
||||
return Object.entries(this.outputs).flatMap(([nodeId, nodeOutputs]) =>
|
||||
Object.entries(nodeOutputs).flatMap(([mediaType, items]) =>
|
||||
(items as ResultItem[]).map(
|
||||
(item: ResultItem) =>
|
||||
new ResultItemImpl({
|
||||
...item,
|
||||
nodeId,
|
||||
mediaType
|
||||
})
|
||||
)
|
||||
)
|
||||
)
|
||||
return parseTaskOutput(this.outputs)
|
||||
}
|
||||
|
||||
/** All outputs that support preview (images, videos, audio, 3D) */
|
||||
|
||||
157
src/stores/resultItemParsing.test.ts
Normal file
157
src/stores/resultItemParsing.test.ts
Normal file
@@ -0,0 +1,157 @@
|
||||
import { describe, expect, it } from 'vitest'
|
||||
|
||||
import type { NodeExecutionOutput } from '@/schemas/apiSchema'
|
||||
import { parseNodeOutput, parseTaskOutput } from '@/stores/resultItemParsing'
|
||||
|
||||
function makeOutput(
|
||||
overrides: Partial<NodeExecutionOutput> = {}
|
||||
): NodeExecutionOutput {
|
||||
return { ...overrides }
|
||||
}
|
||||
|
||||
describe(parseNodeOutput, () => {
|
||||
it('returns empty array for output with no known media types', () => {
|
||||
const result = parseNodeOutput('1', makeOutput({ text: 'hello' }))
|
||||
expect(result).toEqual([])
|
||||
})
|
||||
|
||||
it('flattens images into ResultItemImpl instances', () => {
|
||||
const output = makeOutput({
|
||||
images: [
|
||||
{ filename: 'a.png', subfolder: '', type: 'output' },
|
||||
{ filename: 'b.png', subfolder: 'sub', type: 'output' }
|
||||
]
|
||||
})
|
||||
|
||||
const result = parseNodeOutput('42', output)
|
||||
|
||||
expect(result).toHaveLength(2)
|
||||
expect(result[0].filename).toBe('a.png')
|
||||
expect(result[0].nodeId).toBe('42')
|
||||
expect(result[0].mediaType).toBe('images')
|
||||
expect(result[1].filename).toBe('b.png')
|
||||
expect(result[1].subfolder).toBe('sub')
|
||||
})
|
||||
|
||||
it('flattens audio outputs', () => {
|
||||
const output = makeOutput({
|
||||
audio: [{ filename: 'sound.wav', subfolder: '', type: 'output' }]
|
||||
})
|
||||
|
||||
const result = parseNodeOutput(7, output)
|
||||
|
||||
expect(result).toHaveLength(1)
|
||||
expect(result[0].mediaType).toBe('audio')
|
||||
expect(result[0].nodeId).toBe(7)
|
||||
})
|
||||
|
||||
it('flattens multiple media types in a single output', () => {
|
||||
const output = makeOutput({
|
||||
images: [{ filename: 'img.png', subfolder: '', type: 'output' }],
|
||||
video: [{ filename: 'vid.mp4', subfolder: '', type: 'output' }]
|
||||
})
|
||||
|
||||
const result = parseNodeOutput('1', output)
|
||||
|
||||
expect(result).toHaveLength(2)
|
||||
const types = result.map((r) => r.mediaType)
|
||||
expect(types).toContain('images')
|
||||
expect(types).toContain('video')
|
||||
})
|
||||
|
||||
it('handles gifs and 3d output types', () => {
|
||||
const output = makeOutput({
|
||||
gifs: [
|
||||
{ filename: 'anim.gif', subfolder: '', type: 'output' }
|
||||
] as NodeExecutionOutput['gifs'],
|
||||
'3d': [
|
||||
{ filename: 'model.glb', subfolder: '', type: 'output' }
|
||||
] as NodeExecutionOutput['3d']
|
||||
})
|
||||
|
||||
const result = parseNodeOutput('5', output)
|
||||
|
||||
expect(result).toHaveLength(2)
|
||||
const types = result.map((r) => r.mediaType)
|
||||
expect(types).toContain('gifs')
|
||||
expect(types).toContain('3d')
|
||||
})
|
||||
|
||||
it('ignores empty arrays', () => {
|
||||
const output = makeOutput({ images: [], audio: [] })
|
||||
const result = parseNodeOutput('1', output)
|
||||
expect(result).toEqual([])
|
||||
})
|
||||
|
||||
it('excludes animated key', () => {
|
||||
const output = makeOutput({
|
||||
images: [{ filename: 'img.png', subfolder: '', type: 'output' }],
|
||||
animated: [true]
|
||||
})
|
||||
|
||||
const result = parseNodeOutput('1', output)
|
||||
|
||||
expect(result).toHaveLength(1)
|
||||
expect(result[0].mediaType).toBe('images')
|
||||
})
|
||||
|
||||
it('excludes text key', () => {
|
||||
const output = makeOutput({
|
||||
images: [{ filename: 'img.png', subfolder: '', type: 'output' }],
|
||||
text: 'some text output'
|
||||
})
|
||||
|
||||
const result = parseNodeOutput('1', output)
|
||||
|
||||
expect(result).toHaveLength(1)
|
||||
expect(result[0].mediaType).toBe('images')
|
||||
})
|
||||
|
||||
it('excludes non-ResultItem array items', () => {
|
||||
const output = {
|
||||
images: [{ filename: 'img.png', subfolder: '', type: 'output' }],
|
||||
custom_data: [{ randomKey: 123 }]
|
||||
} as unknown as NodeExecutionOutput
|
||||
|
||||
const result = parseNodeOutput('1', output)
|
||||
|
||||
expect(result).toHaveLength(1)
|
||||
expect(result[0].mediaType).toBe('images')
|
||||
})
|
||||
|
||||
it('excludes partial ResultItem objects missing required fields', () => {
|
||||
const output = {
|
||||
images: [
|
||||
{ filename: 'valid.png', subfolder: '', type: 'output' },
|
||||
{ filename: 'no-subfolder.png' },
|
||||
{ subfolder: '', type: 'output' }
|
||||
]
|
||||
} as unknown as NodeExecutionOutput
|
||||
|
||||
const result = parseNodeOutput('1', output)
|
||||
|
||||
expect(result).toHaveLength(1)
|
||||
expect(result[0].filename).toBe('valid.png')
|
||||
})
|
||||
})
|
||||
|
||||
describe(parseTaskOutput, () => {
|
||||
it('flattens across multiple nodes', () => {
|
||||
const taskOutput: Record<string, NodeExecutionOutput> = {
|
||||
'1': makeOutput({
|
||||
images: [{ filename: 'a.png', subfolder: '', type: 'output' }]
|
||||
}),
|
||||
'2': makeOutput({
|
||||
audio: [{ filename: 'b.wav', subfolder: '', type: 'output' }]
|
||||
})
|
||||
}
|
||||
|
||||
const result = parseTaskOutput(taskOutput)
|
||||
|
||||
expect(result).toHaveLength(2)
|
||||
expect(result[0].nodeId).toBe('1')
|
||||
expect(result[0].filename).toBe('a.png')
|
||||
expect(result[1].nodeId).toBe('2')
|
||||
expect(result[1].filename).toBe('b.wav')
|
||||
})
|
||||
})
|
||||
52
src/stores/resultItemParsing.ts
Normal file
52
src/stores/resultItemParsing.ts
Normal file
@@ -0,0 +1,52 @@
|
||||
import type { NodeExecutionOutput, ResultItem } from '@/schemas/apiSchema'
|
||||
import { resultItemType } from '@/schemas/apiSchema'
|
||||
import { ResultItemImpl } from '@/stores/queueStore'
|
||||
|
||||
const METADATA_KEYS = new Set(['animated', 'text'])
|
||||
|
||||
/**
|
||||
* Validates that an unknown value is a well-formed ResultItem with required
|
||||
* fields for constructing a previewable output.
|
||||
*
|
||||
* Stricter than `zResultItem.safeParse()` because the Zod schema marks
|
||||
* `filename` and `subfolder` as optional (matching the wire format), but
|
||||
* a ResultItemImpl needs both to construct a valid URL.
|
||||
*/
|
||||
function isResultItem(item: unknown): item is ResultItem {
|
||||
if (!item || typeof item !== 'object' || Array.isArray(item)) return false
|
||||
|
||||
const candidate = item as Record<string, unknown>
|
||||
|
||||
if (typeof candidate.filename !== 'string') return false
|
||||
if (typeof candidate.subfolder !== 'string') return false
|
||||
|
||||
if (
|
||||
candidate.type !== undefined &&
|
||||
!resultItemType.safeParse(candidate.type).success
|
||||
) {
|
||||
return false
|
||||
}
|
||||
|
||||
return true
|
||||
}
|
||||
|
||||
export function parseNodeOutput(
|
||||
nodeId: string | number,
|
||||
nodeOutput: NodeExecutionOutput
|
||||
): ResultItemImpl[] {
|
||||
return Object.entries(nodeOutput)
|
||||
.filter(([key, value]) => !METADATA_KEYS.has(key) && Array.isArray(value))
|
||||
.flatMap(([mediaType, items]) =>
|
||||
(items as unknown[])
|
||||
.filter(isResultItem)
|
||||
.map((item) => new ResultItemImpl({ ...item, mediaType, nodeId }))
|
||||
)
|
||||
}
|
||||
|
||||
export function parseTaskOutput(
|
||||
taskOutput: Record<string, NodeExecutionOutput>
|
||||
): ResultItemImpl[] {
|
||||
return Object.entries(taskOutput).flatMap(([nodeId, nodeOutput]) =>
|
||||
parseNodeOutput(nodeId, nodeOutput)
|
||||
)
|
||||
}
|
||||
Reference in New Issue
Block a user