refactor: extract createAssetWidget to shared factory, remove # privates

- Extract createAssetWidget factory to src/platform/assets/utils/ - Refactor useComboWidget.ts to use the shared factory - Simplify PrimitiveNode to use shared factory - Convert JS # privates to underscore convention - Add knip ignore for isAssetWidget (litegraph public API) Amp-Thread-ID: https://ampcode.com/threads/T-019c0839-bbdc-754a-9d3b-151417058ded Co-authored-by: Amp <amp@ampcode.com>
2026-02-04 15:10:06 +00:00 · 2026-01-28 23:56:01 -08:00
parent 1c898b729a
commit 8f3bafdf31
6 changed files with 180 additions and 153 deletions
--- a/src/extensions/core/widgetInputs.ts
+++ b/src/extensions/core/widgetInputs.ts
@@ -10,16 +10,10 @@ import { NodeSlot } from '@/lib/litegraph/src/node/NodeSlot'
 import type { CanvasPointerEvent } from '@/lib/litegraph/src/types/events'
 import type {
  IBaseWidget,
-  IWidgetAssetOptions,
  TWidgetValue
 } from '@/lib/litegraph/src/types/widgets'
-import { useAssetBrowserDialog } from '@/platform/assets/composables/useAssetBrowserDialog'
-import {
-  assetFilenameSchema,
-  assetItemSchema
-} from '@/platform/assets/schemas/assetSchema'
 import { assetService } from '@/platform/assets/services/assetService'
-import { getAssetFilename } from '@/platform/assets/utils/assetMetadataUtils'
+import { createAssetWidget } from '@/platform/assets/utils/createAssetWidget'
 import { isCloud } from '@/platform/distribution/types'
 import type { InputSpec } from '@/schemas/nodeDefSchema'
 import { app } from '@/scripts/app'
@@ -112,7 +106,7 @@ export class PrimitiveNode extends LGraphNode {

  override onAfterGraphConfigured() {
    if (this.outputs[0].links?.length && !this.widgets?.length) {
-      this.#onFirstConnection()
+      this._onFirstConnection()

      // Populate widget values from config data
      if (this.widgets && this.widgets_values) {
@@ -125,7 +119,7 @@ export class PrimitiveNode extends LGraphNode {
      }

      // Merge values if required
-      this.#mergeWidgetConfig()
+      this._mergeWidgetConfig()
    }
  }

@@ -142,11 +136,11 @@ export class PrimitiveNode extends LGraphNode {
    const links = this.outputs[0].links
    if (connected) {
      if (links?.length && !this.widgets?.length) {
-        this.#onFirstConnection()
+        this._onFirstConnection()
      }
    } else {
      // We may have removed a link that caused the constraints to change
-      this.#mergeWidgetConfig()
+      this._mergeWidgetConfig()

      if (!links?.length) {
        this.onLastDisconnect()
@@ -168,7 +162,7 @@ export class PrimitiveNode extends LGraphNode {
    }

    if (this.outputs[slot].links?.length) {
-      const valid = this.#isValidConnection(input)
+      const valid = this._isValidConnection(input)
      if (valid) {
        // On connect of additional outputs, copy our value to their widget
        this.applyToGraph([{ target_id: target_node.id, target_slot } as LLink])
@@ -179,7 +173,7 @@ export class PrimitiveNode extends LGraphNode {
    return true
  }

-  #onFirstConnection(recreating?: boolean) {
+  private _onFirstConnection(recreating?: boolean) {
    // First connection can fire before the graph is ready on initial load so random things can be missing
    if (!this.outputs[0].links || !this.graph) {
      this.onLastDisconnect()
@@ -213,7 +207,7 @@ export class PrimitiveNode extends LGraphNode {
    this.outputs[0].name = type
    this.outputs[0].widget = widget

-    this.#createWidget(
+    this._createWidget(
      widget[CONFIG] ?? config,
      theirNode,
      widget.name,
@@ -222,7 +216,7 @@ export class PrimitiveNode extends LGraphNode {
    )
  }

-  #createWidget(
+  private _createWidget(
    inputData: InputSpec,
    node: LGraphNode,
    widgetName: string,
@@ -245,8 +239,8 @@ export class PrimitiveNode extends LGraphNode {
        widgetName
      )
      if (isEligible) {
-        widget = this.#createAssetWidget(node, widgetName, inputData)
-        this.#finalizeWidget(widget, oldWidth, oldHeight, recreating)
+        widget = this._createAssetWidget(node, widgetName, inputData)
+        this._finalizeWidget(widget, oldWidth, oldHeight, recreating)
        return
      }
    }
@@ -300,69 +294,34 @@ export class PrimitiveNode extends LGraphNode {
      }
    }

-    this.#finalizeWidget(widget, oldWidth, oldHeight, recreating)
+    this._finalizeWidget(widget, oldWidth, oldHeight, recreating)
  }

-  #createAssetWidget(
+  private _createAssetWidget(
    targetNode: LGraphNode,
-    widgetName: string,
+    _widgetName: string,
    inputData: InputSpec
  ): IBaseWidget {
    const defaultValue = inputData[1]?.default as string | undefined
-    const assetBrowserDialog = useAssetBrowserDialog()
-
-    const openModal = async (widget: IBaseWidget) => {
-      await assetBrowserDialog.show({
-        nodeType: targetNode.comfyClass ?? '',
-        inputName: widgetName,
-        currentValue: widget.value as string,
-        onAssetSelected: (asset) => {
-          const validatedAsset = assetItemSchema.safeParse(asset)
-          if (!validatedAsset.success) {
-            console.error('Invalid asset item:', validatedAsset.error.errors)
-            return
-          }
-
-          const filename = getAssetFilename(validatedAsset.data)
-          const validatedFilename = assetFilenameSchema.safeParse(filename)
-          if (!validatedFilename.success) {
-            console.error(
-              'Invalid asset filename:',
-              validatedFilename.error.errors
-            )
-            return
-          }
-
-          const oldValue = widget.value
-          widget.value = validatedFilename.data
-          widget.callback?.(
-            widget.value,
-            app.canvas,
-            this,
-            app.canvas.graph_mouse,
-            {} as CanvasPointerEvent
-          )
-          this.onWidgetChanged?.(
-            widget.name,
-            validatedFilename.data,
-            oldValue,
-            widget
-          )
-        }
-      })
-    }
-
-    const options: IWidgetAssetOptions = { openModal }
-    return this.addWidget(
-      'asset',
-      'value',
-      defaultValue ?? '',
-      () => {},
-      options
-    )
+    return createAssetWidget({
+      node: this,
+      widgetName: 'value',
+      nodeTypeForBrowser: targetNode.comfyClass ?? '',
+      defaultValue,
+      onValueChange: (widget, newValue, oldValue) => {
+        widget.callback?.(
+          widget.value,
+          app.canvas,
+          this,
+          app.canvas.graph_mouse,
+          {} as CanvasPointerEvent
+        )
+        this.onWidgetChanged?.(widget.name, newValue, oldValue, widget)
+      }
+    })
  }

-  #finalizeWidget(
+  private _finalizeWidget(
    widget: IBaseWidget,
    oldWidth: number,
    oldHeight: number,
@@ -394,8 +353,8 @@ export class PrimitiveNode extends LGraphNode {

  recreateWidget() {
    const values = this.widgets?.map((w) => w.value)
-    this.#removeWidgets()
-    this.#onFirstConnection(true)
+    this._removeWidgets()
+    this._onFirstConnection(true)
    if (values?.length && this.widgets) {
      for (let i = 0; i < this.widgets.length; i++)
        this.widgets[i].value = values[i]
@@ -403,7 +362,7 @@ export class PrimitiveNode extends LGraphNode {
    return this.widgets?.[0]
  }

-  #mergeWidgetConfig() {
+  private _mergeWidgetConfig() {
    // Merge widget configs if the node has multiple outputs
    const output = this.outputs[0]
    const links = output.links ?? []
@@ -435,11 +394,11 @@ export class PrimitiveNode extends LGraphNode {
      const theirInput = theirNode.inputs[link.target_slot]

      // Call is valid connection so it can merge the configs when validating
-      this.#isValidConnection(theirInput, hasConfig)
+      this._isValidConnection(theirInput, hasConfig)
    }
  }

-  #isValidConnection(input: INodeInputSlot, forceUpdate?: boolean) {
+  private _isValidConnection(input: INodeInputSlot, forceUpdate?: boolean) {
    // Only allow connections where the configs match
    const output = this.outputs?.[0]
    const config2 = (input.widget?.[GET_CONFIG] as () => InputSpec)?.()
@@ -454,7 +413,7 @@ export class PrimitiveNode extends LGraphNode {
    )
  }

-  #removeWidgets() {
+  private _removeWidgets() {
    if (this.widgets) {
      // Allow widgets to cleanup
      for (const w of this.widgets) {
@@ -485,7 +444,7 @@ export class PrimitiveNode extends LGraphNode {
    this.outputs[0].name = 'connect to widget input'
    delete this.outputs[0].widget

-    this.#removeWidgets()
+    this._removeWidgets()
  }
 }

--- a/src/lib/litegraph/src/litegraph.ts
+++ b/src/lib/litegraph/src/litegraph.ts
@@ -150,7 +150,9 @@ export { BaseWidget } from './widgets/BaseWidget'

 export { LegacyWidget } from './widgets/LegacyWidget'

-export { isComboWidget, isAssetWidget } from './widgets/widgetMap'
+export { isComboWidget } from './widgets/widgetMap'
+/** @knipIgnoreUnusedButUsedByCustomNodes */
+export { isAssetWidget } from './widgets/widgetMap'
 // Additional test-specific exports
 export { LGraphButton } from './LGraphButton'
 export { MovingOutputLink } from './canvas/MovingOutputLink'
--- a/src/lib/litegraph/src/widgets/widgetMap.ts
+++ b/src/lib/litegraph/src/widgets/widgetMap.ts
@@ -141,7 +141,10 @@ export function isComboWidget(widget: IBaseWidget): widget is IComboWidget {
  return widget.type === 'combo'
 }

-/** Type guard: Narrow **from {@link IBaseWidget}** to {@link IAssetWidget}. */
+/**
+ * Type guard: Narrow **from {@link IBaseWidget}** to {@link IAssetWidget}.
+ * @knipIgnoreUnusedButUsedByCustomNodes
+ */
 export function isAssetWidget(widget: IBaseWidget): widget is IAssetWidget {
  return widget.type === 'asset'
 }
--- a/src/platform/assets/utils/createAssetWidget.ts
+++ b/src/platform/assets/utils/createAssetWidget.ts
@@ -0,0 +1,88 @@
+import { t } from '@/i18n'
+import type { LGraphNode } from '@/lib/litegraph/src/litegraph'
+import type {
+  IBaseWidget,
+  IWidgetAssetOptions
+} from '@/lib/litegraph/src/types/widgets'
+import { useAssetBrowserDialog } from '@/platform/assets/composables/useAssetBrowserDialog'
+import {
+  assetFilenameSchema,
+  assetItemSchema
+} from '@/platform/assets/schemas/assetSchema'
+import { getAssetFilename } from '@/platform/assets/utils/assetMetadataUtils'
+
+interface CreateAssetWidgetParams {
+  /** The node to add the widget to */
+  node: LGraphNode
+  /** The widget name */
+  widgetName: string
+  /** The node type to show in asset browser (may differ from node.comfyClass for PrimitiveNode) */
+  nodeTypeForBrowser: string
+  /** Default value for the widget */
+  defaultValue?: string
+  /** Callback when widget value changes */
+  onValueChange?: (
+    widget: IBaseWidget,
+    newValue: string,
+    oldValue: unknown
+  ) => void
+}
+
+/**
+ * Creates an asset widget that opens the Asset Browser dialog for model selection.
+ * Used by both regular nodes (via useComboWidget) and PrimitiveNode.
+ *
+ * @param params - Configuration for the asset widget
+ * @returns The created asset widget
+ */
+export function createAssetWidget(
+  params: CreateAssetWidgetParams
+): IBaseWidget {
+  const { node, widgetName, nodeTypeForBrowser, defaultValue, onValueChange } =
+    params
+
+  const displayLabel = defaultValue ?? t('widgets.selectModel')
+  const assetBrowserDialog = useAssetBrowserDialog()
+
+  async function openModal(widget: IBaseWidget) {
+    await assetBrowserDialog.show({
+      nodeType: nodeTypeForBrowser,
+      inputName: widgetName,
+      currentValue: widget.value as string,
+      onAssetSelected: (asset) => {
+        const validatedAsset = assetItemSchema.safeParse(asset)
+
+        if (!validatedAsset.success) {
+          console.error(
+            'Invalid asset item:',
+            validatedAsset.error.errors,
+            'Received:',
+            asset
+          )
+          return
+        }
+
+        const filename = getAssetFilename(validatedAsset.data)
+        const validatedFilename = assetFilenameSchema.safeParse(filename)
+
+        if (!validatedFilename.success) {
+          console.error(
+            'Invalid asset filename:',
+            validatedFilename.error.errors,
+            'for asset:',
+            validatedAsset.data.id
+          )
+          return
+        }
+
+        const oldValue = widget.value
+        widget.value = validatedFilename.data
+        onValueChange?.(widget, validatedFilename.data, oldValue)
+      }
+    })
+  }
+
+  const options: IWidgetAssetOptions = { openModal }
+
+  return node.addWidget('asset', widgetName, displayLabel, () => {}, options)
+}
--- a/src/renderer/extensions/vueNodes/widgets/composables/useComboWidget.ts
+++ b/src/renderer/extensions/vueNodes/widgets/composables/useComboWidget.ts
@@ -3,18 +3,10 @@ import { ref } from 'vue'
 import MultiSelectWidget from '@/components/graph/widgets/MultiSelectWidget.vue'
 import { t } from '@/i18n'
 import type { LGraphNode } from '@/lib/litegraph/src/litegraph'
-import { isAssetWidget, isComboWidget } from '@/lib/litegraph/src/litegraph'
-import type {
-  IBaseWidget,
-  IWidgetAssetOptions
-} from '@/lib/litegraph/src/types/widgets'
-import { useAssetBrowserDialog } from '@/platform/assets/composables/useAssetBrowserDialog'
-import {
-  assetFilenameSchema,
-  assetItemSchema
-} from '@/platform/assets/schemas/assetSchema'
-import { getAssetFilename } from '@/platform/assets/utils/assetMetadataUtils'
+import { isComboWidget } from '@/lib/litegraph/src/litegraph'
+import type { IBaseWidget } from '@/lib/litegraph/src/types/widgets'
 import { assetService } from '@/platform/assets/services/assetService'
+import { createAssetWidget } from '@/platform/assets/utils/createAssetWidget'
 import { isCloud } from '@/platform/distribution/types'
 import { useSettingStore } from '@/platform/settings/settingStore'
 import type {
@@ -86,71 +78,20 @@ const addMultiSelectWidget = (
  return widget
 }

-const createAssetBrowserWidget = (
+function createAssetBrowserWidget(
  node: LGraphNode,
  inputSpec: ComboInputSpec,
  defaultValue: string | undefined
-): IBaseWidget => {
-  const currentValue = defaultValue
-  const displayLabel = currentValue ?? t('widgets.selectModel')
-  const assetBrowserDialog = useAssetBrowserDialog()
-
-  async function openModal(widget: IBaseWidget) {
-    if (!isAssetWidget(widget)) {
-      throw new Error(`Expected asset widget but received ${widget.type}`)
+): IBaseWidget {
+  return createAssetWidget({
+    node,
+    widgetName: inputSpec.name,
+    nodeTypeForBrowser: node.comfyClass ?? '',
+    defaultValue,
+    onValueChange: (widget, newValue, oldValue) => {
+      node.onWidgetChanged?.(widget.name, newValue, oldValue, widget)
    }
-    await assetBrowserDialog.show({
-      nodeType: node.comfyClass || '',
-      inputName: inputSpec.name,
-      currentValue: widget.value,
-      onAssetSelected: (asset) => {
-        const validatedAsset = assetItemSchema.safeParse(asset)
-
-        if (!validatedAsset.success) {
-          console.error(
-            'Invalid asset item:',
-            validatedAsset.error.errors,
-            'Received:',
-            asset
-          )
-          return
-        }
-
-        const filename = getAssetFilename(validatedAsset.data)
-        const validatedFilename = assetFilenameSchema.safeParse(filename)
-
-        if (!validatedFilename.success) {
-          console.error(
-            'Invalid asset filename:',
-            validatedFilename.error.errors,
-            'for asset:',
-            validatedAsset.data.id
-          )
-          return
-        }
-
-        const oldValue = widget.value
-        widget.value = validatedFilename.data
-        node.onWidgetChanged?.(
-          widget.name,
-          validatedFilename.data,
-          oldValue,
-          widget
-        )
-      }
-    })
-  }
-  const options: IWidgetAssetOptions = { openModal }
-
-  const widget = node.addWidget(
-    'asset',
-    inputSpec.name,
-    displayLabel,
-    () => undefined,
-    options
-  )
-
-  return widget
+  })
 }

 const createInputMappingWidget = (
--- a/todo.md
+++ b/todo.md
@@ -0,0 +1,34 @@
+# Bug Description
+
+When using a primitive node to select a model, this causes an issue specifically on Comfy Cloud. The issue does not appear to occur in the local version.
+
+## Context
+
+This bug was reported in the #frontend-bug-dump channel with reference links to Slack thread discussions:
+
+- https://comfy-organization.slack.com/archives/C07RCREPL67/p1767751867519549?thread_ts=1767751129.592359&cid=C07RCREPL67
+- https://comfy-organization.slack.com/archives/C07RCREPL67/p1767752100115359?thread_ts=1767751129.592359&cid=C07RCREPL67
+
+## Reproduction
+
+The issue is triggered when a model is selected by a primitive node in workflows running on the cloud.
+
+## Expected Behavior
+
+Model selection via primitive nodes should work consistently between local and cloud environments.
+
+## Actual Behavior
+
+An issue occurs on cloud when using primitive nodes for model selection.
+
+## Additional Information
+
+### Related Bug Ticket
+
+Another bug ticket was created here: https://comfy-organization.slack.com/archives/C09FY39CC3V/p1767753991406309?thread_ts=1767734275.051999&cid=C09FY39CC3V
+
+### Updated Reproduction Details
+
+**Important:** This workflow didn't include any primitive nodes, but still caused the same issue. This indicates the problem is not limited to primitive nodes as originally thought.
+
+**Affected Workflow File:** video*ltx2*t2v_distilled.json (provided in Slack thread)