[refactor] Type createNode options parameter#9262
Merged
christian-byrne merged 2 commits intomainfrom Mar 4, 2026
Merged
Conversation
Replace untyped Dictionary<unknown> parameter and @ts-expect-error suppression in createNode with a proper CreateNodeOptions interface that extends Partial<Omit<LGraphNode, 'constructor' | 'inputs' | 'outputs'>> and widens inputs/outputs to accept partial slot objects. Fixes #4740
dosubot
bot
added
the
size:S
🎨 Storybook: ✅ Built — View Storybook |
🎭 Playwright: ✅ 545 passed, 0 failed · 6 flaky📊 Browser Reports
|
Contributor
📝 WalkthroughWalkthroughIntroduces a typed Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
📦 Bundle: 4.46 MB gzip 🟢 -143 BDetailsSummary
Category Glance App Entry Points — 17.9 kB (baseline 17.9 kB) • ⚪ 0 BMain entry bundles and manifests
Status: 1 added / 1 removed Graph Workspace — 1.02 MB (baseline 1.02 MB) • ⚪ 0 BGraph editor runtime, canvas, workflow orchestration
Status: 1 added / 1 removed Views & Navigation — 72.1 kB (baseline 72.1 kB) • ⚪ 0 BTop-level views, pages, and routed surfaces
Status: 9 added / 9 removed Panels & Settings — 435 kB (baseline 435 kB) • ⚪ 0 BConfiguration panels, inspectors, and settings screens
Status: 10 added / 10 removed User & Accounts — 16 kB (baseline 16 kB) • ⚪ 0 BAuthentication, profile, and account management bundles
Status: 5 added / 5 removed Editors & Dialogs — 736 B (baseline 736 B) • ⚪ 0 BModals, dialogs, drawers, and in-app editors
Status: 1 added / 1 removed UI Components — 47 kB (baseline 47 kB) • ⚪ 0 BReusable component library chunks
Status: 5 added / 5 removed Data & Services — 2.55 MB (baseline 2.55 MB) • 🟢 -17 BStores, services, APIs, and repositories
Status: 13 added / 13 removed Utilities & Hooks — 55.5 kB (baseline 55.5 kB) • ⚪ 0 BHelpers, composables, and utility bundles
Status: 11 added / 11 removed Vendor & Third-Party — 8.84 MB (baseline 8.84 MB) • ⚪ 0 BExternal libraries and shared vendor chunks
Other — 7.86 MB (baseline 7.86 MB) • ⚪ 0 BBundles that do not match a named category
Status: 52 added / 52 removed |
⚡ Performance Report
Raw data{
"timestamp": "2026-02-27T04:05:23.177Z",
"gitSha": "b888d9e5333a0e0a6665507dc417eefb146faddb",
"branch": "refactor/typed-create-node-options",
"measurements": [
{
"name": "canvas-idle",
"durationMs": 2057.9369999999813,
"styleRecalcs": 125,
"styleRecalcDurationMs": 19.741000000000003,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 457.51200000000006,
"heapDeltaBytes": -2095684
},
{
"name": "canvas-mouse-sweep",
"durationMs": 1895.103000000006,
"styleRecalcs": 174,
"styleRecalcDurationMs": 50.606,
"layouts": 12,
"layoutDurationMs": 3.621,
"taskDurationMs": 785.2099999999999,
"heapDeltaBytes": -3900884
},
{
"name": "dom-widget-clipping",
"durationMs": 555.6569999999965,
"styleRecalcs": 41,
"styleRecalcDurationMs": 11.928,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 337.888,
"heapDeltaBytes": 6709840
}
]
} |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/lib/litegraph/src/litegraph.ts (1)
94-94: Avoid expanding barrel exports unless this must be part of the public API.This adds another re-export from a
/srcbarrel. If only internal callers need the type, prefer importingCreateNodeOptionsdirectly from./interfacesat use sites.As per coding guidelines: "Don't use barrel files (e.g.,
/some/package/index.ts) to re-export within/src" and "Minimize the surface area (exported values) of each module and composable."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/litegraph/src/litegraph.ts` at line 94, Remove CreateNodeOptions from the exported list in litegraph.ts and instead import CreateNodeOptions directly from its defining module (./interfaces) at the places that consume it; update any files currently relying on the new re-export to import CreateNodeOptions from ./interfaces, leaving litegraph.ts exports unchanged to avoid expanding the /src barrel surface area.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/lib/litegraph/src/litegraph.ts`:
- Line 94: Remove CreateNodeOptions from the exported list in litegraph.ts and
instead import CreateNodeOptions directly from its defining module
(./interfaces) at the places that consume it; update any files currently relying
on the new re-export to import CreateNodeOptions from ./interfaces, leaving
litegraph.ts exports unchanged to avoid expanding the /src barrel surface area.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/lib/litegraph/src/LGraphCanvas.tssrc/lib/litegraph/src/LiteGraphGlobal.tssrc/lib/litegraph/src/interfaces.tssrc/lib/litegraph/src/litegraph.tssrc/services/litegraphService.ts
| slot_index?: number | ||
| } | ||
|
|
||
| /** Options for {@link LiteGraphGlobal.createNode}. Shallow-copied onto the new node. */ |
Contributor
There was a problem hiding this comment.
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.
Contributor
Author
There was a problem hiding this comment.
Done: renamed to ModelNodeCreateOptions in the previous commit
src/lib/litegraph/src/interfaces.ts
Outdated
| } | ||
|
|
||
| /** Options for {@link LiteGraphGlobal.createNode}. Shallow-copied onto the new node. */ | ||
| export interface CreateNodeOptions extends Partial< |
Contributor
There was a problem hiding this comment.
nitpick (non-blocking): constructor is not a member of keyof LGraphNode (TypeScript class instance types don't include it), so Omit<LGraphNode, 'constructor' | ...> is equivalent to Omit<LGraphNode, 'inputs' | 'outputs'>. Keeping it is harmless as a guard, just noting it's a no-op today.
Contributor
Author
There was a problem hiding this comment.
the interface no longer extends Partial<Omit<LGraphNode, ...>>, it's an explicit property list
| // @ts-expect-error #577 Requires interface | ||
| node[i] = options[i] | ||
| } | ||
| Object.assign(node, options) |
Contributor
There was a problem hiding this comment.
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).
Contributor
Author
There was a problem hiding this comment.
Acknowledged: all callers pass plain object literals, so the behavior change is safe. The stricter semantics are actually preferable here
dosubot
bot
added
size:M
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/lib/litegraph/src/interfaces.ts`:
- Around line 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 })[]`.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/lib/litegraph/src/interfaces.tssrc/platform/assets/utils/createModelNodeFromAsset.ts
Comment on lines
+392
to
+393
| inputs?: Partial<INodeInputSlot>[] | ||
| outputs?: Partial<INodeOutputSlot>[] |
Contributor
There was a problem hiding this comment.
🧩 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/litegraphRepository: 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/litegraphRepository: 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/litegraphRepository: 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 -50Repository: 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 -50Repository: 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.tsRepository: 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.tsRepository: 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 1Repository: 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 -80Repository: 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 -80Repository: 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.tsRepository: 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.tsRepository: 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/litegraphRepository: 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 })[]`.
christian-byrne
approved these changes
Mar 4, 2026
Myestery
added a commit
that referenced
this pull request
Mar 5, 2026
## Summary Narrow `CreateNodeOptions` from `Partial<Omit<LGraphNode, ...>>` (exposing hundreds of properties/methods) to an explicit interface listing only creation-time properties. ## Changes - Replace `Partial<Omit<LGraphNode, 'constructor' | 'inputs' | 'outputs'>>` with explicit `CreateNodeOptions` interface containing only: `pos`, `size`, `properties`, `flags`, `mode`, `color`, `bgcolor`, `boxcolor`, `title`, `shape`, `inputs`, `outputs` - Rename local `CreateNodeOptions` in `createModelNodeFromAsset.ts` to `ModelNodeCreateOptions` to avoid collision ## Ecosystem verification GitHub code search across ~50 repos confirms only `pos` and `outputs` are used externally. All covered by the narrowed interface. Fixes #9276 Fixes #4740
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Narrow
CreateNodeOptionsfromPartial<Omit<LGraphNode, ...>>(exposing hundreds of properties/methods) to an explicit interface listing only creation-time properties.Changes
Partial<Omit<LGraphNode, 'constructor' | 'inputs' | 'outputs'>>with explicitCreateNodeOptionsinterface containing only:pos,size,properties,flags,mode,color,bgcolor,boxcolor,title,shape,inputs,outputsCreateNodeOptionsincreateModelNodeFromAsset.tstoModelNodeCreateOptionsto avoid collisionEcosystem verification
GitHub code search across ~50 repos confirms only
posandoutputsare used externally. All covered by the narrowed interface.Fixes #9276
Fixes #4740