Skip to content

[refactor] Type createNode options parameter #9262

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
christian-byrne merged 2 commits into from
Mar 4, 2026
Merged

[refactor] Type createNode options parameter #9262

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
312435d
refactor: type createNode options with CreateNodeOptions interface
Myestery Feb 26, 2026
425084c
refactor: narrow CreateNodeOptions to explicit creation-time properties
Myestery Feb 27, 2026
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
2 changes: 1 addition & 1 deletion src/lib/litegraph/src/LGraphCanvas.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6573,7 +6573,7 @@ export class LGraphCanvas implements CustomEventDispatcher<LGraphCanvasEventMap>
const ySizeFix = opts.posSizeFix[1] * LiteGraph.NODE_SLOT_HEIGHT
const nodeX = opts.position[0] + opts.posAdd[0] + xSizeFix
const nodeY = opts.position[1] + opts.posAdd[1] + ySizeFix
const pos = [nodeX, nodeY]
const pos: [number, number] = [nodeX, nodeY]
const newNode = LiteGraph.createNode(nodeTypeStr, nodeNewOpts?.title, {
pos
})
Expand Down
15 changes: 9 additions & 6 deletions src/lib/litegraph/src/LiteGraphGlobal.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,13 @@ import { Reroute } from './Reroute'
import { InputIndicators } from './canvas/InputIndicators'
import { LabelPosition, SlotDirection, SlotShape, SlotType } from './draw'
import { Rectangle } from './infrastructure/Rectangle'
import type { Dictionary, ISlotType, Rect, WhenNullish } from './interfaces'
import type {
CreateNodeOptions,
Dictionary,
ISlotType,
Rect,
WhenNullish
} from './interfaces'
import { distance, isInsideRectangle, overlapBounding } from './measure'
import { SubgraphIONodeBase } from './subgraph/SubgraphIONodeBase'
import { SubgraphSlot } from './subgraph/SubgraphSlotBase'
Expand Down Expand Up @@ -525,7 +531,7 @@ export class LiteGraphGlobal {
createNode(
type: string,
title?: string,
options?: Dictionary<unknown>
options?: CreateNodeOptions
): LGraphNode | null {
const base_class = this.registered_node_types[type]
if (!base_class) {
Expand Down Expand Up @@ -561,10 +567,7 @@ export class LiteGraphGlobal {

// extra options
if (options) {
for (const i in options) {
// @ts-expect-error #577 Requires interface
node[i] = options[i]
}
Object.assign(node, options)
Copy link
Contributor

@christian-byrne christian-byrne Feb 27, 2026

Choose a reason for hiding this comment

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

note (non-blocking): the old for...in loop traversed the prototype chain, while Object.assign only copies own enumerable properties. That's actually safer behavior - just flagging the subtle semantic difference in case any caller relied on inherited properties (unlikely with plain object literals).

Copy link
Contributor Author

@Myestery Myestery Feb 27, 2026

Choose a reason for hiding this comment

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

Acknowledged: all callers pass plain object literals, so the behavior change is safe. The stricter semantics are actually preferable here

}

// callback
Expand Down
24 changes: 22 additions & 2 deletions src/lib/litegraph/src/interfaces.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,17 @@ import type { CanvasPointerEvent } from '@/lib/litegraph/src/types/events'
import type { TWidgetValue } from '@/lib/litegraph/src/types/widgets'

import type { ContextMenu } from './ContextMenu'
import type { LGraphNode, NodeId } from './LGraphNode'
import type { LGraphNode, NodeId, NodeProperty } from './LGraphNode'
import type { LLink, LinkId } from './LLink'
import type { Reroute, RerouteId } from './Reroute'
import type { SubgraphInput } from './subgraph/SubgraphInput'
import type { SubgraphInputNode } from './subgraph/SubgraphInputNode'
import type { SubgraphOutputNode } from './subgraph/SubgraphOutputNode'
import type { LinkDirection, RenderShape } from './types/globalEnums'
import type {
LGraphEventMode,
LinkDirection,
RenderShape
} from './types/globalEnums'
import type { IBaseWidget } from './types/widgets'

export type Dictionary<T> = { [key: string]: T }
Expand Down Expand Up @@ -373,6 +377,22 @@ export interface INodeOutputSlot extends INodeSlot {
slot_index?: number
}

/** Options for {@link LiteGraphGlobal.createNode}. Shallow-copied onto the new node. */
Copy link
Contributor

@christian-byrne christian-byrne Feb 27, 2026

Choose a reason for hiding this comment

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

question: src/platform/assets/utils/createModelNodeFromAsset.ts already defines a local interface CreateNodeOptions { position?: Point } with a completely different shape, and it imports from the same @/lib/litegraph/src/litegraph barrel. Now that this new CreateNodeOptions is exported from that barrel, any future import will shadow or conflict with the local one. Worth renaming one of them (e.g. the local one to ModelNodeCreateOptions) or acknowledging the collision.

Copy link
Contributor Author

@Myestery Myestery Feb 27, 2026

Choose a reason for hiding this comment

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

Done: renamed to ModelNodeCreateOptions in the previous commit

export interface CreateNodeOptions {
pos?: Point
size?: Size
properties?: Dictionary<NodeProperty | undefined>
flags?: Partial<INodeFlags>
mode?: LGraphEventMode
color?: string
bgcolor?: string
boxcolor?: string
title?: string
shape?: RenderShape
inputs?: Partial<INodeInputSlot>[]
outputs?: Partial<INodeOutputSlot>[]
Comment on lines +392 to +393
Copy link
Contributor

@coderabbitai coderabbitai bot Feb 27, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

# First, let's find the interfaces.ts file and examine the current type definitions
fd "interfaces\.ts" src/lib/litegraph --exec wc -l {}

Repository: Comfy-Org/ComfyUI_frontend

Length of output: 109

🏁 Script executed:

# Look at the CreateNodeOptions type and related interfaces
rg -n "type CreateNodeOptions|interface CreateNodeOptions" --type ts src/lib/litegraph

Repository: Comfy-Org/ComfyUI_frontend

Length of output: 146

🏁 Script executed:

# Find the INodeInputSlot and INodeOutputSlot definitions
rg -n "interface INodeInputSlot|interface INodeOutputSlot" --type ts src/lib/litegraph

Repository: Comfy-Org/ComfyUI_frontend

Length of output: 254

🏁 Script executed:

# Search for createNode function calls to see actual usage patterns
rg -n "createNode\s*\(" --type ts -C 2 src/lib/litegraph

Repository: Comfy-Org/ComfyUI_frontend

Length of output: 14236

🏁 Script executed:

# Search for where inputs and outputs are being set in node creation context
rg -n "inputs\s*:\s*\[" --type ts -C 2 src/lib/litegraph | head -50

Repository: Comfy-Org/ComfyUI_frontend

Length of output: 3957

🏁 Script executed:

# Also check for outputs assignments
rg -n "outputs\s*:\s*\[" --type ts -C 2 src/lib/litegraph | head -50

Repository: Comfy-Org/ComfyUI_frontend

Length of output: 3337

🏁 Script executed:

# Read the interfaces.ts file around the relevant definitions
sed -n '355,400p' src/lib/litegraph/src/interfaces.ts

Repository: Comfy-Org/ComfyUI_frontend

Length of output: 1102

🏁 Script executed:

# Read more to see INodeSlot definition
sed -n '350,365p' src/lib/litegraph/src/interfaces.ts

Repository: Comfy-Org/ComfyUI_frontend

Length of output: 536

🏁 Script executed:

# Search for all usages of inputs/outputs in CreateNodeOptions context
# to understand if empty arrays and partial objects are common
rg "inputs\s*:\s*\[\s*\]|outputs\s*:\s*\[\s*\]" --type ts src/lib/litegraph -C 1

Repository: Comfy-Org/ComfyUI_frontend

Length of output: 2888

🏁 Script executed:

# Check if there are any createNode calls that pass inputs/outputs with incomplete fields
rg "createNode\s*\([^)]*\{[^}]*inputs|outputs" --type ts -A 5 src/lib/litegraph | head -80

Repository: Comfy-Org/ComfyUI_frontend

Length of output: 5485

🏁 Script executed:

# Look for any createNode calls with inputs/outputs that have incomplete field definitions
# Search for patterns where inputs/outputs have objects but might be missing name or type
rg "inputs\s*:\s*\[\s*\{" --type ts -A 3 src/lib/litegraph | grep -v "name\|type"

Repository: Comfy-Org/ComfyUI_frontend

Length of output: 12762

🏁 Script executed:

# Check the actual createNode implementation to see how it handles inputs/outputs
rg -n "createNode\(" --type ts -A 20 src/lib/litegraph/src/LiteGraphGlobal.ts | head -80

Repository: Comfy-Org/ComfyUI_frontend

Length of output: 668

🏁 Script executed:

# Look for SubgraphConversion test's createNode to understand the test helper
sed -n '16,50p' src/lib/litegraph/src/subgraph/SubgraphConversion.test.ts

Repository: Comfy-Org/ComfyUI_frontend

Length of output: 1153

🏁 Script executed:

# Look at how createNode actually uses the inputs/outputs from CreateNodeOptions
sed -n '531,600p' src/lib/litegraph/src/LiteGraphGlobal.ts

Repository: Comfy-Org/ComfyUI_frontend

Length of output: 1863

🏁 Script executed:

# Check if there are any real cases where inputs/outputs objects are provided without name field
rg "inputs\s*:\s*\[\s*\{[^}]*\}" --type ts src/lib/litegraph -A 1 | grep -B 1 -v "name"

Repository: Comfy-Org/ComfyUI_frontend

Length of output: 10134

🏁 Script executed:

# Search for actual complete usage of createNode with inputs/outputs options
rg "LiteGraph\.createNode\([^)]*inputs:|LiteGraph\.createNode\([^)]*outputs:" --type ts -B 2 -A 5 src/lib/litegraph

Repository: Comfy-Org/ComfyUI_frontend

Length of output: 52

Tighten inputs/outputs slot types to require core fields.

The current Partial<INodeInputSlot>[] and Partial<INodeOutputSlot>[] types allow creating nodes with incomplete slot objects (e.g., missing name). Consider requiring at least the name field—e.g., (Partial<INodeInputSlot> & { name: string })[]—to enforce stronger compile-time guarantees and prevent malformed slot definitions at runtime.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@src/lib/litegraph/src/interfaces.ts` around lines 392 - 393, The types for
the node slot arrays allow missing required fields; tighten the types for the
properties `inputs` and `outputs` (currently declared as
`Partial<INodeInputSlot>[]` and `Partial<INodeOutputSlot>[]`) to require at
least a `name` property so malformed slots can't be created—e.g., change each to
a union that intersects Partial with `{ name: string }` (e.g.,
`(Partial<INodeInputSlot> & { name: string })[]`) and apply the analogous change
for `INodeOutputSlot` so `outputs` become `(Partial<INodeOutputSlot> & { name:
string })[]`.

}

/** Links */
export interface ConnectingLink extends IInputOrOutput {
node: LGraphNode
Expand Down
1 change: 1 addition & 0 deletions src/lib/litegraph/src/litegraph.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ export { RecursionError } from './infrastructure/RecursionError'
export type {
CanvasColour,
ColorOption,
CreateNodeOptions,
IContextMenuOptions,
IContextMenuValue,
INodeInputSlot,
Expand Down
4 changes: 2 additions & 2 deletions src/platform/assets/utils/createModelNodeFromAsset.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import { app } from '@/scripts/app'
import { useLitegraphService } from '@/services/litegraphService'
import { useModelToNodeStore } from '@/stores/modelToNodeStore'

interface CreateNodeOptions {
interface ModelNodeCreateOptions {
position?: Point
}

Expand Down Expand Up @@ -48,7 +48,7 @@ type Result<T, E> = { success: true; value: T } | { success: false; error: E }
*/
export function createModelNodeFromAsset(
asset: AssetItem,
options?: CreateNodeOptions
options?: ModelNodeCreateOptions
): Result<LGraphNode, NodeCreationError> {
const validatedAsset = assetItemSchema.safeParse(asset)

Expand Down
3 changes: 2 additions & 1 deletion src/services/litegraphService.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import {
createBounds
} from '@/lib/litegraph/src/litegraph'
import type {
CreateNodeOptions,
GraphAddOptions,
IContextMenuValue,
Point,
Expand Down Expand Up @@ -885,7 +886,7 @@ export const useLitegraphService = () => {

function addNodeOnGraph(
nodeDef: ComfyNodeDefV1 | ComfyNodeDefV2,
options: Record<string, unknown> & { pos?: Point } = {},
options: CreateNodeOptions = {},
addOptions?: GraphAddOptions
): LGraphNode | null {
options.pos ??= getCanvasCenter()
Expand Down
Loading