refactor: address trivial coderabbitai review comments

Amp-Thread-ID: https://ampcode.com/threads/T-019bb479-36cd-721b-8415-b0723dfeea83
Co-authored-by: Amp <amp@ampcode.com>
This commit is contained in:
DrJKL
2026-01-12 15:36:20 -08:00
parent 21310c9fad
commit 3a0c84145c
4 changed files with 68 additions and 86 deletions

View File

@@ -15,6 +15,29 @@ import {
createTestSubgraphNode
} from './__fixtures__/subgraphHelpers'
function createTestExportedSubgraph(
overrides: Partial<ExportedSubgraph>
): ExportedSubgraph {
return {
version: 1,
revision: 0,
state: { lastGroupId: 0, lastNodeId: 0, lastLinkId: 0, lastRerouteId: 0 },
id: 'test-id',
name: 'Test Subgraph',
nodes: [],
links: [],
groups: [],
config: {},
definitions: { subgraphs: [] },
inputs: [],
outputs: [],
inputNode: { id: -10, bounding: [0, 0, 120, 60] },
outputNode: { id: -20, bounding: [300, 0, 120, 60] },
widgets: [],
...overrides
}
}
describe.skip('SubgraphSerialization - Basic Serialization', () => {
it('should save and load simple subgraphs', () => {
const original = createTestSubgraph({
@@ -228,33 +251,15 @@ describe.skip('SubgraphSerialization - Version Compatibility', () => {
})
it('should load version 1.0+ format', () => {
const modernFormat = {
version: 1, // Number as expected by current implementation
const modernFormat = createTestExportedSubgraph({
id: 'test-modern-id',
name: 'Modern Subgraph',
nodes: [],
links: {},
groups: [],
config: {},
definitions: { subgraphs: [] },
inputs: [{ id: 'input-id', name: 'modern_input', type: 'number' }],
outputs: [{ id: 'output-id', name: 'modern_output', type: 'string' }],
inputNode: {
id: -10,
bounding: [0, 0, 120, 60]
},
outputNode: {
id: -20,
bounding: [300, 0, 120, 60]
},
widgets: []
}
outputs: [{ id: 'output-id', name: 'modern_output', type: 'string' }]
})
expect(() => {
const subgraph = new Subgraph(
new LGraph(),
modernFormat as unknown as ExportedSubgraph
)
const subgraph = new Subgraph(new LGraph(), modernFormat)
expect(subgraph.name).toBe('Modern Subgraph')
expect(subgraph.inputs.length).toBe(1)
expect(subgraph.outputs.length).toBe(1)
@@ -262,31 +267,16 @@ describe.skip('SubgraphSerialization - Version Compatibility', () => {
})
it('should handle missing fields gracefully', () => {
const incompleteFormat = {
version: 1,
const incompleteFormat = createTestExportedSubgraph({
id: 'incomplete-id',
name: 'Incomplete Subgraph',
nodes: [],
links: {},
groups: [],
config: {},
definitions: { subgraphs: [] },
inputNode: {
id: -10,
bounding: [0, 0, 120, 60]
},
outputNode: {
id: -20,
bounding: [300, 0, 120, 60]
}
// Missing optional: inputs, outputs, widgets
}
inputs: undefined,
outputs: undefined,
widgets: undefined
})
expect(() => {
const subgraph = new Subgraph(
new LGraph(),
incompleteFormat as unknown as ExportedSubgraph
)
const subgraph = new Subgraph(new LGraph(), incompleteFormat)
expect(subgraph.name).toBe('Incomplete Subgraph')
// Should have default empty arrays
expect(Array.isArray(subgraph.inputs)).toBe(true)
@@ -295,35 +285,20 @@ describe.skip('SubgraphSerialization - Version Compatibility', () => {
})
it('should consider future-proofing', () => {
const futureFormat = {
version: 2, // Future version (number)
const futureFormat = createTestExportedSubgraph({
id: 'future-id',
name: 'Future Subgraph',
nodes: [],
links: {},
groups: [],
config: {},
definitions: { subgraphs: [] },
inputs: [],
outputs: [],
inputNode: {
id: -10,
bounding: [0, 0, 120, 60]
},
outputNode: {
id: -20,
bounding: [300, 0, 120, 60]
},
widgets: [],
futureFeature: 'unknown_data' // Unknown future field
}
name: 'Future Subgraph'
})
// Test with unknown future fields - simulating a hypothetical future version
const extendedFormat = {
...futureFormat,
version: 2 as const, // Type assertion for test purposes
futureFeature: 'unknown_data'
} as unknown as ExportedSubgraph
// Should handle future format gracefully
expect(() => {
const subgraph = new Subgraph(
new LGraph(),
futureFormat as unknown as ExportedSubgraph
)
const subgraph = new Subgraph(new LGraph(), extendedFormat)
expect(subgraph.name).toBe('Future Subgraph')
}).not.toThrow()
})

View File

@@ -18,6 +18,7 @@ const ACCEPTED_VIDEO_TYPES = 'video/webm,video/mp4'
type InternalFile = string | ResultItem
type InternalValue = InternalFile | InternalFile[]
type ExposedValue = string | string[]
type WritableComboWidget = Omit<IComboWidget, 'value'> & { value: ExposedValue }
const isImageFile = (file: File) => file.type.startsWith('image/')
const isVideoFile = (file: File) => file.type.startsWith('video/')
@@ -80,11 +81,7 @@ export const useImageUploadWidget = () => {
// Create a NEW array to ensure Vue reactivity detects the change
// Value property is redefined via Object.defineProperty to support batch uploads
const newValue: ExposedValue = allow_batch ? [...output] : output[0]
;(
fileComboWidget as unknown as Omit<IComboWidget, 'value'> & {
value: ExposedValue
}
).value = newValue
;(fileComboWidget as unknown as WritableComboWidget).value = newValue
fileComboWidget.callback?.(newValue)
}
})

View File

@@ -190,7 +190,7 @@ function findBox(
start: number,
end: number,
type: string
): IsobmffBoxContentRange {
): IsobmffBoxContentRange | null {
let offset = start
while (offset < end) {
if (offset + 8 > end) break
@@ -210,7 +210,7 @@ function findBox(
return null
}
function parseAvifMetadata(buffer: ArrayBuffer): ComfyMetadata {
function parseAvifMetadata(buffer: ArrayBuffer): Partial<ComfyMetadata> {
const metadata: ComfyMetadata = {}
const dataView = new DataView(buffer)
@@ -318,7 +318,9 @@ function parseAvifMetadata(buffer: ArrayBuffer): ComfyMetadata {
return metadata
}
function parseExifData(exifData: Uint8Array) {
function parseExifData(
exifData: Uint8Array
): Record<number, string | undefined> {
// Check for the correct TIFF header (0x4949 for little-endian or 0x4D4D for big-endian)
const isLittleEndian = String.fromCharCode(...exifData.slice(0, 2)) === 'II'

View File

@@ -1,10 +1,14 @@
import { $el } from '../../ui'
import { ComfyDialog } from '../dialog'
export class ComfyAsyncDialog extends ComfyDialog<HTMLDialogElement> {
#resolve!: (value: any) => void
type DialogAction<T> = string | { value?: T; text: string }
constructor(actions?: Array<string | { value?: any; text: string }>) {
export class ComfyAsyncDialog<
T = string
> extends ComfyDialog<HTMLDialogElement> {
#resolve!: (value: T | string | null) => void
constructor(actions?: Array<DialogAction<T>>) {
super(
'dialog.comfy-dialog.comfyui-dialog',
actions?.map((opt) => {
@@ -18,7 +22,9 @@ export class ComfyAsyncDialog extends ComfyDialog<HTMLDialogElement> {
)
}
override show(html: string | HTMLElement | HTMLElement[]) {
override show(
html: string | HTMLElement | HTMLElement[]
): Promise<T | string | null> {
this.element.addEventListener('close', () => {
this.close()
})
@@ -30,7 +36,9 @@ export class ComfyAsyncDialog extends ComfyDialog<HTMLDialogElement> {
})
}
showModal(html: string | HTMLElement | HTMLElement[]) {
showModal(
html: string | HTMLElement | HTMLElement[]
): Promise<T | string | null> {
this.element.addEventListener('close', () => {
this.close()
})
@@ -43,22 +51,22 @@ export class ComfyAsyncDialog extends ComfyDialog<HTMLDialogElement> {
})
}
override close(result = null) {
override close(result: T | string | null = null): void {
this.#resolve(result)
this.element.close()
super.close()
}
static async prompt({
static async prompt<U = string>({
title = null,
message,
actions
}: {
title: string | null
message: string
actions: Array<string | { value?: any; text: string }>
}) {
const dialog = new ComfyAsyncDialog(actions)
actions: Array<DialogAction<U>>
}): Promise<U | string | null> {
const dialog = new ComfyAsyncDialog<U>(actions)
const content = [$el('span', message)]
if (title) {
content.unshift($el('h3', title))