mirror of
https://github.com/Comfy-Org/ComfyUI_frontend.git
synced 2026-04-22 15:29:44 +00:00
fix: improve type safety in type definitions (#7337)
## Summary
- Replace `any` types with proper TypeScript types in core type
definitions
- Add generics to `SettingParams`, `setting.get<T>()`, and
`setting.set<T>()` for type-safe setting access
- Add `NodeExecutionOutput` interface for `onExecuted` callback
- Change function parameters from `any` to `unknown` where appropriate
## Type Research
Performed GitHub code search across custom node repositories to
understand actual usage patterns:
**`onExecuted` output properties** (used in rgthree-comfy,
ComfyUI-KJNodes, ComfyUI-ExLlama-Nodes, comfy_mtb, etc.):
- `output.text` - string or string array for text display nodes
- `output.images`, `output.audio`, `output.video` - media outputs
- `output.ui.items` - complex debug/preview data with `input`, `text`,
`b64_images`
**`extensionManager.setting.get/set`** (used in ComfyUI-Crystools,
ComfyUI-Copilot, etc.):
- Returns various types (boolean, number, string, objects)
- Now uses generics: `setting.get<boolean>('MyExt.Setting')`
**`ComfyExtension` custom properties** (used in rgthree-comfy,
ComfyUI-Manager):
- `aboutPageBadges`, `commands`, custom methods
- Kept as `any` index signature since extensions add arbitrary
properties
## Changes
| File | Change |
|------|--------|
| `extensionTypes.ts` | Generic `setting.get<T>()` and
`setting.set<T>()`, typed Toast options |
| `litegraph-augmentation.d.ts` | `onExecuted(output:
NodeExecutionOutput)` |
| `metadataTypes.ts` | GLTF index signatures `any` → `unknown` |
| `apiSchema.ts` | New `NodeExecutionOutput` interface |
| `settings/types.ts` | `SettingOnChange<T>`, `SettingMigration<T>`,
`SettingParams<TValue>` |
| `nodeDefSchema.ts` | `validateComfyNodeDef(data: unknown)` |
| `workflowSchema.ts` | `isSubgraphDefinition(obj: unknown)` |
| `telemetry/types.ts` | `checkForCompletedTopup(events: AuditLog[])` |
## Test plan
- [x] `pnpm typecheck` passes
- [x] `pnpm test:unit` passes (3732 tests)
- [x] `pnpm lint` passes
┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-7337-fix-improve-type-safety-in-type-definitions-2c66d73d365081bdbc30e916cac607d6)
by [Unito](https://www.unito.io)
This commit is contained in:
committed by
GitHub
parent
3c8b7b015c
commit
b9f75b6cc8
@@ -602,10 +602,12 @@ export const CORE_SETTINGS: SettingParams[] = [
|
||||
defaultValue: [] as Keybinding[],
|
||||
versionAdded: '1.3.7',
|
||||
versionModified: '1.7.3',
|
||||
migrateDeprecatedValue: (value: any[]) => {
|
||||
migrateDeprecatedValue: (
|
||||
value: (Keybinding & { targetSelector?: string })[]
|
||||
) => {
|
||||
return value.map((keybinding) => {
|
||||
if (keybinding['targetSelector'] === '#graph-canvas') {
|
||||
keybinding['targetElementId'] = 'graph-canvas-container'
|
||||
if (keybinding.targetSelector === '#graph-canvas') {
|
||||
keybinding.targetElementId = 'graph-canvas-container'
|
||||
}
|
||||
return keybinding
|
||||
})
|
||||
@@ -780,7 +782,7 @@ export const CORE_SETTINGS: SettingParams[] = [
|
||||
tooltip: 'Server config values used for frontend display only',
|
||||
type: 'hidden',
|
||||
// Mapping from server config id to value.
|
||||
defaultValue: {} as Record<string, any>,
|
||||
defaultValue: {} as Record<string, unknown>,
|
||||
versionAdded: '1.4.8'
|
||||
},
|
||||
{
|
||||
|
||||
@@ -117,9 +117,10 @@ export const useSettingStore = defineStore('setting', () => {
|
||||
return versionedDefault
|
||||
}
|
||||
|
||||
return typeof param.defaultValue === 'function'
|
||||
? param.defaultValue()
|
||||
: param.defaultValue
|
||||
const defaultValue = param.defaultValue
|
||||
return typeof defaultValue === 'function'
|
||||
? (defaultValue as () => Settings[K])()
|
||||
: defaultValue
|
||||
}
|
||||
|
||||
function getVersionedDefaultValue<
|
||||
|
||||
@@ -16,21 +16,21 @@ type SettingInputType =
|
||||
|
||||
type SettingCustomRenderer = (
|
||||
name: string,
|
||||
setter: (v: any) => void,
|
||||
value: any,
|
||||
attrs: any
|
||||
setter: (v: unknown) => void,
|
||||
value: unknown,
|
||||
attrs?: Record<string, unknown>
|
||||
) => HTMLElement
|
||||
|
||||
export interface SettingOption {
|
||||
text: string
|
||||
value?: any
|
||||
value?: string | number
|
||||
}
|
||||
|
||||
export interface SettingParams<TValue = unknown> extends FormItem {
|
||||
export interface SettingParams<TValue = any> extends FormItem {
|
||||
id: keyof Settings
|
||||
defaultValue: any | (() => any)
|
||||
defaultValue: TValue | (() => TValue)
|
||||
defaultsByInstallVersion?: Record<`${number}.${number}.${number}`, TValue>
|
||||
onChange?: (newValue: any, oldValue?: any) => void
|
||||
onChange?: (newValue: TValue, oldValue?: TValue) => void
|
||||
// By default category is id.split('.'). However, changing id to assign
|
||||
// new category has poor backward compatibility. Use this field to overwrite
|
||||
// default category from id.
|
||||
@@ -38,8 +38,7 @@ export interface SettingParams<TValue = unknown> extends FormItem {
|
||||
category?: string[]
|
||||
experimental?: boolean
|
||||
deprecated?: boolean
|
||||
// Deprecated values are mapped to new values.
|
||||
migrateDeprecatedValue?: (value: any) => any
|
||||
migrateDeprecatedValue?: (value: TValue) => TValue
|
||||
// Version of the setting when it was added
|
||||
versionAdded?: string
|
||||
// Version of the setting when it was last modified
|
||||
@@ -57,7 +56,7 @@ export interface FormItem {
|
||||
name: string
|
||||
type: SettingInputType | SettingCustomRenderer
|
||||
tooltip?: string
|
||||
attrs?: Record<string, any>
|
||||
attrs?: Record<string, unknown>
|
||||
options?: Array<string | SettingOption>
|
||||
}
|
||||
|
||||
|
||||
@@ -12,6 +12,8 @@
|
||||
* 3. Check dist/assets/*.js files contain no tracking code
|
||||
*/
|
||||
|
||||
import type { AuditLog } from '@/services/customerEventsService'
|
||||
|
||||
/**
|
||||
* Authentication metadata for sign-up tracking
|
||||
*/
|
||||
@@ -276,7 +278,7 @@ export interface TelemetryProvider {
|
||||
|
||||
// Credit top-up tracking (composition with internal utilities)
|
||||
startTopupTracking(): void
|
||||
checkForCompletedTopup(events: any[] | undefined | null): boolean
|
||||
checkForCompletedTopup(events: AuditLog[] | undefined | null): boolean
|
||||
clearTopupTracking(): void
|
||||
|
||||
// Survey flow events
|
||||
|
||||
@@ -466,14 +466,14 @@ type SubgraphDefinition = z.infer<typeof zSubgraphDefinition>
|
||||
* Type guard to check if an object is a SubgraphDefinition.
|
||||
* This helps TypeScript understand the type when z.lazy() breaks inference.
|
||||
*/
|
||||
export function isSubgraphDefinition(obj: any): obj is SubgraphDefinition {
|
||||
export function isSubgraphDefinition(obj: unknown): obj is SubgraphDefinition {
|
||||
return (
|
||||
obj &&
|
||||
obj !== null &&
|
||||
typeof obj === 'object' &&
|
||||
'id' in obj &&
|
||||
'name' in obj &&
|
||||
'nodes' in obj &&
|
||||
Array.isArray(obj.nodes) &&
|
||||
Array.isArray((obj as SubgraphDefinition).nodes) &&
|
||||
'inputNode' in obj &&
|
||||
'outputNode' in obj
|
||||
)
|
||||
@@ -514,6 +514,7 @@ export async function validateComfyWorkflow(
|
||||
*/
|
||||
const zNodeInputValue = z.union([
|
||||
// For widget values (can be any type)
|
||||
|
||||
z.any(),
|
||||
// For node links [nodeId, slotIndex]
|
||||
z.tuple([zNodeId, zSlotIndex])
|
||||
|
||||
Reference in New Issue
Block a user