Merged
Conversation
Fixes multiple TypeScript errors across the codebase to improve type safety: - Add TgpuRoot type import for maskEditorStore - Add Point type annotation for useCanvasDrop position array - Add proper type assertions for settings store retrieval - Add index signature to IWidgetOptions for Record compatibility - Add NumericWidgetOptions interface for widget numeric properties - Add type guards for widget option callbacks and properties - Add null checks for load3dService viewer access - Replace any types with proper unknown or specific types - Fix console.log usage in scripts to use allowed methods - Move template type assertions to computed properties
Contributor
📝 WalkthroughWalkthroughThis pull request refactors the codebase's type safety by systematically replacing permissive Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 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 |
🎭 Playwright Tests: ✅ PassedResults: 511 passed, 0 failed, 0 flaky, 8 skipped (Total: 519) 📊 Browser Reports
|
🎨 Storybook Build Status✅ Build completed successfully! ⏰ Completed at: 02/05/2026, 10:03:50 PM UTC 🔗 Links🎉 Your Storybook is ready for review! |
Bundle Size ReportSummary
Category Glance Per-category breakdownApp Entry Points — 22.5 kB (baseline 22.5 kB) • ⚪ 0 BMain entry bundles and manifests
Status: 1 added / 1 removed Graph Workspace — 840 kB (baseline 840 kB) • ⚪ 0 BGraph editor runtime, canvas, workflow orchestration
Status: 1 added / 1 removed Views & Navigation — 69 kB (baseline 69 kB) • ⚪ 0 BTop-level views, pages, and routed surfaces
Status: 9 added / 9 removed Panels & Settings — 410 kB (baseline 410 kB) • ⚪ 0 BConfiguration panels, inspectors, and settings screens
Status: 12 added / 12 removed User & Accounts — 16 kB (baseline 16 kB) • ⚪ 0 BAuthentication, profile, and account management bundles
Status: 5 added / 5 removed Editors & Dialogs — 3.47 kB (baseline 3.47 kB) • ⚪ 0 BModals, dialogs, drawers, and in-app editors
Status: 2 added / 2 removed UI Components — 37.8 kB (baseline 37.8 kB) • ⚪ 0 BReusable component library chunks
Status: 6 added / 6 removed Data & Services — 2.1 MB (baseline 2.1 MB) • 🔴 +279 BStores, services, APIs, and repositories
Status: 11 added / 11 removed Utilities & Hooks — 234 kB (baseline 234 kB) • ⚪ 0 BHelpers, composables, and utility bundles
Status: 14 added / 14 removed Vendor & Third-Party — 9.37 MB (baseline 9.37 MB) • ⚪ 0 BExternal libraries and shared vendor chunks
Other — 7.08 MB (baseline 7.08 MB) • 🔴 +738 BBundles that do not match a named category
Status: 59 added / 59 removed |
Myestery
requested review from
a team,
benceruleanlu,
christian-byrne,
jtydhr88,
ltdrdata and
viva-jinyi
as code owners
dosubot
bot
added
the
size:L
Myestery
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (6)
src/platform/workflow/management/stores/workflowStore.ts (1)
720-742: 🧹 Nitpick | 🔵 TrivialLGTM! The explicit return type annotation is correct.
All return paths align with
string | undefined:
- Line 726: returns
id(string) orundefined- Line 728: implicit
undefined- Line 740:
subgraphNodeIds.at(-1)returnsstring | undefined- End of function: implicit
undefined
Note: There's a pre-existing
FIXMEcomment at Line 720. Would you like assistance refactoring this to use an existing utility function, or should I open an issue to track this?src/renderer/extensions/vueNodes/widgets/components/WidgetSelectDropdown.vue (2)
76-83: 🧹 Nitpick | 🔵 TrivialType-check
nodeTypebefore invokinguseAssetWidgetData.The assertion can hide non-string values and push invalid data into the composable. A simple
typeofguard keeps the runtime safe.Proposed fix
- const nodeType = (props.widget.options?.nodeType ?? props.nodeType) as - | string - | undefined - if (props.isAssetMode && nodeType) { - return useAssetWidgetData(toRef(nodeType)) - } + const nodeType = props.widget.options?.nodeType ?? props.nodeType + if (props.isAssetMode && typeof nodeType === 'string') { + return useAssetWidgetData(toRef(() => nodeType)) + }
285-290: 🧹 Nitpick | 🔵 TrivialOnly return
placeholderwhen it’s a string.The assertion can let non-string values flow into the UI. Guarding keeps the placeholder stable.
Proposed fix
- if (options?.placeholder) { - return options.placeholder as string - } + if (typeof options?.placeholder === 'string') { + return options.placeholder + }src/platform/settings/settingStore.ts (1)
99-107:⚠️ Potential issue | 🟠 MajorUse deep equality comparison to detect unchanged object/array settings.
The
get()function returns a deep clone (line 123), so the reference equality check on line 107 will always fail for objects and arrays, even when values are unchanged. This triggers unnecessaryonChangecallbacks and API calls.Use
_.isEqual()to compare the migrated value against the original stored/default value instead:🔧 Suggested fix
- const oldValue = get(key) - if (newValue === oldValue) return + const storedValue = settingValues.value[key] as Settings[K] | undefined + const oldValueForCompare = storedValue ?? getDefaultValue(key) + if (_.isEqual(newValue, oldValueForCompare)) return + const oldValue = get(key)
_.isEqualis available ines-toolkit/compat(already imported on line 2).src/renderer/extensions/vueNodes/widgets/components/WidgetInputNumberInput.vue (2)
185-188:⚠️ Potential issue | 🟠 MajorLocalize the tooltip string with vue-i18n.
The tooltip is user-facing and must be added to
src/locales/en/main.jsonunder the widgets namespace, then referenced viat()in the component. Addtto theuseI18n()destructuring and update the return statement to uset('widgets.inputNumber.precisionLimitTooltip').Suggested i18n update
-const { n } = useI18n() +const { n, t } = useI18n() if (buttonsDisabled.value) { - return 'Increment/decrement disabled: value exceeds JavaScript precision limit (±2^53)' + return t('widgets.inputNumber.precisionLimitTooltip') }Per coding guidelines: Use vue-i18n for ALL user-facing strings, configured in
src/locales/en/main.json.
104-126:⚠️ Potential issue | 🔴 CriticalReplace
return 0with safe fallback to match canonicalgetWidgetStep()logic.Returning
0whenprecisionis undefined andstepis≤ 10causes silent no-op stepping while buttons remain enabled. This contradicts the established pattern insrc/lib/litegraph/src/utils/widget.tswhich always returns a minimum step of1. Widgets that setstepwithoutprecisionwill be broken.Recommended fix
if (precision.value !== undefined) { if (precision.value === 0) { return 1 } return Number((1 / Math.pow(10, precision.value)).toFixed(precision.value)) } - return 0 + if (step !== undefined) return Number(step) + return 1
🤖 Fix all issues with AI agents
In `@scripts/i18n-types.ts`:
- Line 5: The LocaleData type is too narrow (string | LocaleData) and conflicts
with the codebase's use of Record<string, unknown>; update the LocaleData alias
in scripts/i18n-types.ts so it uses the broader recursive shape (Record<string,
unknown>) to match src/i18n.ts and other loader patterns and avoid rejecting
non-string leaves—locate the LocaleData type definition and replace its current
union with Record<string, unknown>.
In `@src/lib/litegraph/src/widgets/ComboWidget.ts`:
- Around line 38-43: Remove the redundant type assertions on getOptionLabel:
instead of casting this.options.getOptionLabel to ((value?: string | null) =>
string) | undefined, assign it directly (const getOptionLabel =
this.options.getOptionLabel) and let the declared IWidgetOptions type provide
the signature; keep the existing null/undefined check and try/catch logic. Do
the same for the other occurrence around the later block (the code handling
option labels at the other location) so both uses rely on the IWidgetOptions
typing rather than explicit casts.
In
`@src/platform/workflow/persistence/composables/useWorkflowPersistence.test.ts`:
- Around line 192-195: Replace the unsafe cast to Record<string, unknown> for
the test variable drafts with the canonical draft type exported from the
persistence store: import the exported type (e.g., DraftRecord, DraftsMap, or
the appropriate name) from the module that defines the persistence shapes and
use that type in the JSON.parse cast for drafts; update the top-of-file imports
to include that type and change the cast on the const drafts declaration so
assertions in useWorkflowPersistence.test.ts are strongly typed and match
production shapes.
In `@src/renderer/extensions/vueNodes/widgets/components/WidgetSelectDefault.vue`:
- Around line 48-52: The default function for the defineModel called modelValue
blindly casts props.widget.options?.values to string[] and indexes it; change it
to first check Array.isArray(props.widget.options?.values) before accessing [0],
remove the unsafe cast, and return the first element if it's an array or
undefined otherwise (respecting the string | undefined type); update the
default(props: Props) implementation to use a guarded lookup on
widget.options.values.
In
`@src/renderer/extensions/vueNodes/widgets/components/WidgetSelectDropdown.vue`:
- Around line 58-62: The default() for defineModel's modelValue reads
props.widget.options?.values and indexes it directly, which can return a
character if values is a string; change the guard to check
Array.isArray(props.widget.options?.values) before indexing (use the array only
when true), otherwise return undefined or '' to match the model type; update the
default function and references to props.widget.options?.values to rely on the
Array.isArray check to ensure runtime safety.
- Around line 121-125: The code in getDisplayLabel casts
props.widget.options?.getOptionLabel to a function but doesn't verify it's
actually callable; replace the cast with a runtime check using typeof to ensure
props.widget.options?.getOptionLabel is a function before invoking it (e.g.,
check typeof props.widget.options?.getOptionLabel === 'function'), and fallback
to returning the raw value if not callable; update getDisplayLabel to call the
validated function reference (from props.widget.options?.getOptionLabel) only
when the typeof guard passes.
In `@src/renderer/extensions/vueNodes/widgets/composables/useRemoteWidget.ts`:
- Around line 131-134: The onFirstLoad handler currently sets widget.value to
data[0] when data is an array, which yields undefined for an empty array; update
onFirstLoad to treat empty arrays as a fallback by checking Array.isArray(data)
&& data.length > 0 and otherwise assigning a safe default (e.g., a provided
defaultValue or a new instance) before calling widget.callback; adjust
references in onFirstLoad, widget.value, isLoaded and widget.callback to ensure
widget.value is never set to undefined on first load.
In `@src/services/load3dService.ts`:
- Around line 188-190: The guard "if (!viewer) return" in handleViewerClose is
dead code because getOrCreateViewer always returns an instance; remove the
unreachable check or, if you want defensive behavior, call a new getViewer
method (which returns undefined) instead of getOrCreateViewer and keep the
guard; update handleViewerClose to use either getOrCreateViewer without the
if-statement or replace getOrCreateViewer with getViewer and retain the guard
accordingly.
In
`@src/workbench/extensions/manager/components/manager/skeleton/PackCardGridSkeleton.test.ts`:
- Around line 16-21: Replace the mountComponent arrow-function expression with a
named function declaration to follow repo guidance: change the const
mountComponent = ({ props = {} }: { props?: Partial<ComponentProps<typeof
GridSkeleton>> } = {}): VueWrapper => { ... } into a function mountComponent({
props = {} }: { props?: Partial<ComponentProps<typeof GridSkeleton>> } = {}):
VueWrapper { ... } and keep the same body (including i18n creation),
exports/usages, and types intact so all existing references to mountComponent
continue to work.
src/platform/workflow/persistence/composables/useWorkflowPersistence.test.ts
Show resolved
Hide resolved
src/renderer/extensions/vueNodes/widgets/components/WidgetSelectDefault.vue
Show resolved
Hide resolved
src/renderer/extensions/vueNodes/widgets/components/WidgetSelectDropdown.vue
Show resolved
Hide resolved
src/renderer/extensions/vueNodes/widgets/components/WidgetSelectDropdown.vue
Show resolved
Hide resolved
src/workbench/extensions/manager/components/manager/skeleton/PackCardGridSkeleton.test.ts
Outdated
Show resolved
Hide resolved
AustinMroz
reviewed
Feb 3, 2026
Collaborator
AustinMroz
left a comment
AustinMroz
left a comment
There was a problem hiding this comment.
Going to need to invest more time in this one later.
The simplifiedWidget typing needs some more love. I'd rather we didn't have to rewrite the widgets because strongly typed options get lost in translation. At a minimum, I'd like the generic typing to be updated to
O extends IWidgetOptions = IWidgetOptions
But ideally, there'd be a way to transfer the already strongly typed litegraph options 🤔
… components Replace toRaw() prop mutations with proper Vue emit events pattern: - TabSubgraphInputs now emits update:proxyWidgets event - WidgetItem now emits update:widgetValue event - Parent components handle events and perform mutations - Follows Vue's one-way data flow principle This change removes the vue/no-mutating-props lint violations without using toRaw() to bypass Vue's reactivity system. While this improves code maintainability and follows Vue best practices, it does not address the underlying unified state management issues with widget values across the application.
- settingStore: change settingValues to Partial<Settings>, simplify get() with non-null assertion - litegraph: remove vestigial GraphOrSubgraph export, use LGraph directly in tests - widgets: replace type assertions with runtime guards (typeof, Array.isArray) - useRemoteWidget: handle empty arrays with fallback to defaultValue - load3dService: remove unreachable null check - tests: use real types (WorkflowDraftSnapshot) instead of Record<string, unknown> - tests: convert arrow function to function declaration per guidelines
- Change SimplifiedWidget generic from Record<string, unknown> to IWidgetOptions - Remove index signature from IWidgetOptions (was causing all props to become unknown) - Add missing Vue widget properties to IWidgetOptions (disabled, useGrouping, placeholder, etc.) - Update filterWidgetProps to accept object instead of Record<string, unknown> This provides proper type safety for widget options without needing runtime guards or type assertions.
- WidgetSelectDefault: remove Array.isArray check, use optional chaining - WidgetSelectDropdown: remove typeof check for getOptionLabel - ComboWidget: simplify getOptionLabel usage TypeScript now properly infers types from IWidgetOptions, eliminating the need for runtime type guards.
- WidgetChart: extend chart options with IWidgetOptions - WidgetColorPicker: change from Record<string, unknown> to IWidgetOptions - NodeWidgets: add type assertion for widgetOptions to satisfy SimplifiedWidget Ensures all widget components properly extend IWidgetOptions for type safety.
- WidgetInputText.test: cast options to IWidgetOptions - WidgetSelectDropdown.test: update getOptionLabel signature to match IWidgetOptions Aligns test mocks with updated SimplifiedWidget type signature.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/platform/workflow/persistence/composables/useWorkflowPersistence.test.ts (1)
166-168: 🧹 Nitpick | 🔵 TrivialInconsistent typing: Use
WorkflowDraftSnapshothere as well.This test parses the same
Comfy.Workflow.DraftslocalStorage key as the test on line 193-195, but uses an inline type instead ofWorkflowDraftSnapshot. For consistency and to prevent type drift, use the same canonical type.♻️ Suggested fix
const drafts = JSON.parse( localStorage.getItem('Comfy.Workflow.Drafts') ?? '{}' - ) as Record<string, { data: string; isTemporary: boolean }> + ) as Record<string, WorkflowDraftSnapshot>Based on learnings: "In TypeScript test files (e.g., any test under src), avoid duplicating interface/type definitions. Import real type definitions from the component modules under test and reference them directly, so there is a single source of truth and to prevent type drift."
src/components/rightSidePanel/parameters/TabSubgraphInputs.vue (1)
188-190: 🧹 Nitpick | 🔵 TrivialMinor: Redundant
setDirtycall.The parent handler (
handleProxyWidgetsUpdatein RightSidePanel.vue) already callssetDirty(true, true)after handling the emitted event. This call on line 189 results in double-marking the canvas dirty. While harmless, consider removing it for consistency with the event-driven pattern where the parent owns all side effects.♻️ Optional: Remove redundant setDirty call
proxyWidgets.value = pw - canvasStore.canvas?.setDirty(true, true) triggerRef(proxyWidgets)src/renderer/extensions/vueNodes/widgets/components/WidgetInputText.test.ts (1)
14-24: 🧹 Nitpick | 🔵 TrivialAlign parameter type with IWidgetOptions for consistency.
The helper accepts
Partial<InputTextProps>but casts toIWidgetOptions. For better type safety and to prevent type drift, consider updating the parameter type toPartial<IWidgetOptions>.Proposed fix
const createMockWidget = ( value: string = 'default', - options: Partial<InputTextProps> = {}, + options: Partial<IWidgetOptions> = {}, callback?: (value: string) => void ): SimplifiedWidget<string> => ({ name: 'test_input', type: 'string', value, options: options as IWidgetOptions, callback })Based on learnings: In test files, import real type definitions from the component modules under test instead of using unrelated types to prevent type drift.
🤖 Fix all issues with AI agents
In `@src/platform/settings/settingStore.ts`:
- Around line 119-121: The get function uses a non-null assertion on
getDefaultValue(key) which can be undefined; update get to return Settings[K] |
undefined (change signature function get<K extends keyof Settings>(key: K):
Settings[K] | undefined) and return _.cloneDeep(settingValues.value[key] ??
getDefaultValue(key)); alternatively, if you want to guarantee a value, call
getDefaultValue and throw a descriptive error when it returns undefined
(referencing get and getDefaultValue to locate the code) so you avoid the `!`
operator and ensure type safety.
In `@src/renderer/extensions/vueNodes/components/NodeWidgets.vue`:
- Around line 175-177: The current inline cast of widgetOptions masks typing —
instead of asserting to IWidgetOptions | undefined, declare widgetOptions with
an explicit type and construct it accordingly: use the existing symbols
(widgetOptions, slotMetadata, options, IWidgetOptions) and build widgetOptions
as either undefined or an IWidgetOptions object (e.g., when slotMetadata?.linked
return a new object merging options with disabled: true, otherwise return
options) so the compiler infers correct types without the `as` assertion.
In `@src/renderer/extensions/vueNodes/widgets/composables/useRemoteWidget.ts`:
- Around line 143-153: The initialization check treats falsy cached values as
uninitialized because isInitialized uses a truthiness test on entry?.data;
update isInitialized to explicitly check entry?.data !== undefined (not
Boolean(entry?.data)) so valid falsy values like 0, false, or '' are considered
initialized, and then leave useRemoteWidget's logic (the checks around
isInitialized(entry), isFailed(entry), isBackingOff(entry), isFetching(entry),
and the CacheEntry/dataCache handling) unchanged so onFirstLoad and fetch gating
work correctly.
Comment on lines
119
to
+121
| function get<K extends keyof Settings>(key: K): Settings[K] { | ||
| // Clone the value when returning to prevent external mutations | ||
| return _.cloneDeep(settingValues.value[key] ?? getDefaultValue(key)) | ||
| return _.cloneDeep(settingValues.value[key] ?? getDefaultValue(key)!) |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cd /tmp && find . -name "settingStore.ts" -type f 2>/dev/null | head -5Repository: Comfy-Org/ComfyUI_frontend
Length of output: 52
🏁 Script executed:
git ls-files | grep -i settingstoreRepository: Comfy-Org/ComfyUI_frontend
Length of output: 150
🏁 Script executed:
fd "settingStore" --type fRepository: Comfy-Org/ComfyUI_frontend
Length of output: 150
🏁 Script executed:
fd "settingStore.ts"Repository: Comfy-Org/ComfyUI_frontend
Length of output: 107
🏁 Script executed:
wc -l src/platform/settings/settingStore.tsRepository: Comfy-Org/ComfyUI_frontend
Length of output: 111
🏁 Script executed:
cat -n src/platform/settings/settingStore.tsRepository: Comfy-Org/ComfyUI_frontend
Length of output: 10462
🏁 Script executed:
cat -n src/platform/settings/settingStore.test.tsRepository: Comfy-Org/ComfyUI_frontend
Length of output: 21597
🏁 Script executed:
rg "\.get\(" src --type ts --type tsx -B 2 -A 2 | head -100Repository: Comfy-Org/ComfyUI_frontend
Length of output: 97
🏁 Script executed:
rg "\.get\(" src --type ts --type js -B 2 -A 2 | head -150Repository: Comfy-Org/ComfyUI_frontend
Length of output: 10471
Avoid the non-null assertion on default lookup.
The signature promises Settings[K], but getDefaultValue() explicitly returns Settings[K] | undefined (see line 137–139). If a setting lacks a registered default, getDefaultValue(key) returns undefined, and the ! hides this type violation. Either widen the return type to Settings[K] | undefined or ensure all settings have registered defaults enforced at the type level.
✅ Suggested fix (type-safe fallback)
- function get<K extends keyof Settings>(key: K): Settings[K] {
- // Clone the value when returning to prevent external mutations
- return _.cloneDeep(settingValues.value[key] ?? getDefaultValue(key)!)
- }
+ function get<K extends keyof Settings>(key: K): Settings[K] | undefined {
+ // Clone the value when returning to prevent external mutations
+ const value = settingValues.value[key]
+ if (value !== undefined) return _.cloneDeep(value)
+ const defaultValue = getDefaultValue(key)
+ return defaultValue === undefined ? undefined : _.cloneDeep(defaultValue)
+ }🤖 Prompt for AI Agents
In `@src/platform/settings/settingStore.ts` around lines 119 - 121, The get
function uses a non-null assertion on getDefaultValue(key) which can be
undefined; update get to return Settings[K] | undefined (change signature
function get<K extends keyof Settings>(key: K): Settings[K] | undefined) and
return _.cloneDeep(settingValues.value[key] ?? getDefaultValue(key));
alternatively, if you want to guarantee a value, call getDefaultValue and throw
a descriptive error when it returns undefined (referencing get and
getDefaultValue to locate the code) so you avoid the `!` operator and ensure
type safety.
…iles # Conflicts: # browser_tests/fixtures/ComfyPage.ts # browser_tests/fixtures/ws.ts # browser_tests/tests/actionbar.spec.ts # src/services/load3dService.ts
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@browser_tests/fixtures/ws.ts`:
- Around line 3-5: Replace the unsafe non-null assertion when pulling a captured
WebSocket from the registry (the use of window.__ws__![url]) with explicit
null/undefined checks: first cast to TestWindow and read the registry into a
local variable (e.g., const registry = (window as TestWindow).__ws__), throw a
clear error if registry is undefined, then lookup the socket by URL (e.g., const
ws = registry[url]) and throw another clear error if ws is undefined before
returning/using it; this uses optional chaining/explicit validation rather than
! to prevent flaky failures.
In `@src/services/load3dService.ts`:
- Line 84: The Map typing uses typeof useLoad3dViewer which fails because
useLoad3dViewer is dynamically imported; change the Map value type to the
existing alias UseLoad3dViewerFn instead. Update the declaration of
viewerInstances (currently const viewerInstances = new Map<NodeId,
ReturnType<typeof useLoad3dViewer>>()) to use ReturnType<UseLoad3dViewerFn> or
directly UseLoad3dViewerFn as appropriate so the type no longer references the
out-of-scope useLoad3dViewer symbol.
🧹 Nitpick comments (1)
browser_tests/fixtures/ws.ts (1)
8-8: Validateunknownpayloads before JSON.stringify.With
unknown, non-serializable values (e.g., BigInt or circular objects) will throw insideJSON.stringify, producing a cryptic failure. Consider guarding serialization and surfacing a clearer error.♻️ Suggested hardening
async trigger(data, url) { + let payload = '' + try { + payload = JSON.stringify(data) + } catch (error) { + throw new Error( + 'webSocketFixture.trigger expects JSON-serializable data', + { cause: error } + ) + } // Trigger a websocket event on the page await page.evaluate( function ([data, url]) { if (!url) { // If no URL specified, use page URL const u = new URL(window.location.toString()) u.protocol = 'ws:' u.pathname = '/' url = u.toString() + 'ws' } const ws: WebSocket = window.__ws__![url] ws.dispatchEvent( new MessageEvent('message', { data }) ) }, - [JSON.stringify(data), url] + [payload, url] ) }Also applies to: 30-49
- Fix LGraphCanvas to use _visibleReroutes instead of #visibleReroutes - Fix load3dService viewer type to use ReturnType<UseLoad3dViewerFn> - Make getOrCreateViewerSync generic to preserve actual return type - Remove index signature from UseLoad3dViewerFn type
AustinMroz
reviewed
Feb 5, 2026
| const widgetOptions = slotMetadata?.linked | ||
| ? { ...options, disabled: true } | ||
| : options | ||
| const widgetOptions = ( |
Collaborator
There was a problem hiding this comment.
I'd remove the cast here, remove the generic in useGraphNodeManager, and fix LGraphNodePreview instead
diff
diff --git a/src/composables/graph/useGraphNodeManager.ts b/src/composables/graph/useGraphNodeManager.ts
index 33822d1c5..7fec92440 100644
--- a/src/composables/graph/useGraphNodeManager.ts
+++ b/src/composables/graph/useGraphNodeManager.ts
@@ -52,7 +52,7 @@ export interface SafeWidgetData {
isDOMWidget?: boolean
label?: string
nodeType?: string
- options?: IWidgetOptions<unknown>
+ options?: IWidgetOptions
spec?: InputSpec
slotMetadata?: WidgetSlotMetadata
}
diff --git a/src/renderer/extensions/vueNodes/components/LGraphNodePreview.vue b/src/renderer/extensions/vueNodes/components/LGraphNodePreview.vue
index da7423850..34ee83543 100644
--- a/src/renderer/extensions/vueNodes/components/LGraphNodePreview.vue
+++ b/src/renderer/extensions/vueNodes/components/LGraphNodePreview.vue
@@ -37,6 +37,7 @@ import type {
INodeOutputSlot
} from '@/lib/litegraph/src/interfaces'
import { RenderShape } from '@/lib/litegraph/src/litegraph'
+import type { IWidgetOptions } from '@/lib/litegraph/src/types/widgets'
import NodeHeader from '@/renderer/extensions/vueNodes/components/NodeHeader.vue'
import NodeSlots from '@/renderer/extensions/vueNodes/components/NodeSlots.vue'
import NodeWidgets from '@/renderer/extensions/vueNodes/components/NodeWidgets.vue'
@@ -69,8 +70,11 @@ const nodeData = computed<VueNodeData>(() => {
options: {
hidden: input.hidden,
advanced: input.advanced,
- values: input.type === 'COMBO' ? input.options : undefined // For combo widgets
- }
+ values:
+ input.type === 'COMBO' && Array.isArray(input.options)
+ ? input.options
+ : undefined
+ } satisfies IWidgetOptions
}))
const inputs: INodeInputSlot[] = Object.entries(nodeDef.inputs || {})
diff --git a/src/renderer/extensions/vueNodes/components/NodeWidgets.vue b/src/renderer/extensions/vueNodes/components/NodeWidgets.vue
index c44419b71..37da74490 100644
--- a/src/renderer/extensions/vueNodes/components/NodeWidgets.vue
+++ b/src/renderer/extensions/vueNodes/components/NodeWidgets.vue
@@ -79,7 +79,6 @@ import type {
VueNodeData,
WidgetSlotMetadata
} from '@/composables/graph/useGraphNodeManager'
-import type { IWidgetOptions } from '@/lib/litegraph/src/types/widgets'
import { useSettingStore } from '@/platform/settings/settingStore'
import { useErrorHandling } from '@/composables/useErrorHandling'
import { st } from '@/i18n'
@@ -172,9 +171,9 @@ const processedWidgets = computed((): ProcessedWidget[] => {
// Core feature: Disable Vue widgets when their input slots are connected
// This prevents conflicting input sources - when a slot is linked to another
// node's output, the widget should be read-only to avoid data conflicts
- const widgetOptions = (
- slotMetadata?.linked ? { ...options, disabled: true } : options
- ) as IWidgetOptions | undefined
+ const widgetOptions = slotMetadata?.linked
+ ? { ...options, disabled: true }
+ : options
const simplified: SimplifiedWidget = {
name: widget.name,
Comment on lines
76
to
78
| const nodeType = (props.widget.options?.nodeType ?? props.nodeType) as | ||
| | string | ||
| | undefined |
Collaborator
There was a problem hiding this comment.
Suggested change
| const nodeType = (props.widget.options?.nodeType ?? props.nodeType) as | |
| | string | |
| | undefined | |
| const nodeType: string | undefined = | |
| props.widget.options?.nodeType ?? props.nodeType |
| } | ||
|
|
||
| return values.map((value: string, index: number) => ({ | ||
| return (values as string[]).map((value, index) => ({ |
Collaborator
There was a problem hiding this comment.
return values.map((value, index) => ({
id: `input-${index}`,
preview_url: getMediaUrl(String(value), 'input'),
name: String(value),
label: getDisplayLabel(String(value))
}))|
|
||
| if (options?.placeholder) { | ||
| return options.placeholder | ||
| return options.placeholder as string |
Collaborator
There was a problem hiding this comment.
Suggested change
| return options.placeholder as string | |
| return options.placeholder |
Comment on lines
62
to
64
| const isReadOnly = computed(() => { | ||
| return (widget.options?.read_only as boolean | undefined) ?? false | ||
| }) |
Collaborator
There was a problem hiding this comment.
Suggested change
| const isReadOnly = computed(() => { | |
| return (widget.options?.read_only as boolean | undefined) ?? false | |
| }) | |
| const isReadOnly = computed(() => widget.options?.read_only ?? false) |
src/services/litegraphService.ts
Outdated
| function addNodeOnGraph( | ||
| nodeDef: ComfyNodeDefV1 | ComfyNodeDefV2, | ||
| options: Record<string, any> = {} | ||
| options?: { pos?: Point } |
Collaborator
There was a problem hiding this comment.
Options can be used to pass arbitrary values to the createNode call, so filtering options other than pos is a regression
function addNodeOnGraph(
nodeDef: ComfyNodeDefV1 | ComfyNodeDefV2,
options: Record<string, unknown> & { pos?: Point } = {}
): LGraphNode {
options.pos ??= getCanvasCenter()
if (nodeDef.name.startsWith(useSubgraphStore().typePrefix)) {
const canvas = canvasStore.getCanvas()
const bp = useSubgraphStore().getBlueprint(nodeDef.name)
const items: object = {
nodes: bp.nodes,
subgraphs: bp.definitions?.subgraphs
}
const results = canvas._deserializeItems(items, {
position: options.pos
})
if (!results) throw new Error('Failed to add subgraph blueprint')
const node = results.nodes.values().next().value
if (!node)
throw new Error(
'Subgraph blueprint was added, but failed to resolve a subgraph Node'
)
return node
}
const node = LiteGraph.createNode(
nodeDef.name,
nodeDef.display_name,
options
)EDIT: Or maybe IWidgetOptions? forgot to test this.
src/stores/nodeDefStore.ts
Outdated
| readonly inputs: Record<string, InputSpecV2> | ||
| readonly outputs: OutputSpecV2[] | ||
| readonly hidden?: Record<string, any> | ||
| readonly hidden?: Record<string, unknown> |
Collaborator
There was a problem hiding this comment.
Suggested change
| readonly hidden?: Record<string, unknown> | |
| readonly hidden?: Record<string, boolean> |
Comment on lines
83
to
85
| const isDisabled = computed(() => { | ||
| return (props.widget.options?.disabled as boolean | undefined) ?? false | ||
| }) |
Collaborator
There was a problem hiding this comment.
Suggested change
| const isDisabled = computed(() => { | |
| return (props.widget.options?.disabled as boolean | undefined) ?? false | |
| }) | |
| const isDisabled = computed(() => props.widget.options?.disabled ?? false) |
Patch version increment to 1.39.6 **Base branch:** `main` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Chores** * Version bumped to 1.39.6. * **New Features** * Added multilingual UI strings for secrets management, asset import errors, app-mode prompts, and HitPaw tools. * New node types for image/video/audio workflows and expanded export formats (GLB/FBX/OBJ) for 3D/model outputs. * **Bug Fixes / Removals** * Removed the "Open 3D Viewer (Beta) for Selected Node" menu entry and related 3D viewer settings. * Added setting to enable automatic node replacement when mappings exist. <!-- end of auto-generated comment: release notes by coderabbit.ai --> ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-8595-1-39-6-2fd6d73d3650818cba9cffca313909e8) by [Unito](https://www.unito.io) --------- Co-authored-by: christian-byrne <72887196+christian-byrne@users.noreply.github.com> Co-authored-by: github-actions <github-actions@github.com>
…arison Switch from reference-based to content-based duplicate detection in contextMenuCompat. This properly identifies and filters out duplicate menu items even when they are different objects with the same content, which commonly occurs with extension patches. - Compare menu items by their 'content' property instead of object reference - Create Sets of content strings for efficient duplicate detection - Improve removal detection to accurately count removed items Fixes issues where context menu items were duplicated by some extensions.
- Remove unnecessary type casts in widget components - Use IWidgetOptions consistently instead of Record<string, any> - Add Array.isArray guard and satisfies in LGraphNodePreview - Restore extension pass-through in addNodeOnGraph options - Narrow nodeDefStore.hidden to Record<string, boolean> - Fix falsy check in useRemoteWidget isInitialized
dosubot
bot
added
size:XXL
Contributor
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (9)
src/composables/graph/useGraphNodeManager.ts (1)
162-174:⚠️ Potential issue | 🟡 MinorGuard
widget.optionsbefore asserting toIWidgetOptions.
widget.options as IWidgetOptionscan pass throughnull/primitive values from extensions, which then get treated as an object downstream and can throw. Safer to only assign when it’s an object and otherwise leaveoptionsundefined.🛡️ Suggested guard
return { value: useReactiveWidgetValue(widget), controlWidget: getControlWidget(widget), spec: nodeDefStore.getInputSpecForWidget(node, widget.name), nodeType: getNodeType(node, widget), borderStyle: widget.promoted ? 'ring ring-component-node-widget-promoted' : widget.advanced ? 'ring ring-component-node-widget-advanced' : undefined, label: widget.label, - options: widget.options as IWidgetOptions + options: + widget.options && typeof widget.options === 'object' + ? (widget.options as IWidgetOptions) + : undefined }src/renderer/extensions/vueNodes/components/LGraphNodePreview.vue (1)
59-77:⚠️ Potential issue | 🟡 MinorUse the resolved widget type when populating combo values.
typeis derived frominput.widgetType || input.type, butvalues(and the combo default) check onlyinput.type. If a widget overrideswidgetType(common for COMBO), the preview will omit options.✅ Align with resolved widget type
- const widgets = Object.entries(nodeDef.inputs || {}) + const widgets = Object.entries(nodeDef.inputs || {}) .filter(([_, input]) => widgetStore.inputIsWidget(input)) - .map(([name, input]) => ({ - name, - type: input.widgetType || input.type, - value: - input.default !== undefined - ? input.default - : input.type === 'COMBO' && - Array.isArray(input.options) && - input.options.length > 0 - ? input.options[0] - : '', - options: { - hidden: input.hidden, - advanced: input.advanced, - values: - input.type === 'COMBO' && Array.isArray(input.options) - ? input.options - : undefined - } satisfies IWidgetOptions - })) + .map(([name, input]) => { + const widgetType = input.widgetType || input.type + const comboValues = + widgetType === 'COMBO' && Array.isArray(input.options) + ? input.options + : undefined + + return { + name, + type: widgetType, + value: + input.default !== undefined + ? input.default + : comboValues && comboValues.length > 0 + ? comboValues[0] + : '', + options: { + hidden: input.hidden, + advanced: input.advanced, + values: comboValues + } satisfies IWidgetOptions + } + })src/locales/fa/nodeDefs.json (1)
6430-6447:⚠️ Potential issue | 🟡 MinorFix missing
namekeys for Load3D button labels.Line 6431, Line 6443, and Line 6447 use an empty-string key, which won’t match the
nameschema used elsewhere in this file.🛠️ Proposed fix
@@ "clear": { - "": "پاکسازی" + "name": "پاکسازی" }, @@ "upload 3d model": { - "": "بارگذاری مدل سهبعدی" + "name": "بارگذاری مدل سهبعدی" }, "upload extra resources": { - "": "بارگذاری منابع اضافی" + "name": "بارگذاری منابع اضافی" },src/locales/pt-BR/main.json (1)
16-2051:⚠️ Potential issue | 🟡 MinorAvoid manual edits to generated locale files (pt-BR).
These non‑English locale files are auto‑generated in this repo and manual edits will be overwritten and create churn. Please revert the pt‑BR changes and update only
src/locales/en/main.json, then regenerate locales.
Based on learnings: “locale files other thansrc/locales/en/main.jsonare generated automatically on every release… manual updates to other locale files … are not required.”src/locales/tr/main.json (1)
16-2051:⚠️ Potential issue | 🟡 MinorAvoid manual edits to generated locale files (tr).
These non‑English locale files are auto‑generated in this repo and manual edits will be overwritten and create churn. Please revert the tr changes and update only
src/locales/en/main.json, then regenerate locales.
Based on learnings: “locale files other thansrc/locales/en/main.jsonare generated automatically on every release… manual updates to other locale files … are not required.”src/locales/fr/nodeDefs.json (1)
6421-6438:⚠️ Potential issue | 🟡 MinorFix missing
namekeys for new Load3D inputs.These entries use an empty-string key, so the label lookup will likely fail.
🛠️ Proposed fix
"clear": { - "": "effacer" + "name": "effacer" }, @@ "upload 3d model": { - "": "téléverser un modèle 3D" + "name": "téléverser un modèle 3D" }, "upload extra resources": { - "": "téléverser des ressources supplémentaires" + "name": "téléverser des ressources supplémentaires" },src/locales/ja/nodeDefs.json (1)
6421-6438:⚠️ Potential issue | 🟡 MinorFix missing label keys for Load3D inputs.
Using an empty-string key will likely drop the label in the UI; these should be
"name"like the other inputs.🛠️ Suggested fix
"clear": { - "": "クリア" + "name": "クリア" }, ... "upload 3d model": { - "": "3Dモデルをアップロード" + "name": "3Dモデルをアップロード" }, "upload extra resources": { - "": "追加リソースをアップロード" + "name": "追加リソースをアップロード" },src/locales/es/nodeDefs.json (1)
6421-6438:⚠️ Potential issue | 🟡 MinorFix missing
namekeys for Load3D labels.These entries use an empty-string key, so the UI label likely won’t render. Replace with
"name"for consistency.✅ Suggested fix
"clear": { - "": "limpiar" + "name": "limpiar" }, @@ "upload 3d model": { - "": "subir modelo 3D" + "name": "subir modelo 3D" }, "upload extra resources": { - "": "subir recursos adicionales" + "name": "subir recursos adicionales" },src/locales/pt-BR/nodeDefs.json (1)
6428-6447:⚠️ Potential issue | 🟡 MinorFix missing
namekeys for new Load3D inputs.
Theclear,upload 3d model, andupload extra resourcesentries use an empty-string key, so the UI label won’t resolve. Usenamelike the other inputs.🔧 Proposed fix
"clear": { - "": "limpar" + "name": "limpar" }, @@ "upload 3d model": { - "": "enviar modelo 3D" + "name": "enviar modelo 3D" }, "upload extra resources": { - "": "enviar recursos extras" + "name": "enviar recursos extras" },
🤖 Fix all issues with AI agents
In `@src/lib/litegraph/src/contextMenuCompat.ts`:
- Around line 160-185: The set-diff logic for context menu items incorrectly
treats undefined content as a single key, so update the filters used when
building originalContents and patchedContents and when computing addedItems to
only include entries whose item.content is a stable key (e.g., typeof
item.content === 'string') rather than any undefined; specifically change the
predicates around originalItems, patchedItems, originalContents,
patchedContents, and addedItems (and ensure removedCount calculation uses the
filtered patchedContents) so separators or items with undefined content are not
collapsed into one set entry.
In `@src/locales/en/nodeDefs.json`:
- Around line 2212-2227: The batch_size tooltip for the node
"EmptyAceStep1_5LatentAudio" is incorrect (mentions latent images); update the
"inputs"."batch_size"."tooltip" text to reference audio instead — e.g., change
it to "The number of latent audio items in the batch." so the UI correctly
describes latent audio batching for the EmptyAceStep1_5LatentAudio node.
In `@src/locales/ko/nodeDefs.json`:
- Around line 7320-7331: Several multi-format export nodes (e.g.,
MeshyAnimateModelNode, MeshyImageToModelNode and other listed nodes) in the
Korean locale have array-based "outputs" where index entries were replaced with
nulls; restore those entries to match the English locale by replacing nulls with
the original object labels (use object keys "0","1","2", etc. as in the English
version) so that index 0 becomes "model_file", index 1 becomes "meshy_task_id"
(and index 2 "GLB"/"FBX" tooltips where applicable); update each affected node's
"outputs" array to mirror the English keys and values for the nodes named in the
comment to ensure UI labels attach to the correct outputs.
In `@src/locales/ru/nodeDefs.json`:
- Around line 11433-11449: Translate the English labels in the
ReferenceTimbreAudio node for the RU locale: update "display_name":
"ReferenceTimbreAudio" to a Russian display name (e.g., "ЭталонныйТембрАудио"),
translate input names for "conditioning" and "latent" to Russian equivalents,
and provide a Russian tooltip for the output "0". Locate the JSON object named
ReferenceTimbreAudio and replace the values of "display_name", inputs.*.name,
and outputs."0".tooltip with appropriate Russian strings while preserving the
JSON structure and keys.
- Around line 7316-7326: The RU locale's nodeDefs.json "outputs" entries are
using an array form and are missing many keys that the EN canonical uses; update
each 3D node (e.g., Meshy, Rodin3D, Tripo and the other affected nodes) so their
"outputs" match the EN structure exactly: convert the array to an
object/dictionary mapping output keys to objects (e.g., include "model_file",
"meshy_task_id", "3d_model_path"/"3D Model Path", "<node>_task_id", etc. as
present in EN), ensure each output object has the same "name" and "tooltip"
fields as EN, and mirror the exact key names and values used in the EN canonical
to avoid mislabeling.
In `@src/locales/zh-TW/nodeDefs.json`:
- Around line 6431-6436: The locale entries for the Load3D upload labels
("upload 3d model" and "upload extra resources") use a "zh-TW" key instead of
the schema's expected "name" field, so update those objects to replace the
"zh-TW" property with "name" (keeping the same Chinese strings) so the labels
conform to the locale schema and will be rendered; locate the entries by the
keys "upload 3d model" and "upload extra resources" in nodeDefs.json and change
their inner property from "zh-TW" to "name".
In `@src/locales/zh/main.json`:
- Around line 607-613: Remove unvalidated 3D formats from the code and
localization: delete .gltf, .glb, and .fbx entries from the SUPPORTED_EXTENSIONS
constant (symbol: SUPPORTED_EXTENSIONS in load3d/constants.ts), and remove the
localization keys FILE_3D_FBX, FILE_3D_GLB, FILE_3D_GLTF and FILE_3D_USDZ from
the locale JSON (symbol keys in main.json); then update the unsupportedFileType
message (symbol: unsupportedFileType) to list only the validated formats (.obj,
.stl, .ply, .spz) so UI/error text matches the actual supported set.
In `@src/locales/zh/nodeDefs.json`:
- Around line 2210-2226: The display_name for the node key
"EmptyAceStep1_5LatentAudio" is mixed English/Chinese; update its display_name
to a fully localized Chinese string (e.g., "空 Ace 步骤 1.5 潜在音频" or whatever
matches your existing AceStep naming convention) and apply the same consistent
localization to the other AceStep node display_name entries referenced (the
other AceStep entries in the diff ranges), ensuring only the display_name fields
are changed and input/output keys remain intact.
🧹 Nitpick comments (3)
src/locales/en/nodeDefs.json (1)
7335-7345: Optional: add format-specific tooltips for new 3D outputs.The newly added GLB/FBX/OBJ sockets all have null tooltips. Adding a short tooltip (e.g., “GLB model file”) would make the UI more self-describing.
✍️ Example update (apply similarly to other GLB/FBX/OBJ outputs)
"1": { "name": "GLB", - "tooltip": null + "tooltip": "GLB model file" }, "2": { "name": "FBX", - "tooltip": null + "tooltip": "FBX model file" }Also applies to: 7394-7409, 7457-7472, 7499-7514, 7533-7548, 7590-7605, 7634-7649, 11836-11843, 11867-11873, 11895-11902, 11917-11923, 11945-11951, 13947-13957, 13990-14000, 14622-14633, 14687-14698, 14711-14722, 14736-14747, 14758-14769, 14816-14827, 14853-14864
src/locales/ru/main.json (1)
19-83: Avoid manual edits to generated locale files.
These RU entries live in a non‑EN locale file that is regenerated on release; please drop these changes from the PR and keep only the EN additions insrc/locales/en/main.json.Based on learnings In the Comfy-Org/ComfyUI_frontend repository, locale files other than
src/locales/en/main.jsonare generated automatically on every release. Developers only need to add English (en) key/values insrc/locales/en/main.jsonwhen making PRs; manual updates to other locale files (fr, ja, ko, ru, zh, zh-TW, es, ar, tr, etc.) are not required and should not be suggested in reviews.src/locales/fr/main.json (1)
19-83: Avoid manual edits to generated locale files.
These FR entries live in a non‑EN locale file that is regenerated on release; please drop these changes from the PR and keep only the EN additions insrc/locales/en/main.json.Based on learnings In the Comfy-Org/ComfyUI_frontend repository, locale files other than
src/locales/en/main.jsonare generated automatically on every release. Developers only need to add English (en) key/values insrc/locales/en/main.jsonwhen making PRs; manual updates to other locale files (fr, ja, ko, ru, zh, zh-TW, es, ar, tr, etc.) are not required and should not be suggested in reviews.
Comment on lines
2212
to
2227
| "EmptyAceStep1_5LatentAudio": { | ||
| "display_name": "Empty Ace Step 1.5 Latent Audio", | ||
| "inputs": { | ||
| "seconds": { | ||
| "name": "seconds" | ||
| }, | ||
| "batch_size": { | ||
| "name": "batch_size", | ||
| "tooltip": "The number of latent images in the batch." | ||
| } | ||
| }, | ||
| "outputs": { | ||
| "0": { | ||
| "tooltip": null | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
Fix audio batch_size tooltip wording.
This node is for latent audio, but the tooltip still says “latent images,” which is confusing in the UI.
✏️ Proposed text fix
"batch_size": {
"name": "batch_size",
- "tooltip": "The number of latent images in the batch."
+ "tooltip": "The number of latent audio samples in the batch."
}📝 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.
Suggested change
| "EmptyAceStep1_5LatentAudio": { | |
| "display_name": "Empty Ace Step 1.5 Latent Audio", | |
| "inputs": { | |
| "seconds": { | |
| "name": "seconds" | |
| }, | |
| "batch_size": { | |
| "name": "batch_size", | |
| "tooltip": "The number of latent images in the batch." | |
| } | |
| }, | |
| "outputs": { | |
| "0": { | |
| "tooltip": null | |
| } | |
| } | |
| "EmptyAceStep1_5LatentAudio": { | |
| "display_name": "Empty Ace Step 1.5 Latent Audio", | |
| "inputs": { | |
| "seconds": { | |
| "name": "seconds" | |
| }, | |
| "batch_size": { | |
| "name": "batch_size", | |
| "tooltip": "The number of latent audio samples in the batch." | |
| } | |
| }, | |
| "outputs": { | |
| "0": { | |
| "tooltip": null | |
| } | |
| } |
🤖 Prompt for AI Agents
In `@src/locales/en/nodeDefs.json` around lines 2212 - 2227, The batch_size
tooltip for the node "EmptyAceStep1_5LatentAudio" is incorrect (mentions latent
images); update the "inputs"."batch_size"."tooltip" text to reference audio
instead — e.g., change it to "The number of latent audio items in the batch." so
the UI correctly describes latent audio batching for the
EmptyAceStep1_5LatentAudio node.
Comment on lines
7316
to
7326
| "outputs": [ | ||
| null, | ||
| { | ||
| "name": "GLB", | ||
| "tooltip": null | ||
| }, | ||
| { | ||
| "name": "FBX", | ||
| "tooltip": null | ||
| } | ||
| } | ||
| ] |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
ru_file="src/locales/ru/nodeDefs.json"
en_file="$(fd -t f 'nodeDefs.json' src/locales | rg '/en/' | head -n1)"
if [[ -z "$en_file" ]]; then
echo "English nodeDefs.json not found under src/locales"; exit 1
fi
EN_PATH="$en_file" python - <<'PY'
import json, os, sys
ru_path = "src/locales/ru/nodeDefs.json"
en_path = os.environ["EN_PATH"]
nodes = [
"MeshyAnimateModelNode","MeshyImageToModelNode","MeshyMultiImageToModelNode",
"MeshyRefineNode","MeshyRigModelNode","MeshyTextToModelNode","MeshyTextureNode",
"Rodin3D_Detail","Rodin3D_Gen2","Rodin3D_Regular","Rodin3D_Sketch","Rodin3D_Smooth",
"TencentImageToModelNode","TencentTextToModelNode",
"TripoImageToModelNode","TripoMultiviewToModelNode","TripoRefineNode",
"TripoRetargetNode","TripoRigNode","TripoTextToModelNode","TripoTextureNode",
]
def normalize(outputs):
if isinstance(outputs, list):
return [o.get("name") if isinstance(o, dict) else None for o in outputs]
if isinstance(outputs, dict):
return [(k, (v.get("name") if isinstance(v, dict) else None)) for k, v in sorted(outputs.items(), key=lambda kv: kv[0])]
return outputs
with open(ru_path, "r", encoding="utf-8") as f:
ru = json.load(f)
with open(en_path, "r", encoding="utf-8") as f:
en = json.load(f)
mismatches = []
for n in nodes:
ru_out = ru.get(n, {}).get("outputs")
en_out = en.get(n, {}).get("outputs")
if ru_out is None or en_out is None:
mismatches.append((n, "missing outputs", ru_out is None, en_out is None))
continue
if normalize(ru_out) != normalize(en_out):
mismatches.append((n, normalize(ru_out), normalize(en_out)))
if mismatches:
print("Output mismatches:")
for m in mismatches:
print("-", m)
sys.exit(1)
else:
print("All outputs match for listed nodes.")
PYRepository: Comfy-Org/ComfyUI_frontend
Length of output: 2296
Align RU output definitions with EN canonical format—critical mismatches detected across all 3D nodes.
Verification confirms significant discrepancies: the RU locale is missing essential outputs present in EN. For example, Meshy nodes in RU lack model_file and meshy_task_id outputs; Rodin3D nodes lack 3D Model Path; Tripo nodes lack model_file and task ID outputs. Additionally, RU uses array structure while EN uses object/dict structure, compounding the mislabeling risk. All 21 tested nodes show mismatches. The RU definitions must be completed to match EN's full output set and structure.
🤖 Prompt for AI Agents
In `@src/locales/ru/nodeDefs.json` around lines 7316 - 7326, The RU locale's
nodeDefs.json "outputs" entries are using an array form and are missing many
keys that the EN canonical uses; update each 3D node (e.g., Meshy, Rodin3D,
Tripo and the other affected nodes) so their "outputs" match the EN structure
exactly: convert the array to an object/dictionary mapping output keys to
objects (e.g., include "model_file", "meshy_task_id", "3d_model_path"/"3D Model
Path", "<node>_task_id", etc. as present in EN), ensure each output object has
the same "name" and "tooltip" fields as EN, and mirror the exact key names and
values used in the EN canonical to avoid mislabeling.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/locales/pt-BR/nodeDefs.json (1)
6430-6463:⚠️ Potential issue | 🔴 CriticalFix Load3D input and output schema to match base locale format.
The pt-BR Load3D localization has two schema mismatches with the English base locale:
- Button/action inputs use
{"": "..."}instead of{}for "clear", "upload 3d model", and "upload extra resources"- Outputs use array format instead of object format with indexed string keys ("0"–"6")
💡 Correct schema (matching English base)
- "clear": { - "": "limpar" - }, + "clear": {}, ... - "upload 3d model": { - "": "enviar modelo 3D" - }, + "upload 3d model": {}, ... - "upload extra resources": { - "": "enviar recursos extras" - }, + "upload extra resources": {}, ... - "outputs": [ - null, - null, - null, - null, - null, - null, - { - "name": "model_3d", - "tooltip": null - } - ] + "outputs": { + "0": { "name": "image", "tooltip": null }, + "1": { "name": "mask", "tooltip": null }, + "2": { "name": "mesh_path", "tooltip": null }, + "3": { "name": "normal", "tooltip": null }, + "4": { "name": "camera_info", "tooltip": null }, + "5": { "name": "recording_video", "tooltip": null }, + "6": { "name": "model_3d", "tooltip": null } + }
AustinMroz
approved these changes
Feb 5, 2026
snomiao
pushed a commit
that referenced
this pull request
Feb 6, 2026
## Summary
This PR removes `any` types from widgets, services, stores, and test
files, replacing them with proper TypeScript types.
### Key Changes
#### Type Safety Improvements
- Replaced `any` with `unknown`, explicit types, or proper interfaces
across widgets and services
- Added proper type imports (TgpuRoot, Point, StyleValue, etc.)
- Created typed interfaces (NumericWidgetOptions, TestWindow,
ImportFailureDetail, etc.)
- Fixed function return types to be non-nullable where appropriate
- Added type guards and null checks instead of non-null assertions
- Used `ComponentProps` from vue-component-type-helpers for component
testing
#### Widget System
- Added index signature to IWidgetOptions for Record compatibility
- Centralized disabled logic in WidgetInputNumberInput
- Moved template type assertions to computed properties
- Fixed ComboWidget getOptionLabel type assertions
- Improved remote widget type handling with runtime checks
#### Services & Stores
- Fixed getOrCreateViewer to return non-nullable values
- Updated addNodeOnGraph to use specific options type `{ pos?: Point }`
- Added proper type assertions for settings store retrieval
- Fixed executionIdToCurrentId return type (string | undefined)
#### Test Infrastructure
- Exported GraphOrSubgraph from litegraph barrel to avoid circular
dependencies
- Updated test fixtures with proper TypeScript types (TestInfo,
LGraphNode)
- Replaced loose Record types with ComponentProps in tests
- Added proper error handling in WebSocket fixture
#### Code Organization
- Created shared i18n-types module for locale data types
- Made ImportFailureDetail non-exported (internal use only)
- Added @public JSDoc tag to ElectronWindow type
- Fixed console.log usage in scripts to use allowed methods
### Files Changed
**Widgets & Components:**
-
src/renderer/extensions/vueNodes/widgets/components/WidgetInputNumberInput.vue
-
src/renderer/extensions/vueNodes/widgets/components/WidgetSelectDefault.vue
-
src/renderer/extensions/vueNodes/widgets/components/WidgetSelectDropdown.vue
- src/renderer/extensions/vueNodes/widgets/components/WidgetTextarea.vue
-
src/renderer/extensions/vueNodes/widgets/composables/useRemoteWidget.ts
- src/lib/litegraph/src/widgets/ComboWidget.ts
- src/lib/litegraph/src/types/widgets.ts
- src/components/common/LazyImage.vue
- src/components/load3d/Load3dViewerContent.vue
**Services & Stores:**
- src/services/litegraphService.ts
- src/services/load3dService.ts
- src/services/colorPaletteService.ts
- src/stores/maskEditorStore.ts
- src/stores/nodeDefStore.ts
- src/platform/settings/settingStore.ts
- src/platform/workflow/management/stores/workflowStore.ts
**Composables & Utils:**
- src/composables/node/useWatchWidget.ts
- src/composables/useCanvasDrop.ts
- src/utils/widgetPropFilter.ts
- src/utils/queueDisplay.ts
- src/utils/envUtil.ts
**Test Files:**
- browser_tests/fixtures/ComfyPage.ts
- browser_tests/fixtures/ws.ts
- browser_tests/tests/actionbar.spec.ts
-
src/workbench/extensions/manager/components/manager/skeleton/PackCardGridSkeleton.test.ts
- src/lib/litegraph/src/subgraph/subgraphUtils.test.ts
- src/components/rightSidePanel/shared.test.ts
- src/platform/cloud/subscription/composables/useSubscription.test.ts
-
src/platform/workflow/persistence/composables/useWorkflowPersistence.test.ts
**Scripts & Types:**
- scripts/i18n-types.ts (new shared module)
- scripts/diff-i18n.ts
- scripts/check-unused-i18n-keys.ts
- src/workbench/extensions/manager/types/conflictDetectionTypes.ts
- src/types/algoliaTypes.ts
- src/types/simplifiedWidget.ts
**Infrastructure:**
- src/lib/litegraph/src/litegraph.ts (added GraphOrSubgraph export)
- src/lib/litegraph/src/infrastructure/CustomEventTarget.ts
- src/platform/assets/services/assetService.ts
**Stories:**
- apps/desktop-ui/src/views/InstallView.stories.ts
- src/components/queue/job/JobDetailsPopover.stories.ts
**Extension Manager:**
- src/workbench/extensions/manager/composables/useConflictDetection.ts
- src/workbench/extensions/manager/composables/useManagerQueue.ts
- src/workbench/extensions/manager/services/comfyManagerService.ts
- src/workbench/extensions/manager/utils/conflictMessageUtil.ts
### Testing
- [x] All TypeScript type checking passes (`pnpm typecheck`)
- [x] ESLint passes without errors (`pnpm lint`)
- [x] Format checks pass (`pnpm format:check`)
- [x] Knip (unused exports) passes (`pnpm knip`)
- [x] Pre-commit and pre-push hooks pass
Part of the "Road to No Explicit Any" initiative.
### Previous PRs in this series:
- Part 2: #7401
- Part 3: #7935
- Part 4: #7970
- Part 5: #8064
- Part 6: #8083
- Part 7: #8092
- Part 8 Group 1: #8253
- Part 8 Group 2: #8258
- Part 8 Group 3: #8304
- Part 8 Group 4: #8314
- Part 8 Group 5: #8329
- Part 8 Group 6: #8344
- Part 8 Group 7: #8459
- Part 8 Group 8: #8496
- Part 9: #8498
- Part 10: #8499
---------
Co-authored-by: Comfy Org PR Bot <snomiao+comfy-pr@gmail.com>
Co-authored-by: christian-byrne <72887196+christian-byrne@users.noreply.github.com>
Co-authored-by: github-actions <github-actions@github.com>
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
3 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
This PR removes
anytypes from widgets, services, stores, and test files, replacing them with proper TypeScript types.Key Changes
Type Safety Improvements
anywithunknown, explicit types, or proper interfaces across widgets and servicesComponentPropsfrom vue-component-type-helpers for component testingWidget System
Services & Stores
{ pos?: Point }Test Infrastructure
Code Organization
Files Changed
Widgets & Components:
Services & Stores:
Composables & Utils:
Test Files:
Scripts & Types:
Infrastructure:
Stories:
Extension Manager:
Testing
pnpm typecheck)pnpm lint)pnpm format:check)pnpm knip)Part of the "Road to No Explicit Any" initiative.
Previous PRs in this series: