Implement a CustomCombo node #7142
Conversation
dosubot
bot
added
the
size:L
📝 WalkthroughWalkthroughAdds a frontend LiteGraph extension implementing a CustomCombo node that manages dynamic string option widgets, wires a combo widget with custom getters/setters and auto-added options, and propagates the selected value along downstream links via an added applyToGraph flow. Registers the node in system definitions. Changes
sequenceDiagram
autonumber
participant User
participant CustomCombo as Extension
participant Graph
participant DownNode as DownstreamNode
participant Widget
User->>CustomCombo: change combo selection / trigger widget callback
CustomCombo->>Graph: applyToGraph(extraLinks?)
Graph->>Graph: gather outgoing Links from combo node
Graph->>DownNode: set input widget value (via link target)
DownNode->>Widget: update widget.value (setter/getter)
Widget->>DownNode: invoke original callback with synthetic CanvasPointerEvent
DownNode-->>User: downstream node updated
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: ASSERTIVE Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (11)**/*.{vue,ts,tsx}📄 CodeRabbit inference engine (.cursorrules)
Files:
**/*.{ts,tsx,js}📄 CodeRabbit inference engine (.cursorrules)
Files:
**/*.{ts,tsx}📄 CodeRabbit inference engine (.cursorrules)
Files:
**/*.{ts,tsx,js,vue}📄 CodeRabbit inference engine (.cursorrules)
Files:
src/**/*.{vue,ts}📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
src/**/*.ts📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
**/*.{ts,tsx,js,jsx,vue}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
**/*.{ts,tsx,vue}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
**/*.ts📄 CodeRabbit inference engine (CLAUDE.md)
Files:
src/**/*.{ts,tsx,vue}📄 CodeRabbit inference engine (src/CLAUDE.md)
Files:
src/**/*.{vue,ts,tsx}📄 CodeRabbit inference engine (src/CLAUDE.md)
Files:
🧬 Code graph analysis (1)src/extensions/core/customCombo.ts (3)
⏰ 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)
🔇 Additional comments (1)
Comment |
🎨 Storybook Build Status✅ Build completed successfully! ⏰ Completed at: 12/04/2025, 06:44:24 AM UTC 🔗 Links🎉 Your Storybook is ready for review! |
🎭 Playwright Test Results⏰ Completed at: 12/04/2025, 06:52:44 AM 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) • 🔴 +3.4 kBMain entry bundles and manifests
Status: 3 added / 3 removed Graph Workspace — 972 kB (baseline 972 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 — 173 kB (baseline 173 kB) • ⚪ 0 BReusable component library chunks
Status: 6 added / 6 removed Data & Services — 12.5 kB (baseline 12.5 kB) • ⚪ 0 BStores, services, APIs, and repositories
Status: 2 added / 2 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
Other — 3.81 MB (baseline 3.81 MB) • ⚪ 0 BBundles that do not match a named category
Status: 17 added / 17 removed |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/extensions/core/customCombo.ts (2)
20-21: Consider adding a null check instead of non-null assertion.The
!assertion at line 21 assumesaddWidgetalways succeeds andat(-1)returns a value. While this should be true immediately afteraddWidget, a defensive check would be safer.this.addWidget('combo', 'choice', '', () => {}, options) - const comboWidget = this.widgets?.at(-1)! + const comboWidget = this.widgets?.at(-1) + if (!comboWidget) return
31-61: Widget management is clever but could benefit from a brief comment.The auto-grow/shrink logic via
Object.definePropertyis functional but non-obvious. Consider adding a brief inline comment explaining the behavior: new widgets are added when the last option is filled, and empty trailing widgets are removed when the second-to-last becomes empty.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/extensions/core/customCombo.ts(1 hunks)src/extensions/core/index.ts(1 hunks)src/stores/nodeDefStore.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (13)
**/*.{vue,ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{vue,ts,tsx}: Leverage VueUse functions for performance-enhancing utilities
Use vue-i18n in Composition API for any string literals and place new translation entries in src/locales/en/main.json
Files:
src/stores/nodeDefStore.tssrc/extensions/core/index.tssrc/extensions/core/customCombo.ts
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursorrules)
Use es-toolkit for utility functions
Files:
src/stores/nodeDefStore.tssrc/extensions/core/index.tssrc/extensions/core/customCombo.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
Use TypeScript for type safety
**/*.{ts,tsx}: Never useanytype - use proper TypeScript types
Never useas anytype assertions - fix the underlying type issue
Files:
src/stores/nodeDefStore.tssrc/extensions/core/index.tssrc/extensions/core/customCombo.ts
**/*.{ts,tsx,js,vue}
📄 CodeRabbit inference engine (.cursorrules)
Implement proper error handling in components and services
**/*.{ts,tsx,js,vue}: Use 2-space indentation, single quotes, no semicolons, and maintain 80-character line width as configured in.prettierrc
Organize imports by sorting and grouping by plugin, and runpnpm formatbefore committing
Files:
src/stores/nodeDefStore.tssrc/extensions/core/index.tssrc/extensions/core/customCombo.ts
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/stores/nodeDefStore.tssrc/extensions/core/index.tssrc/extensions/core/customCombo.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/stores/nodeDefStore.tssrc/extensions/core/index.tssrc/extensions/core/customCombo.ts
**/*.{ts,tsx,js,jsx,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
Use camelCase for variable and setting names in TypeScript/Vue files
Files:
src/stores/nodeDefStore.tssrc/extensions/core/index.tssrc/extensions/core/customCombo.ts
**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx,vue}: Useconst settingStore = useSettingStore()andsettingStore.get('Comfy.SomeSetting')to retrieve settings in TypeScript/Vue files
Useawait settingStore.set('Comfy.SomeSetting', newValue)to update settings in TypeScript/Vue files
Check server capabilities usingapi.serverSupportsFeature('feature_name')before using enhanced features
Useapi.getServerFeature('config_name', defaultValue)to retrieve server feature configurationEnforce ESLint rules for Vue + TypeScript including: no floating promises, no unused imports, and i18n raw text restrictions in templates
Files:
src/stores/nodeDefStore.tssrc/extensions/core/index.tssrc/extensions/core/customCombo.ts
**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.ts: Define dynamic setting defaults using runtime context with functions in settings configuration
UsedefaultsByInstallVersionproperty for gradual feature rollout based on version in settings configuration
Files:
src/stores/nodeDefStore.tssrc/extensions/core/index.tssrc/extensions/core/customCombo.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/stores/nodeDefStore.tssrc/extensions/core/index.tssrc/extensions/core/customCombo.ts
src/**/stores/**/*.{ts,tsx}
📄 CodeRabbit inference engine (src/CLAUDE.md)
src/**/stores/**/*.{ts,tsx}: Maintain clear public interfaces and restrict extension access in stores
Use TypeScript for type safety in state management stores
Files:
src/stores/nodeDefStore.ts
src/**/*.{vue,ts,tsx}
📄 CodeRabbit inference engine (src/CLAUDE.md)
Follow Vue 3 composition API style guide
Files:
src/stores/nodeDefStore.tssrc/extensions/core/index.tssrc/extensions/core/customCombo.ts
**/stores/*Store.ts
📄 CodeRabbit inference engine (AGENTS.md)
Name Pinia stores with the
*Store.tssuffix
Files:
src/stores/nodeDefStore.ts
🧬 Code graph analysis (1)
src/extensions/core/customCombo.ts (4)
src/scripts/app.ts (1)
app(1708-1708)src/composables/functional/useChainCallback.ts (1)
useChainCallback(24-35)src/extensions/core/widgetInputs.ts (1)
input(353-366)src/lib/litegraph/src/litegraph.ts (1)
LiteGraph(17-17)
⏰ 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: test
- GitHub Check: setup
- GitHub Check: collect
- GitHub Check: lint-and-format
🔇 Additional comments (3)
src/stores/nodeDefStore.ts (1)
222-237: LGTM! Node definition looks well-structured.The CustomCombo definition follows the pattern of other system nodes. Minor observation: the output type uses lowercase
'string'while the input type uses uppercase'STRING'. Verify this is intentional for LiteGraph compatibility.src/extensions/core/index.ts (1)
5-5: LGTM!Import follows the established pattern and alphabetical ordering.
src/extensions/core/customCombo.ts (1)
110-115: LGTM!Extension registration follows the established pattern with proper naming convention.
There was a problem hiding this comment.
Actionable comments posted: 6
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
src/extensions/core/customCombo.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (11)
**/*.{vue,ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{vue,ts,tsx}: Leverage VueUse functions for performance-enhancing utilities
Use vue-i18n in Composition API for any string literals and place new translation entries in src/locales/en/main.json
Files:
src/extensions/core/customCombo.ts
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursorrules)
Use es-toolkit for utility functions
Files:
src/extensions/core/customCombo.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
Use TypeScript for type safety
**/*.{ts,tsx}: Never useanytype - use proper TypeScript types
Never useas anytype assertions - fix the underlying type issue
Files:
src/extensions/core/customCombo.ts
**/*.{ts,tsx,js,vue}
📄 CodeRabbit inference engine (.cursorrules)
Implement proper error handling in components and services
**/*.{ts,tsx,js,vue}: Use 2-space indentation, single quotes, no semicolons, and maintain 80-character line width as configured in.prettierrc
Organize imports by sorting and grouping by plugin, and runpnpm formatbefore committing
Files:
src/extensions/core/customCombo.ts
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/extensions/core/customCombo.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/extensions/core/customCombo.ts
**/*.{ts,tsx,js,jsx,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
Use camelCase for variable and setting names in TypeScript/Vue files
Files:
src/extensions/core/customCombo.ts
**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx,vue}: Useconst settingStore = useSettingStore()andsettingStore.get('Comfy.SomeSetting')to retrieve settings in TypeScript/Vue files
Useawait settingStore.set('Comfy.SomeSetting', newValue)to update settings in TypeScript/Vue files
Check server capabilities usingapi.serverSupportsFeature('feature_name')before using enhanced features
Useapi.getServerFeature('config_name', defaultValue)to retrieve server feature configurationEnforce ESLint rules for Vue + TypeScript including: no floating promises, no unused imports, and i18n raw text restrictions in templates
Files:
src/extensions/core/customCombo.ts
**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.ts: Define dynamic setting defaults using runtime context with functions in settings configuration
UsedefaultsByInstallVersionproperty for gradual feature rollout based on version in settings configuration
Files:
src/extensions/core/customCombo.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/extensions/core/customCombo.ts
src/**/*.{vue,ts,tsx}
📄 CodeRabbit inference engine (src/CLAUDE.md)
Follow Vue 3 composition API style guide
Files:
src/extensions/core/customCombo.ts
🧬 Code graph analysis (1)
src/extensions/core/customCombo.ts (3)
src/extensions/core/widgetInputs.ts (2)
applyToGraph(43-87)input(353-366)src/scripts/app.ts (1)
app(1708-1708)src/composables/functional/useChainCallback.ts (1)
useChainCallback(24-35)
⏰ 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 (2)
src/extensions/core/customCombo.ts (2)
1-5: Imports are well-organized.The imports are properly typed and follow the project's import conventions.
104-113: Extension registration follows best practices.The extension properly:
- Uses optional chaining to safely check
nodeData?.name- Guards against incorrect node types
- Chains callbacks with
useChainCallbackto compose behavior- Follows the established extension pattern
| function applyToGraph(this: LGraphNode, extraLinks: LLink[] = []) { | ||
| if (!this.outputs[0].links?.length || !this.graph) return | ||
|
|
||
| const links = [ | ||
| ...this.outputs[0].links.map((l) => this.graph!.links[l]), | ||
| ...extraLinks | ||
| ] | ||
| let v = this.widgets?.[0].value | ||
| // For each output link copy our value over the original widget value | ||
| for (const linkInfo of links) { | ||
| const node = this.graph?.getNodeById(linkInfo.target_id) | ||
| const input = node?.inputs[linkInfo.target_slot] | ||
| if (!input) { | ||
| console.warn('Unable to resolve node or input for link', linkInfo) | ||
| continue | ||
| } | ||
|
|
||
| const widgetName = input.widget?.name | ||
| if (!widgetName) { | ||
| console.warn('Invalid widget or widget name', input.widget) | ||
| continue | ||
| } | ||
|
|
||
| const widget = node.widgets?.find((w) => w.name === widgetName) | ||
| if (!widget) { | ||
| console.warn(`Unable to find widget "${widgetName}" on node [${node.id}]`) | ||
| continue | ||
| } | ||
|
|
||
| widget.value = v | ||
| widget.callback?.( | ||
| widget.value, | ||
| app.canvas, | ||
| node, | ||
| app.canvas.graph_mouse, | ||
| {} as CanvasPointerEvent | ||
| ) | ||
| } | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Extract shared logic to reduce duplication.
This function duplicates significant logic from widgetInputs.ts (lines 42-86). The core link-traversal and widget-update pattern is identical.
Consider extracting the shared logic into a common utility function that both implementations can use, perhaps accepting a value-transformer function to handle the text-replacement difference.
Example approach:
// In a shared utility file
function propagateWidgetValue(
node: LGraphNode,
outputIndex: number,
value: any,
extraLinks: LLink[] = []
) {
if (!node.outputs[outputIndex].links?.length || !node.graph) return
const links = [
...node.outputs[outputIndex].links.map((l) => node.graph!.links[l]),
...extraLinks
]
for (const linkInfo of links) {
const targetNode = node.graph?.getNodeById(linkInfo.target_id)
const input = targetNode?.inputs[linkInfo.target_slot]
// ... rest of logic
}
}🤖 Prompt for AI Agents
In src/extensions/core/customCombo.ts around lines 7 to 45 the applyToGraph
implementation duplicates the link-traversal and widget-update logic found in
widgetInputs.ts; extract that shared behavior into a common utility (e.g.,
propagateWidgetValue) placed in a shared utils file, which accepts the node
(this), output index (0), the value to propagate, optional extraLinks, and an
optional value-transformer callback; move the common code that resolves graph
links, finds target node/input/widget, sets widget.value and invokes
widget.callback into that utility, keep all existing null-checks and warning
messages, and replace applyToGraph with a call to the new utility (passing a
transformer that returns v unchanged) and update widgetInputs.ts to call the
same utility with its own transformer for text replacement so both
implementations reuse the single function and preserve typings and behavior.
| ...this.outputs[0].links.map((l) => this.graph!.links[l]), | ||
| ...extraLinks | ||
| ] | ||
| let v = this.widgets?.[0].value |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Add defensive check for widgets array.
The code assumes this.widgets[0] exists without validation. While the node definition likely ensures this, adding a defensive check improves robustness.
- let v = this.widgets?.[0].value
+ const widget = this.widgets?.[0]
+ if (!widget) return
+ let v = widget.value📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| let v = this.widgets?.[0].value | |
| const widget = this.widgets?.[0] | |
| if (!widget) return | |
| let v = widget.value |
🤖 Prompt for AI Agents
In src/extensions/core/customCombo.ts around line 14, the code accesses
this.widgets[0].value without validating this.widgets or this.widgets[0]; add a
defensive check to ensure this.widgets is an array and has at least one element
before reading .value (e.g., guard with if (!Array.isArray(this.widgets) ||
this.widgets.length === 0) return or set v = undefined), and use the safe v
afterwards so you don't access .value on undefined.
src/extensions/core/customCombo.ts
Outdated
| const comboWidget = this.widgets![0] | ||
| Object.defineProperty(comboWidget.options, 'values', { | ||
| get: () => { | ||
| return this.widgets!.filter( | ||
| (w) => w.name.startsWith('option') && w.value | ||
| ).map((w) => w.value) | ||
| } | ||
| }) | ||
| const options = comboWidget.options as { values: string[] } |
There was a problem hiding this comment.
Validate widget existence and type.
The code uses non-null assertions (this.widgets![0]) assuming the combo widget exists and is first. If the node definition changes or widgets aren't initialized, this will throw at runtime.
Add validation:
+ if (!this.widgets || this.widgets.length === 0) {
+ console.error('CustomCombo node missing expected widgets')
+ return
+ }
const comboWidget = this.widgets![0]
+ // Optionally validate it's actually a combo widget
+ if (comboWidget.type !== 'combo') {
+ console.error('Expected first widget to be combo type')
+ return
+ }Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/extensions/core/customCombo.ts around lines 50 to 58, the code assumes
this.widgets and this.widgets[0] exist and are the combo widget; replace the
non-null assertions with explicit validation: check that this.widgets is an
array and has at least one element, find the combo widget by name/type (e.g.,
first widget whose name === 'combo' or matches expected signature) rather than
assuming index 0, verify the found widget has an options object before defining
the 'values' property, and if validation fails either throw a clear error or
return a safe fallback (empty options/values) so the code never dereferences
undefined.
| function updateCombo() { | ||
| if (app.configuringGraph) return | ||
| const { values } = options | ||
| if (values.includes(`${comboWidget.value}`)) return |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Clarify string coercion intent.
The comparison uses string template coercion ${comboWidget.value} rather than direct comparison. This might be intentional to handle type flexibility, but it's unclear.
If intentional, consider adding a comment:
- if (values.includes(`${comboWidget.value}`)) return
+ // Coerce to string to handle mixed types
+ if (values.includes(`${comboWidget.value}`)) returnOr use explicit conversion for clarity:
- if (values.includes(`${comboWidget.value}`)) return
+ if (values.includes(String(comboWidget.value))) return📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (values.includes(`${comboWidget.value}`)) return | |
| if (values.includes(String(comboWidget.value))) return |
🤖 Prompt for AI Agents
In src/extensions/core/customCombo.ts around line 63, the check uses template
string coercion if (values.includes(`${comboWidget.value}`)) which is unclear;
replace the template string with an explicit conversion (e.g.,
String(comboWidget.value)) so intent is obvious, or if the template coercion was
intentional add a one-line comment explaining why coercion via string is
required (type(s) expected and why) so future readers understand the comparison.
src/extensions/core/customCombo.ts
Outdated
| node.widgets | ||
| .findLast((w) => w.name.startsWith('option')) | ||
| ?.name?.slice(6) || '-1' | ||
| const newCount = parseInt(widgetPostfix) + 1 |
There was a problem hiding this comment.
Specify radix in parseInt.
The parseInt call should include a radix parameter to avoid potential parsing issues.
- const newCount = parseInt(widgetPostfix) + 1
+ const newCount = parseInt(widgetPostfix, 10) + 1📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const newCount = parseInt(widgetPostfix) + 1 | |
| const newCount = parseInt(widgetPostfix, 10) + 1 |
🤖 Prompt for AI Agents
In src/extensions/core/customCombo.ts around line 76, the parseInt call lacks a
radix; update the call to include the radix (e.g., parseInt(widgetPostfix, 10))
so the string is parsed as a base-10 integer, and if desired add a quick NaN
check after parsing to handle unexpected input.
| Object.defineProperty(widget, 'value', { | ||
| get() { | ||
| return value | ||
| }, | ||
| set(v) { | ||
| value = v | ||
| updateCombo() | ||
| if (!node.widgets) return | ||
| const lastWidget = node.widgets.at(-1) | ||
| if (lastWidget === this) { | ||
| if (v) addOption(node) | ||
| return | ||
| } | ||
| if (v || node.widgets.at(-2) !== this || lastWidget?.value) return | ||
| node.widgets.pop() | ||
| node.computeSize(node.size) | ||
| } | ||
| }) |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider simplifying the custom value setter logic.
The custom getter/setter has complex side effects including:
- Calling
updateCombo()on every value change - Auto-adding new widgets when the last one is filled
- Auto-removing widgets with complex conditions
- Mutating the
node.widgetsarray during property access
This complexity makes the behavior harder to reason about and test. Consider:
- Extracting the auto-add/remove logic into separate, well-named functions
- Adding comments explaining the removal condition (line 95)
- Ensuring widget array mutations are safe if iteration is happening elsewhere
Example structure:
function shouldAutoAddOption(widget, value, node) {
return node.widgets.at(-1) === widget && value
}
function shouldAutoRemoveOption(widget, value, node) {
const lastWidget = node.widgets.at(-1)
const secondToLast = node.widgets.at(-2)
// Remove this widget if:
// - It has no value
// - AND it's second-to-last
// - AND the last widget also has no value
return !value && secondToLast === widget && !lastWidget?.value
}
set(v) {
value = v
updateCombo()
if (!node.widgets) return
if (shouldAutoAddOption(this, v, node)) {
addOption(node)
} else if (shouldAutoRemoveOption(this, v, node)) {
node.widgets.pop()
node.computeSize(node.size)
}
}🤖 Prompt for AI Agents
In src/extensions/core/customCombo.ts around lines 82 to 99, the custom setter
for widget.value mixes multiple side-effects (updateCombo, auto-add,
auto-remove) and mutates node.widgets inline, making behavior hard to reason
about and unsafe during iteration; refactor by extracting the auto-add and
auto-remove checks into two well-named helper functions (e.g.,
shouldAutoAddOption(widget, value, node) and shouldAutoRemoveOption(widget,
value, node)), replace the inline conditionals with calls to those helpers, add
a concise comment explaining the removal condition (why we remove the
second-to-last empty widget when the last is also empty), and make the mutation
of node.widgets safe (use the widget index to remove via splice or defer
mutation to a safe tick/event instead of mutating during property access).
|
I think this doesn't work when using Nodes 2.0 |
|
Adds a frontend support for a "Custom Combo" node which contains a Combo Widget, and a growing list of string inputs that determine the options available to the combo.
By promoting the combo widget on this node, a list of options determined by workflow builders can be exposed to end users.
CC: @Kosinkadink
┆Issue is synchronized with this Notion page by Unito