Skip to content

Color links as common type #7211

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
AustinMroz merged 2 commits into from
Dec 6, 2025
Merged

Color links as common type #7211

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1dbca30
Color links as common type
AustinMroz Dec 6, 2025
f57b3d1
Apply suggestion from @coderabbitai[bot]
DrJKL Dec 6, 2025
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
34 changes: 3 additions & 31 deletions src/core/graph/widgets/dynamicWidgets.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
import { without } from 'es-toolkit'

import { useChainCallback } from '@/composables/functional/useChainCallback'
import { NodeSlotType } from '@/lib/litegraph/src/types/globalEnums'
import type {
Expand All @@ -10,6 +8,7 @@ import type {
import type { LGraphNode } from '@/lib/litegraph/src/LGraphNode'
import { LiteGraph } from '@/lib/litegraph/src/litegraph'
import type { LLink } from '@/lib/litegraph/src/LLink'
import { commonType } from '@/lib/litegraph/src/utils/type'
import { transformInputSpecV1ToV2 } from '@/schemas/nodeDef/migration'
import type { ComboInputSpec, InputSpec } from '@/schemas/nodeDefSchema'
import type { InputSpec as InputSpecV2 } from '@/schemas/nodeDef/nodeDefSchemaV2'
Expand All @@ -20,7 +19,6 @@ import {
import { useLitegraphService } from '@/services/litegraphService'
import { app } from '@/scripts/app'
import type { ComfyApp } from '@/scripts/app'
import { isStrings } from '@/utils/typeGuardUtil'

const INLINE_INPUTS = false

Expand Down Expand Up @@ -244,30 +242,6 @@ function changeOutputType(
}
}

function combineTypes(...types: ISlotType[]): ISlotType | undefined {
if (!isStrings(types)) return undefined

const withoutWildcards = without(types, '*')
if (withoutWildcards.length === 0) return '*'

const typeLists: string[][] = withoutWildcards.map((type) => type.split(','))

const combinedTypes = intersection(...typeLists)
if (combinedTypes.length === 0) return undefined

return combinedTypes.join(',')
}

function intersection(...sets: string[][]): string[] {
const itemCounts: Record<string, number> = {}
for (const set of sets)
for (const item of new Set(set))
itemCounts[item] = (itemCounts[item] ?? 0) + 1
return Object.entries(itemCounts)
.filter(([, count]) => count == sets.length)
.map(([key]) => key)
}

function withComfyMatchType(node: LGraphNode): asserts node is MatchTypeNode {
if (node.comfyMatchType) return
node.comfyMatchType = {}
Expand All @@ -290,8 +264,6 @@ function withComfyMatchType(node: LGraphNode): asserts node is MatchTypeNode {
if (!matchGroup) return
if (iscon && linf) {
const { output, subgraphInput } = linf.resolve(this.graph)
//TODO: fix this bug globally. A link type (and therefore color)
//should be the combinedType of origin and target type
const connectingType = (output ?? subgraphInput)?.type
if (connectingType) linf.type = connectingType
}
Expand All @@ -316,14 +288,14 @@ function withComfyMatchType(node: LGraphNode): asserts node is MatchTypeNode {
...connectedTypes.slice(0, idx),
...connectedTypes.slice(idx + 1)
]
const combinedType = combineTypes(
const combinedType = commonType(
...otherConnected,
matchGroup[input.name]
)
if (!combinedType) throw new Error('invalid connection')
input.type = combinedType
})
const outputType = combineTypes(...connectedTypes)
const outputType = commonType(...connectedTypes)
if (!outputType) throw new Error('invalid connection')
this.outputs.forEach((output, idx) => {
if (!(outputGroups?.[idx] == matchKey)) return
Expand Down
7 changes: 5 additions & 2 deletions src/lib/litegraph/src/LGraphNode.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import { useLayoutMutations } from '@/renderer/core/layout/operations/layoutMuta
import { LayoutSource } from '@/renderer/core/layout/types'
import { adjustColor } from '@/utils/colorUtil'
import type { ColorAdjustOptions } from '@/utils/colorUtil'
import { commonType, toClass } from '@/lib/litegraph/src/utils/type'

import { SUBGRAPH_OUTPUT_ID } from '@/lib/litegraph/src/constants'
import type { DragAndScale } from './DragAndScale'
Expand Down Expand Up @@ -84,7 +85,6 @@ import { findFreeSlotOfType } from './utils/collections'
import { warnDeprecated } from './utils/feedback'
import { distributeSpace } from './utils/spaceDistribution'
import { truncateText } from './utils/textUtils'
import { toClass } from './utils/type'
import { BaseWidget } from './widgets/BaseWidget'
import { toConcreteWidget } from './widgets/widgetMap'
import type { WidgetTypeMap } from './widgets/widgetMap'
Expand Down Expand Up @@ -2832,9 +2832,12 @@ export class LGraphNode
inputNode.disconnectInput(inputIndex, true)
}

const maybeCommonType =
input.type && output.type && commonType(input.type, output.type)

const link = new LLink(
++graph.state.lastLinkId,
input.type || output.type,
maybeCommonType || input.type || output.type,
this.id,
outputIndex,
inputNode.id,
Expand Down
32 changes: 31 additions & 1 deletion src/lib/litegraph/src/utils/type.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
import type { IColorable } from '@/lib/litegraph/src/interfaces'
import { without } from 'es-toolkit'

import type { IColorable, ISlotType } from '@/lib/litegraph/src/interfaces'

/**
* Converts a plain object to a class instance if it is not already an instance of the class.
Expand Down Expand Up @@ -26,3 +28,31 @@ export function isColorable(obj: unknown): obj is IColorable {
'getColorOption' in obj
)
}

export function commonType(...types: ISlotType[]): ISlotType | undefined {
if (!isStrings(types)) return undefined

const withoutWildcards = without(types, '*')
if (withoutWildcards.length === 0) return '*'

const typeLists: string[][] = withoutWildcards.map((type) => type.split(','))

const combinedTypes = intersection(...typeLists)
if (combinedTypes.length === 0) return undefined

return combinedTypes.join(',')
}

function intersection(...sets: string[][]): string[] {
const itemCounts: Record<string, number> = {}
for (const set of sets)
for (const item of new Set(set))
itemCounts[item] = (itemCounts[item] ?? 0) + 1
return Object.entries(itemCounts)
.filter(([, count]) => count === sets.length)
.map(([key]) => key)
}

function isStrings(types: unknown[]): types is string[] {
Copy link
Collaborator Author

@AustinMroz AustinMroz Dec 6, 2025

Choose a reason for hiding this comment

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

A little conflicted about moving this here, but I think it's good enough.
+ commonType should be in litegraph and should see greater use.
+ Having litegraph reference outside code is bad.
- There's some conflation of litegraph connection types and programming types.
- The function is no longer exported. It's less clear that it's intended as utility.

Copy link
Contributor

@DrJKL DrJKL Dec 6, 2025

Choose a reason for hiding this comment

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

Intent is less important than how it's currently being used, so I support this.
If we had something that was serving as a utility and could be put into the frontend utilities package for use here, that would also be good, but YAGNI.

return types.every((t) => typeof t === 'string')
}
Comment on lines +32 to +58
Copy link
Contributor

@coderabbitai coderabbitai bot Dec 6, 2025

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial

Tighten commonType edge cases and confirm numeric ISlotType handling

The core behavior (ignoring '*' except when all types are wildcards, intersecting comma‑separated lists, and returning undefined on empty intersections) looks correct for the link‑coloring use case. The intersection helper correctly de‑duplicates within each set and preserves first‑set ordering.

Two edge cases are worth tightening/confirming:

  1. Zero‑argument calls return '*'
    With no arguments, isStrings([]) passes, withoutWildcards is [], and commonType() returns '*'. That’s a bit surprising; “no information” arguably should yield undefined, not “any”. You can make this explicit with an early guard:

    export function commonType(...types: ISlotType[]): ISlotType | undefined {
  • if (!isStrings(types)) return undefined
  • if (types.length === 0) return undefined
  • if (!isStrings(types)) return undefined
  1. Numeric ISlotType values are silently dropped
    ISlotType includes number, but commonType currently bails out (undefined) if any non‑string appears. If numeric slot types are still used anywhere in LGraphNode/widgets, this could regress link type inference for those paths. If not, it may be fine, but it’s worth double‑checking the call sites and, if needed, documenting that commonType is string‑only.

Additionally, if any callers ever produce comma‑separated lists with spaces (e.g. 'image, mask'), the lack of trimming could prevent expected intersections (' mask' vs 'mask'); if that’s a realistic input, consider normalizing entries before counting.

If you confirm that commonType is only ever called with 1+ string slot types, the current design is good; otherwise, consider the guard tweak above and/or handling numeric types explicitly.
Based on learnings, this keeps the utility type‑safe while matching the project’s TypeScript and es-toolkit usage patterns.

🤖 Prompt for AI Agents 
In src/lib/litegraph/src/utils/type.ts around lines 32–58, add an explicit guard
so commonType() returns undefined when called with zero arguments (types.length
=== 0) instead of returning '*'; also tighten input handling by normalizing slot
entries: allow numeric ISlotType by converting numbers to strings (or update the
type-guard to accept number | string), and when splitting comma lists trim each
entry before building the intersection to avoid mismatches caused by whitespace.
Ensure intersection still de-duplicates per set and that the final result joins
the normalized strings or returns undefined when empty.

4 changes: 0 additions & 4 deletions src/utils/typeGuardUtil.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,3 @@ export const isResultItemType = (
): value is ResultItemType => {
return value === 'input' || value === 'output' || value === 'temp'
}

export function isStrings(types: unknown[]): types is string[] {
return types.every((t) => typeof t === 'string')
}