fix: address code review feedback

- Fix widget value set to placeholder instead of empty string
- Eliminate duplicate rawValues resolution
- Fix ellipsis formatting and Vue props destructuring
- Add test for undefined placeholder edge case

Amp-Thread-ID: https://ampcode.com/threads/T-019c2c7e-2ac1-7114-9147-b41e6334faa9

src/lib/litegraph/src/widgets/ComboWidget.test.ts

@@ -1064,14 +1064,14 @@ describe('ComboWidget', () => {
           value: '',
           options: {
             values: [],
-            placeholder: 'No models found in ComfyUI/models/checkpoints folder . . .'
+            placeholder: 'No models found in ComfyUI/models/checkpoints folder...'
           }
         }),
         node
       )
 
       expect(widget._displayValue).toBe(
-        'No models found in ComfyUI/models/checkpoints folder . . .'
+        'No models found in ComfyUI/models/checkpoints folder...'
       )
     })
 
@@ -1082,7 +1082,7 @@ describe('ComboWidget', () => {
           value: 'model.safetensors',
           options: {
             values: ['model.safetensors', 'other.safetensors'],
-            placeholder: 'No models found in ComfyUI/models/checkpoints folder . . .'
+            placeholder: 'No models found in ComfyUI/models/checkpoints folder...'
           }
         }),
         node
@@ -1091,6 +1091,22 @@ describe('ComboWidget', () => {
       expect(widget._displayValue).toBe('model.safetensors')
     })
 
+    it('should not display placeholder when placeholder is undefined', () => {
+      widget = new ComboWidget(
+        createMockWidgetConfig({
+          name: 'ckpt_name',
+          value: '',
+          options: {
+            values: [],
+            placeholder: undefined
+          }
+        }),
+        node
+      )
+
+      expect(widget._displayValue).toBe('')
+    })
+
     it('should throw error when values is null in getValues', () => {
       widget = new ComboWidget(
         createMockWidgetConfig({

src/lib/litegraph/src/widgets/ComboWidget.ts

@@ -36,12 +36,12 @@ export class ComboWidget
     if (this.computedDisabled) return ''
 
     const { values: rawValues, placeholder } = this.options
-    if (rawValues) {
-      const values = typeof rawValues === 'function' ? rawValues() : rawValues
-      const valuesArray = toArray(values)
-      if (valuesArray.length === 0 && placeholder) {
-        return placeholder
-      }
+    const resolvedValues =
+      typeof rawValues === 'function' ? rawValues(this, this.node) : rawValues
+    const valuesArray = toArray(resolvedValues)
+
+    if (valuesArray.length === 0 && placeholder) {
+      return placeholder
     }
 
     const getOptionLabel = this.options.getOptionLabel
@@ -54,12 +54,8 @@ export class ComboWidget
       }
     }
 
-    if (rawValues) {
-      const values = typeof rawValues === 'function' ? rawValues() : rawValues
-
-      if (values && !Array.isArray(values)) {
-        return values[this.value]
-      }
+    if (resolvedValues && !Array.isArray(resolvedValues)) {
+      return resolvedValues[this.value]
     }
     return typeof this.value === 'number' ? String(this.value) : this.value
   }

src/renderer/extensions/vueNodes/widgets/components/WidgetSelect.test.ts

@@ -348,4 +348,32 @@ describe('WidgetSelect Value Binding', () => {
       expect(wrapper.findComponent(WidgetSelectDropdown).exists()).toBe(false)
     })
   })
+
+  describe('Empty options with placeholder', () => {
+    it('shows placeholder when options are empty', () => {
+      const widget = createMockWidget('', {
+        values: [],
+        placeholder: 'No models found in ComfyUI/models/checkpoints folder...'
+      })
+      const wrapper = mountComponent(widget, '')
+      const selectDefault = wrapper.findComponent(WidgetSelectDefault)
+      const select = selectDefault.findComponent(SelectPlus)
+
+      expect(select.props('placeholder')).toBe(
+        'No models found in ComfyUI/models/checkpoints folder...'
+      )
+    })
+
+    it('does not show placeholder when options exist', () => {
+      const widget = createMockWidget('option1', {
+        values: ['option1', 'option2'],
+        placeholder: 'No models found'
+      })
+      const wrapper = mountComponent(widget, 'option1')
+      const selectDefault = wrapper.findComponent(WidgetSelectDefault)
+      const select = selectDefault.findComponent(SelectPlus)
+
+      expect(select.props('placeholder')).toBeFalsy()
+    })
+  })
 })

src/renderer/extensions/vueNodes/widgets/components/WidgetSelectDefault.vue

@@ -51,7 +51,7 @@ interface Props {
   widget: SimplifiedWidget<string | undefined>
 }
 
-const props = defineProps<Props>()
+const { widget } = defineProps<Props>()
 
 function resolveValues(values: unknown): string[] {
   if (typeof values === 'function') return values()
@@ -76,15 +76,33 @@ function refreshOptions() {
 }
 const selectOptions = computed(() => {
   void refreshTrigger.value
-  return resolveValues(props.widget.options?.values)
+  return resolveValues(widget.options?.values)
 })
 const invalid = computed(
   () => !!modelValue.value && !selectOptions.value.includes(modelValue.value)
 )
 
-const combinedProps = computed(() => ({
-  ...filterWidgetProps(props.widget.options, PANEL_EXCLUDED_PROPS),
-  ...transformCompatProps.value,
-  ...(invalid.value ? { placeholder: `${modelValue.value}` } : {})
-}))
+const hasEmptyOptions = computed(() => selectOptions.value.length === 0)
+
+const combinedProps = computed(() => {
+  // Extract placeholder to handle it separately based on empty/invalid state
+  const { placeholder: _, ...filteredOptions } = filterWidgetProps(
+    widget.options,
+    PANEL_EXCLUDED_PROPS
+  )
+  const baseProps = {
+    ...filteredOptions,
+    ...transformCompatProps.value
+  }
+
+  if (hasEmptyOptions.value && widget.options?.placeholder) {
+    return { ...baseProps, placeholder: widget.options.placeholder }
+  }
+
+  if (invalid.value) {
+    return { ...baseProps, placeholder: `${modelValue.value}` }
+  }
+
+  return baseProps
+})
 </script>

src/renderer/extensions/vueNodes/widgets/composables/useComboWidget.ts

@@ -204,11 +204,10 @@ const addComboWidget = (
   }
 
   // Standard combo widget
-  const hasEmptyOptions = !inputSpec.options?.length && !inputSpec.remote
   const widget = node.addWidget(
     'combo',
     inputSpec.name,
-    hasEmptyOptions && inputSpec.placeholder ? inputSpec.placeholder : defaultValue,
+    defaultValue ?? '',
     () => {},
     {
       values: inputSpec.options ?? [],