Skip to content
Draft
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
refactor: improve Vue widget type safety and runtime prop handling
- Add proper type guards for all widget input specs (Color, TreeSelect, MultiSelect, FileUpload, Galleria)
- Enhance schemas with missing properties (format, placeholder, accept, extensions, tooltip)
- Fix widgets to honor runtime props like disabled while accessing spec metadata
- Eliminate all 'as any' usage in widget components with proper TypeScript types
- Clean separation: widget.spec.options for metadata, widget.options for runtime state
- Refactor devtools into modular structure with vue_widgets showcase nodes
  • Loading branch information
christian-byrne committed Nov 6, 2025
commit 4699859de028f94b4a0dd186e676cc130d638e8e
41 changes: 41 additions & 0 deletions browser_tests/utils/devtoolsSync.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
import fs from 'fs-extra'
import path from 'path'
import { fileURLToPath } from 'url'

export function syncDevtools(targetComfyDir: string) {
if (!targetComfyDir) {
console.warn('syncDevtools skipped: TEST_COMFYUI_DIR not set')
return
}

const moduleDir =
typeof __dirname !== 'undefined'
? __dirname
: path.dirname(fileURLToPath(import.meta.url))

const devtoolsSrc = path.resolve(moduleDir, '..', '..', 'tools', 'devtools')

if (!fs.pathExistsSync(devtoolsSrc)) {
console.warn(
`syncDevtools skipped: source directory not found at ${devtoolsSrc}`
)
return
}

const devtoolsDest = path.resolve(
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[security] high Priority

Issue: Directory traversal vulnerability - fs.copySync without path validation
Context: The devtoolsDest path is constructed from user input without validation, potentially allowing directory traversal attacks
Suggestion: Validate and sanitize the targetComfyDir path before using it in file operations. Use path.resolve() and check that the resolved path stays within expected boundaries

targetComfyDir,
'custom_nodes',
'ComfyUI_devtools'
)

console.warn(`syncDevtools: copying ${devtoolsSrc} -> ${devtoolsDest}`)

try {
fs.removeSync(devtoolsDest)
fs.ensureDirSync(devtoolsDest)
fs.copySync(devtoolsSrc, devtoolsDest, { overwrite: true })
console.warn('syncDevtools: copy complete')
} catch (error) {
console.error(`Failed to sync DevTools to ${devtoolsDest}:`, error)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[quality] medium Priority

Issue: Error logging uses console.error but function continues execution after failure
Context: The function catches errors during file copying but doesn't throw or return error status to caller
Suggestion: Either throw the error to propagate failure or return a boolean indicating success/failure for better error handling

}
}
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import ColorPicker from 'primevue/colorpicker'
import { computed, ref, watch } from 'vue'

import { isColorInputSpec } from '@/schemas/nodeDef/nodeDefSchemaV2'
import type { SimplifiedWidget } from '@/types/simplifiedWidget'
import { isColorFormat, toHexFromFormat } from '@/utils/colorUtil'
import type { ColorFormat, HSB } from '@/utils/colorUtil'
Expand All @@ -51,18 +52,18 @@ const emit = defineEmits<{
}>()

const format = computed<ColorFormat>(() => {
const optionFormat = props.widget.options?.format
const spec = props.widget.spec
if (!spec || !isColorInputSpec(spec)) {
return 'hex'
}

const optionFormat = spec.options?.format
return isColorFormat(optionFormat) ? optionFormat : 'hex'
})

type PickerValue = string | HSB
const localValue = ref<PickerValue>(
toHexFromFormat(
props.modelValue || '#000000',
isColorFormat(props.widget.options?.format)
? props.widget.options.format
: 'hex'
)
toHexFromFormat(props.modelValue || '#000000', format.value)
)

watch(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,11 @@
ref="fileInputRef"
type="file"
class="hidden"
:accept="widget.options?.accept"
:accept="
widget.spec && isFileUploadInputSpec(widget.spec)
? widget.spec.options?.accept
: undefined
"
:aria-label="`${$t('g.upload')} ${widget.name || $t('g.file')}`"
:multiple="false"
@change="handleFileChange"
Expand All @@ -188,6 +192,7 @@ import { computed, onUnmounted, ref, watch } from 'vue'

import { useWidgetValue } from '@/composables/graph/useWidgetValue'
import { useTransformCompatOverlayProps } from '@/composables/useTransformCompatOverlayProps'
import { isFileUploadInputSpec } from '@/schemas/nodeDef/nodeDefSchemaV2'
import type { SimplifiedWidget } from '@/types/simplifiedWidget'

const { widget, modelValue } = defineProps<{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ import Galleria from 'primevue/galleria'
import { computed, ref } from 'vue'
import { useI18n } from 'vue-i18n'

import { isGalleriaInputSpec } from '@/schemas/nodeDef/nodeDefSchemaV2'
import type { SimplifiedWidget } from '@/types/simplifiedWidget'
import {
GALLERIA_EXCLUDED_PROPS,
Expand All @@ -78,9 +79,9 @@ const activeIndex = ref(0)

const { t } = useI18n()

const filteredProps = computed(() =>
filterWidgetProps(props.widget.options, GALLERIA_EXCLUDED_PROPS)
)
const filteredProps = computed(() => {
return filterWidgetProps(props.widget.options, GALLERIA_EXCLUDED_PROPS)
})

const galleryImages = computed(() => {
if (!value.value || !Array.isArray(value.value)) return []
Expand All @@ -100,16 +101,22 @@ const galleryImages = computed(() => {
})

const showThumbnails = computed(() => {
const spec = props.widget.spec
if (!spec || !isGalleriaInputSpec(spec)) {
return galleryImages.value.length > 1
}
return (
props.widget.options?.showThumbnails !== false &&
galleryImages.value.length > 1
spec.options?.showThumbnails !== false && galleryImages.value.length > 1
)
})

const showNavButtons = computed(() => {
const spec = props.widget.spec
if (!spec || !isGalleriaInputSpec(spec)) {
return galleryImages.value.length > 1
}
return (
props.widget.options?.showItemNavigators !== false &&
galleryImages.value.length > 1
spec.options?.showItemNavigators !== false && galleryImages.value.length > 1
)
})
</script>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import { computed } from 'vue'

import { useWidgetValue } from '@/composables/graph/useWidgetValue'
import { useTransformCompatOverlayProps } from '@/composables/useTransformCompatOverlayProps'
import { isMultiSelectInputSpec } from '@/schemas/nodeDef/nodeDefSchemaV2'
import type { SimplifiedWidget, WidgetValue } from '@/types/simplifiedWidget'
import {
PANEL_EXCLUDED_PROPS,
Expand Down Expand Up @@ -57,19 +58,20 @@ const MULTISELECT_EXCLUDED_PROPS = [
'overlayStyle'
] as const

// Extract spec options directly
const combinedProps = computed(() => ({
...filterWidgetProps(props.widget.options, MULTISELECT_EXCLUDED_PROPS),
...transformCompatProps.value
}))

// Extract multiselect options from widget options
// Extract multiselect options from widget spec options
const multiSelectOptions = computed((): T[] => {
const options = props.widget.options

if (Array.isArray(options?.values)) {
return options.values
const spec = props.widget.spec
if (!spec || !isMultiSelectInputSpec(spec)) {
return []
}

return []
const values = spec.options?.values
return Array.isArray(values) ? (values as T[]) : []
})
</script>
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,18 @@
<WidgetLayoutField :widget="widget">
<FormSelectButton
v-model="localValue"
:options="widget.options?.values || []"
:options="selectOptions"
class="w-full"
@update:model-value="onChange"
/>
</WidgetLayoutField>
</template>

<script setup lang="ts">
import { computed } from 'vue'

import { useStringWidgetValue } from '@/composables/graph/useWidgetValue'
import { isSelectButtonInputSpec } from '@/schemas/nodeDef/nodeDefSchemaV2'
import type { SimplifiedWidget } from '@/types/simplifiedWidget'

import FormSelectButton from './form/FormSelectButton.vue'
Expand All @@ -31,4 +34,13 @@ const { localValue, onChange } = useStringWidgetValue(
props.modelValue,
emit
)

// Extract spec options directly
const selectOptions = computed(() => {
const spec = props.widget.spec
if (!spec || !isSelectButtonInputSpec(spec)) {
return []
}
return spec.options?.values || []
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[quality] low Priority

Issue: Potential null reference in options extraction
Context: Line 44 accesses spec.options?.values but could return undefined values, not empty array
Suggestion: Use more explicit fallback: return spec.options?.values ?? [] to ensure type safety

})
</script>
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,8 @@ import { computed } from 'vue'

import { useWidgetValue } from '@/composables/graph/useWidgetValue'
import { useTransformCompatOverlayProps } from '@/composables/useTransformCompatOverlayProps'
import { isTreeSelectInputSpec } from '@/schemas/nodeDef/nodeDefSchemaV2'
import type { SimplifiedWidget } from '@/types/simplifiedWidget'
import {
PANEL_EXCLUDED_PROPS,
filterWidgetProps
} from '@/utils/widgetPropFilter'

import WidgetLayoutField from './layout/WidgetLayoutField.vue'

Expand Down Expand Up @@ -57,15 +54,29 @@ const { localValue, onChange } = useWidgetValue({
// Transform compatibility props for overlay positioning
const transformCompatProps = useTransformCompatOverlayProps()

// TreeSelect specific excluded props
const TREE_SELECT_EXCLUDED_PROPS = [
...PANEL_EXCLUDED_PROPS,
'inputClass',
'inputStyle'
] as const
const combinedProps = computed(() => {
const spec = props.widget.spec
if (!spec || !isTreeSelectInputSpec(spec)) {
return {
...props.widget.options,
...transformCompatProps.value
}
}

const combinedProps = computed(() => ({
...filterWidgetProps(props.widget.options, TREE_SELECT_EXCLUDED_PROPS),
...transformCompatProps.value
}))
const specOptions = spec.options || {}
return {
// Include runtime props like disabled
...props.widget.options,
// PrimeVue TreeSelect expects 'options' to be an array of tree nodes
options: (specOptions.values as TreeNode[]) || [],
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[performance] medium Priority

Issue: Missing memoization for expensive tree node casting
Context: Line 71 performs array casting on every computed update which could be expensive with large tree data
Suggestion: Consider memoizing the tree node transformation or using a more efficient type check

// Convert 'multiple' to PrimeVue's 'selectionMode'
selectionMode: (specOptions.multiple ? 'multiple' : 'single') as
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[architecture] high Priority

Issue: Type safety concern with casting selectionMode from boolean to union type
Context: Line 73 uses type assertion to cast boolean to union type which can break at runtime
Suggestion: Use proper conditional logic: selectionMode: specOptions.multiple ? 'multiple' : 'single' (remove the union type assertion)

| 'single'
| 'multiple'
| 'checkbox',
// Pass through other props like placeholder
placeholder: specOptions.placeholder,
...transformCompatProps.value
}
})
</script>
54 changes: 50 additions & 4 deletions src/schemas/nodeDef/nodeDefSchemaV2.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,8 @@ const zColorInputSpec = zBaseInputOptions.extend({
isOptional: z.boolean().optional(),
options: z
.object({
default: z.string().optional()
default: z.string().optional(),
format: z.enum(['hex', 'rgb', 'hsl', 'hsb']).optional()
})
.optional()
})
Expand All @@ -54,7 +55,13 @@ const zFileUploadInputSpec = zBaseInputOptions.extend({
type: z.literal('FILEUPLOAD'),
name: z.string(),
isOptional: z.boolean().optional(),
options: z.record(z.unknown()).optional()
options: z
.object({
accept: z.string().optional(),
extensions: z.array(z.string()).optional(),
tooltip: z.string().optional()
})
.optional()
})

const zImageInputSpec = zBaseInputOptions.extend({
Expand Down Expand Up @@ -89,7 +96,8 @@ const zTreeSelectInputSpec = zBaseInputOptions.extend({
options: z
.object({
multiple: z.boolean().optional(),
values: z.array(z.unknown()).optional()
values: z.array(z.unknown()).optional(),
placeholder: z.string().optional()
})
.optional()
})
Expand Down Expand Up @@ -123,7 +131,9 @@ const zGalleriaInputSpec = zBaseInputOptions.extend({
isOptional: z.boolean().optional(),
options: z
.object({
images: z.array(z.string()).optional()
images: z.array(z.string()).optional(),
showThumbnails: z.boolean().optional(),
showItemNavigators: z.boolean().optional()
})
.optional()
})
Expand Down Expand Up @@ -262,3 +272,39 @@ export const isChartInputSpec = (
): inputSpec is ChartInputSpec => {
return inputSpec.type === 'CHART'
}

export const isTreeSelectInputSpec = (
inputSpec: InputSpec
): inputSpec is TreeSelectInputSpec => {
return inputSpec.type === 'TREESELECT'
}

export const isSelectButtonInputSpec = (
inputSpec: InputSpec
): inputSpec is SelectButtonInputSpec => {
return inputSpec.type === 'SELECTBUTTON'
}

export const isMultiSelectInputSpec = (
inputSpec: InputSpec
): inputSpec is MultiSelectInputSpec => {
return inputSpec.type === 'MULTISELECT'
}

export const isGalleriaInputSpec = (
inputSpec: InputSpec
): inputSpec is GalleriaInputSpec => {
return inputSpec.type === 'GALLERIA'
}

export const isColorInputSpec = (
inputSpec: InputSpec
): inputSpec is ColorInputSpec => {
return inputSpec.type === 'COLOR'
}

export const isFileUploadInputSpec = (
inputSpec: InputSpec
): inputSpec is FileUploadInputSpec => {
return inputSpec.type === 'FILEUPLOAD'
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[quality] medium Priority

Issue: Missing type guard for ImageInputSpec - no matching isImageInputSpec function exported
Context: All other input specs have corresponding type guard functions, but ImageInputSpec is missing its type guard
Suggestion: Add export const isImageInputSpec = (inputSpec: InputSpec): inputSpec is ImageInputSpec => inputSpec.type === 'IMAGE'

4 changes: 3 additions & 1 deletion tools/devtools/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,13 @@ This directory contains development tools and test utilities for ComfyUI, previo

- `__init__.py` - Server endpoints for development tools (`/api/devtools/*`)
- `dev_nodes.py` - Development and testing nodes for ComfyUI
- `nodes/vue_widgets.py` - Widget showcase nodes used to exercise new Vue-based widgets
- `fake_model.safetensors` - Test fixture for model loading tests

## Purpose

These tools provide:

- Test endpoints for browser automation
- Development nodes for testing various UI features
- Mock data for consistent testing environments
Expand All @@ -25,4 +27,4 @@ cp -r tools/devtools/* /path/to/your/ComfyUI/custom_nodes/ComfyUI_devtools/

## Migration

This directory was created as part of issue #4683 to merge the ComfyUI_devtools repository into the main frontend repository, eliminating the need for separate versioning and simplifying the development workflow.
This directory was created as part of issue #4683 to merge the ComfyUI_devtools repository into the main frontend repository, eliminating the need for separate versioning and simplifying the development workflow.
Loading