-
Notifications
You must be signed in to change notification settings - Fork 440
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.
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
Conversation
📝 WalkthroughWalkthroughReplaces local type-combination helpers with a new exported Changes
Possibly related PRs
Pre-merge checks and finishing touches✅ Passed checks (2 passed)
✨ 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 |
🎨 Storybook Build Status✅ Build completed successfully! ⏰ Completed at: 12/06/2025, 07:50:39 PM UTC 🔗 Links🎉 Your Storybook is ready for review! |
🎭 Playwright Test Results⏰ Completed at: 12/06/2025, 07:59:23 PM UTC 📈 Summary
📊 Test Reports by Browser
🎉 Click on the links above to view detailed test results for each browser configuration. |
Bundle Size ReportSummary
Category Glance Per-category breakdownApp Entry Points — 3.2 MB (baseline 3.2 MB) • 🔴 +104 BMain entry bundles and manifests
Status: 3 added / 3 removed Graph Workspace — 976 kB (baseline 976 kB) • ⚪ 0 BGraph editor runtime, canvas, workflow orchestration
Status: 1 added / 1 removed Views & Navigation — 6.54 kB (baseline 6.54 kB) • ⚪ 0 BTop-level views, pages, and routed surfaces
Status: 1 added / 1 removed Panels & Settings — 298 kB (baseline 298 kB) • ⚪ 0 BConfiguration panels, inspectors, and settings screens
Status: 6 added / 6 removed UI Components — 177 kB (baseline 177 kB) • ⚪ 0 BReusable component library chunks
Status: 9 added / 9 removed Data & Services — 12.5 kB (baseline 12.5 kB) • ⚪ 0 BStores, services, APIs, and repositories
Status: 3 added / 3 removed Utilities & Hooks — 2.94 kB (baseline 2.94 kB) • ⚪ 0 BHelpers, composables, and utility bundles
Status: 1 added / 1 removed Vendor & Third-Party — 8.56 MB (baseline 8.56 MB) • ⚪ 0 BExternal libraries and shared vendor chunks
Status: 5 added / 5 removed Other — 3.81 MB (baseline 3.81 MB) • ⚪ 0 BBundles that do not match a named category
Status: 23 added / 23 removed |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
src/core/graph/widgets/dynamicWidgets.ts(2 hunks)src/lib/litegraph/src/LGraphNode.ts(2 hunks)src/lib/litegraph/src/utils/type.ts(2 hunks)src/utils/typeGuardUtil.ts(0 hunks)
💤 Files with no reviewable changes (1)
- src/utils/typeGuardUtil.ts
🧰 Additional context used
📓 Path-based instructions (8)
src/**/*.{vue,ts}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/**/*.{vue,ts}: Leverage VueUse functions for performance-enhancing styles
Implement proper error handling
Use vue-i18n in composition API for any string literals. Place new translation entries in src/locales/en/main.json
Files:
src/lib/litegraph/src/utils/type.tssrc/core/graph/widgets/dynamicWidgets.tssrc/lib/litegraph/src/LGraphNode.ts
src/**/*.ts
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/**/*.ts: Use es-toolkit for utility functions
Use TypeScript for type safety
Files:
src/lib/litegraph/src/utils/type.tssrc/core/graph/widgets/dynamicWidgets.tssrc/lib/litegraph/src/LGraphNode.ts
src/**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (src/CLAUDE.md)
src/**/*.{ts,tsx,vue}: Sanitize HTML with DOMPurify to prevent XSS attacks
Avoid using @ts-expect-error; use proper TypeScript types instead
Use es-toolkit for utility functions instead of other utility libraries
Implement proper TypeScript types throughout the codebase
Files:
src/lib/litegraph/src/utils/type.tssrc/core/graph/widgets/dynamicWidgets.tssrc/lib/litegraph/src/LGraphNode.ts
src/**/*.{vue,ts,tsx}
📄 CodeRabbit inference engine (src/CLAUDE.md)
Follow Vue 3 composition API style guide
Files:
src/lib/litegraph/src/utils/type.tssrc/core/graph/widgets/dynamicWidgets.tssrc/lib/litegraph/src/LGraphNode.ts
src/lib/litegraph/**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (src/lib/litegraph/CLAUDE.md)
src/lib/litegraph/**/*.{js,ts,jsx,tsx}: Run ESLint instead of manually figuring out whitespace fixes or other trivial style concerns using thepnpm lint:fixcommand
Take advantage ofTypedArraysubarraywhen appropriate
Thesizeandposproperties ofRectangleshare the same array buffer (subarray); they may be used to set the rectangle's size and position
Prefer single lineifsyntax over adding curly braces, when the statement has a very concise expression and concise, single line statement
Do not replace&&=or||=with=when there is no reason to do so. If you do find a reason to remove either&&=or||=, leave a comment explaining why the removal occurred
When writing methods, prefer returning idiomatic JavaScriptundefinedovernull
Files:
src/lib/litegraph/src/utils/type.tssrc/lib/litegraph/src/LGraphNode.ts
src/lib/litegraph/**/*.{ts,tsx}
📄 CodeRabbit inference engine (src/lib/litegraph/CLAUDE.md)
Type assertions are an absolute last resort. In almost all cases, they are a crutch that leads to brittle code
Files:
src/lib/litegraph/src/utils/type.tssrc/lib/litegraph/src/LGraphNode.ts
**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript exclusively; no new JavaScript code
Files:
src/lib/litegraph/src/utils/type.tssrc/core/graph/widgets/dynamicWidgets.tssrc/lib/litegraph/src/LGraphNode.ts
**/*.{ts,vue}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,vue}: Use camelCase for variable and function names
Indent with 2 spaces (see.prettierrc)
Use single quotes for strings (see.prettierrc)
No trailing semicolons (see.prettierrc)
Maximum line width of 80 characters (see.prettierrc)
Sort and group imports by plugin (runpnpm formatbefore committing)
Never useanytype; use proper TypeScript types instead
Never useas anytype assertions; fix the underlying type issue instead
Avoid code comments unless absolutely necessary; write expressive, self-documenting code instead
When writing new code, ask if there is a simpler way to introduce the same functionality; if yes, choose the simpler approach
Use refactoring to make complex code simpler
Use es-toolkit for utility functions
Use Vite for fast development and building
Implement proper error handling
Write tests for all changes, especially bug fixes to catch future regressions
Files:
src/lib/litegraph/src/utils/type.tssrc/core/graph/widgets/dynamicWidgets.tssrc/lib/litegraph/src/LGraphNode.ts
🧠 Learnings (12)
📚 Learning: 2025-11-24T19:47:02.860Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T19:47:02.860Z
Learning: Applies to src/**/*.ts : Use es-toolkit for utility functions
Applied to files:
src/lib/litegraph/src/utils/type.tssrc/core/graph/widgets/dynamicWidgets.ts
📚 Learning: 2025-11-24T19:47:56.371Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/lib/litegraph/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:56.371Z
Learning: Applies to src/lib/litegraph/**/*.{ts,tsx} : Type assertions are an absolute last resort. In almost all cases, they are a crutch that leads to brittle code
Applied to files:
src/lib/litegraph/src/utils/type.tssrc/core/graph/widgets/dynamicWidgets.tssrc/lib/litegraph/src/LGraphNode.ts
📚 Learning: 2025-11-24T19:47:34.324Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:34.324Z
Learning: Applies to src/**/*.{ts,tsx,vue} : Implement proper TypeScript types throughout the codebase
Applied to files:
src/lib/litegraph/src/utils/type.ts
📚 Learning: 2025-11-24T19:47:56.371Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/lib/litegraph/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:56.371Z
Learning: Applies to src/lib/litegraph/**/*.{test,spec}.{ts,tsx} : Use provided test helpers `createTestSubgraph` and `createTestSubgraphNode` from `./fixtures/subgraphHelpers` for consistent subgraph test setup
Applied to files:
src/core/graph/widgets/dynamicWidgets.tssrc/lib/litegraph/src/LGraphNode.ts
📚 Learning: 2025-11-24T19:47:34.324Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:34.324Z
Learning: Applies to src/**/*.{ts,tsx,vue} : Use es-toolkit for utility functions instead of other utility libraries
Applied to files:
src/core/graph/widgets/dynamicWidgets.ts
📚 Learning: 2025-12-06T00:52:35.750Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-06T00:52:35.750Z
Learning: Applies to **/*.{ts,vue} : Use es-toolkit for utility functions
Applied to files:
src/core/graph/widgets/dynamicWidgets.ts
📚 Learning: 2025-11-24T19:47:56.371Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/lib/litegraph/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:56.371Z
Learning: Applies to src/lib/litegraph/**/*.{test,spec}.{ts,tsx} : When writing tests for subgraph-related code, always import from the barrel export at `@/lib/litegraph/src/litegraph` to avoid circular dependency issues
Applied to files:
src/core/graph/widgets/dynamicWidgets.tssrc/lib/litegraph/src/LGraphNode.ts
📚 Learning: 2025-11-24T19:47:56.371Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/lib/litegraph/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:56.371Z
Learning: Applies to src/lib/litegraph/**/*.{test,spec}.{js,ts,jsx,tsx} : When adding features, always write vitest unit tests using cursor rules in @.cursor
Applied to files:
src/core/graph/widgets/dynamicWidgets.ts
📚 Learning: 2025-11-24T19:47:56.371Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/lib/litegraph/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:56.371Z
Learning: Applies to src/lib/litegraph/**/*.{js,ts,jsx,tsx} : Take advantage of `TypedArray` `subarray` when appropriate
Applied to files:
src/core/graph/widgets/dynamicWidgets.tssrc/lib/litegraph/src/LGraphNode.ts
📚 Learning: 2025-11-24T19:47:56.371Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/lib/litegraph/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:56.371Z
Learning: Applies to src/lib/litegraph/**/*.{js,ts,jsx,tsx} : When writing methods, prefer returning idiomatic JavaScript `undefined` over `null`
Applied to files:
src/core/graph/widgets/dynamicWidgets.ts
📚 Learning: 2025-11-24T19:47:56.371Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/lib/litegraph/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:56.371Z
Learning: Applies to src/lib/litegraph/**/*.{js,ts,jsx,tsx} : Prefer single line `if` syntax over adding curly braces, when the statement has a very concise expression and concise, single line statement
Applied to files:
src/core/graph/widgets/dynamicWidgets.ts
📚 Learning: 2025-11-24T19:47:56.371Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/lib/litegraph/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:56.371Z
Learning: Applies to src/lib/litegraph/**/*.{js,ts,jsx,tsx} : The `size` and `pos` properties of `Rectangle` share the same array buffer (`subarray`); they may be used to set the rectangle's size and position
Applied to files:
src/lib/litegraph/src/LGraphNode.ts
🧬 Code graph analysis (3)
src/lib/litegraph/src/utils/type.ts (1)
src/lib/litegraph/src/interfaces.ts (1)
ISlotType(281-281)
src/core/graph/widgets/dynamicWidgets.ts (1)
src/lib/litegraph/src/utils/type.ts (1)
commonType(32-44)
src/lib/litegraph/src/LGraphNode.ts (1)
src/lib/litegraph/src/utils/type.ts (1)
commonType(32-44)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: setup
- GitHub Check: test
- GitHub Check: lint-and-format
- GitHub Check: collect
🔇 Additional comments (6)
src/lib/litegraph/src/utils/type.ts (2)
32-44: Well-structured utility for type intersection.The implementation correctly handles wildcards, comma-separated type lists, and returns
undefinedwhen no common type exists. The use ofes-toolkit'swithoutfollows coding guidelines.
56-58: LGTM!Simple and effective type guard for filtering string slot types.
src/lib/litegraph/src/LGraphNode.ts (2)
13-13: LGTM!Clean import consolidation with
toClassthat was previously imported elsewhere.
2835-2846: LGTM! Correct implementation of common type resolution.The short-circuit evaluation ensures
commonTypeis only called with truthy types, and the fallback chain (maybeCommonType || input.type || output.type) correctly handles cases where no common type exists. This properly addresses the PR objective of coloring links by the greatest common type.src/core/graph/widgets/dynamicWidgets.ts (2)
11-11: LGTM!Clean import migration from local
combineTypesimplementation to the centralizedcommonTypeutility.
291-303: LGTM! Clean migration to centralized type utility.The
commonTypeusage correctly computes:
- Each input's acceptable type by intersecting other connected types with the original allowed types
- The output type from all connected input types
The error handling appropriately catches invalid connection states.
| .map(([key]) => key) | ||
| } | ||
|
|
||
| function isStrings(types: unknown[]): types is string[] { |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
src/lib/litegraph/src/utils/type.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (8)
src/**/*.{vue,ts}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/**/*.{vue,ts}: Leverage VueUse functions for performance-enhancing styles
Implement proper error handling
Use vue-i18n in composition API for any string literals. Place new translation entries in src/locales/en/main.json
Files:
src/lib/litegraph/src/utils/type.ts
src/**/*.ts
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/**/*.ts: Use es-toolkit for utility functions
Use TypeScript for type safety
Files:
src/lib/litegraph/src/utils/type.ts
src/**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (src/CLAUDE.md)
src/**/*.{ts,tsx,vue}: Sanitize HTML with DOMPurify to prevent XSS attacks
Avoid using @ts-expect-error; use proper TypeScript types instead
Use es-toolkit for utility functions instead of other utility libraries
Implement proper TypeScript types throughout the codebase
Files:
src/lib/litegraph/src/utils/type.ts
src/**/*.{vue,ts,tsx}
📄 CodeRabbit inference engine (src/CLAUDE.md)
Follow Vue 3 composition API style guide
Files:
src/lib/litegraph/src/utils/type.ts
src/lib/litegraph/**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (src/lib/litegraph/CLAUDE.md)
src/lib/litegraph/**/*.{js,ts,jsx,tsx}: Run ESLint instead of manually figuring out whitespace fixes or other trivial style concerns using thepnpm lint:fixcommand
Take advantage ofTypedArraysubarraywhen appropriate
Thesizeandposproperties ofRectangleshare the same array buffer (subarray); they may be used to set the rectangle's size and position
Prefer single lineifsyntax over adding curly braces, when the statement has a very concise expression and concise, single line statement
Do not replace&&=or||=with=when there is no reason to do so. If you do find a reason to remove either&&=or||=, leave a comment explaining why the removal occurred
When writing methods, prefer returning idiomatic JavaScriptundefinedovernull
Files:
src/lib/litegraph/src/utils/type.ts
src/lib/litegraph/**/*.{ts,tsx}
📄 CodeRabbit inference engine (src/lib/litegraph/CLAUDE.md)
Type assertions are an absolute last resort. In almost all cases, they are a crutch that leads to brittle code
Files:
src/lib/litegraph/src/utils/type.ts
**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript exclusively; no new JavaScript code
Files:
src/lib/litegraph/src/utils/type.ts
**/*.{ts,vue}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,vue}: Use camelCase for variable and function names
Indent with 2 spaces (see.prettierrc)
Use single quotes for strings (see.prettierrc)
No trailing semicolons (see.prettierrc)
Maximum line width of 80 characters (see.prettierrc)
Sort and group imports by plugin (runpnpm formatbefore committing)
Never useanytype; use proper TypeScript types instead
Never useas anytype assertions; fix the underlying type issue instead
Avoid code comments unless absolutely necessary; write expressive, self-documenting code instead
When writing new code, ask if there is a simpler way to introduce the same functionality; if yes, choose the simpler approach
Use refactoring to make complex code simpler
Use es-toolkit for utility functions
Use Vite for fast development and building
Implement proper error handling
Write tests for all changes, especially bug fixes to catch future regressions
Files:
src/lib/litegraph/src/utils/type.ts
🧠 Learnings (5)
📚 Learning: 2025-11-24T19:47:02.860Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T19:47:02.860Z
Learning: Applies to src/**/*.ts : Use es-toolkit for utility functions
Applied to files:
src/lib/litegraph/src/utils/type.ts
📚 Learning: 2025-11-24T19:47:56.371Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/lib/litegraph/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:56.371Z
Learning: Applies to src/lib/litegraph/**/*.{ts,tsx} : Type assertions are an absolute last resort. In almost all cases, they are a crutch that leads to brittle code
Applied to files:
src/lib/litegraph/src/utils/type.ts
📚 Learning: 2025-11-24T19:47:34.324Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:34.324Z
Learning: Applies to src/**/*.{ts,tsx,vue} : Implement proper TypeScript types throughout the codebase
Applied to files:
src/lib/litegraph/src/utils/type.ts
📚 Learning: 2025-11-24T19:47:56.371Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/lib/litegraph/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:56.371Z
Learning: Applies to src/lib/litegraph/**/*.{js,ts,jsx,tsx} : Take advantage of `TypedArray` `subarray` when appropriate
Applied to files:
src/lib/litegraph/src/utils/type.ts
📚 Learning: 2025-11-24T19:47:56.371Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/lib/litegraph/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:56.371Z
Learning: Applies to src/lib/litegraph/**/*.{js,ts,jsx,tsx} : Do not replace `&&=` or `||=` with `=` when there is no reason to do so. If you do find a reason to remove either `&&=` or `||=`, leave a comment explaining why the removal occurred
Applied to files:
src/lib/litegraph/src/utils/type.ts
🧬 Code graph analysis (1)
src/lib/litegraph/src/utils/type.ts (1)
src/lib/litegraph/src/interfaces.ts (1)
ISlotType(281-281)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: setup
- GitHub Check: lint-and-format
- GitHub Check: collect
- GitHub Check: test
🔇 Additional comments (1)
src/lib/litegraph/src/utils/type.ts (1)
1-3: Imports align with utility and typing guidelinesUsing
withoutfromes-toolkitand bringing inISlotTypevia a type-only import is consistent with the repo’s guidance to centralize utilities and enforce strong typing; nothing to change here.Based on learnings, this matches the preference for es-toolkit and explicit TypeScript types.
| 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[] { | ||
| return types.every((t) => typeof t === 'string') | ||
| } |
There was a problem hiding this comment.
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:
-
Zero‑argument calls return
'*'
With no arguments,isStrings([])passes,withoutWildcardsis[], andcommonType()returns'*'. That’s a bit surprising; “no information” arguably should yieldundefined, 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
- Numeric
ISlotTypevalues are silently dropped
ISlotTypeincludesnumber, butcommonTypecurrently bails out (undefined) if any non‑string appears. If numeric slot types are still used anywhere inLGraphNode/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 thatcommonTypeis 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.
Quick followup adding tests to #7211 ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-7213-Add-tests-for-link-type-2c16d73d365081898707f94a40f2e866) by [Unito](https://www.unito.io)
Previously the color of a link would simply use the type of the target slot and fallback to the type of the origin slot. When a connection is made to a node that accepts the any type ('*'), the link has the green color of an unknown type.
Instead, when a connection is made, the type of a link is now calculated as the greatest common type of the source and destination. This means that connections to reroutes are correctly colored.
The code for calculating common types already exists, it has simply been moved into litegraph and given a more descriptive name.
Resolves #7196
┆Issue is synchronized with this Notion page by Unito