From 975f45842cd0edfde186a045d4f43c2df1b3172f Mon Sep 17 00:00:00 2001
From: Christian Byrne <cbyrne@comfy.org>
Date: Sun, 2 Nov 2025 18:23:50 -0800
Subject: [PATCH] refactor: use placeholder to display deserialized values
 instead of modifying options

Simplified the approach to show deserialized workflow values that are not in
the current options list. Instead of adding missing values to the options array,
we now display them as the placeholder text. This is cleaner, avoids array
manipulation, and addresses performance concerns from code review.
---
 .../widgets/components/WidgetSelect.test.ts   |  29 +++--
 .../components/WidgetSelectDefault.vue        |  21 +++-
 .../components/WidgetSelectDropdown.vue       |  21 +++-
 .../widgets/utils/widgetOptionsUtils.test.ts  | 117 ------------------
 .../widgets/utils/widgetOptionsUtils.ts       |  31 -----
 5 files changed, 49 insertions(+), 170 deletions(-)
 delete mode 100644 src/renderer/extensions/vueNodes/widgets/utils/widgetOptionsUtils.test.ts
 delete mode 100644 src/renderer/extensions/vueNodes/widgets/utils/widgetOptionsUtils.ts

diff --git a/src/renderer/extensions/vueNodes/widgets/components/WidgetSelect.test.ts b/src/renderer/extensions/vueNodes/widgets/components/WidgetSelect.test.ts
index a2bea8ba7..fad2def1f 100644
--- a/src/renderer/extensions/vueNodes/widgets/components/WidgetSelect.test.ts
+++ b/src/renderer/extensions/vueNodes/widgets/components/WidgetSelect.test.ts
@@ -180,7 +180,7 @@ describe('WidgetSelect Value Binding', () => {
       expect(emitted![0]).toContain('100')
     })
 
-    it('displays value not in options list (deserialized workflow value)', async () => {
+    it('displays value not in options list as placeholder (deserialized workflow value)', async () => {
       // Simulate a workflow loaded with a value that's no longer in the options
       // (e.g., a deleted model file)
       const currentOptions = ['model1.ckpt', 'model2.safetensors']
@@ -191,30 +191,29 @@ describe('WidgetSelect Value Binding', () => {
       const wrapper = mountComponent(widget, deserializedValue)
 
       const select = wrapper.findComponent({ name: 'Select' })
-      const options = select.props('options')
 
-      // The current value should be included in the options
-      expect(options).toContain(deserializedValue)
-      // And it should be first
-      expect(options[0]).toBe(deserializedValue)
-      // Original options should still be present
+      // The deserialized value should be shown as the placeholder
+      expect(select.props('placeholder')).toBe(deserializedValue)
+
+      // The options should remain unchanged (not include the deserialized value)
+      const options = select.props('options')
+      expect(options).not.toContain(deserializedValue)
       expect(options).toContain('model1.ckpt')
       expect(options).toContain('model2.safetensors')
     })
 
-    it('does not duplicate value if it exists in options', async () => {
+    it('uses widget placeholder when value exists in options', async () => {
       const options = ['option1', 'option2', 'option3']
-      const widget = createMockWidget('option2', { values: options })
+      const widget = createMockWidget('option2', {
+        values: options,
+        placeholder: 'Select an option'
+      })
       const wrapper = mountComponent(widget, 'option2')
 
       const select = wrapper.findComponent({ name: 'Select' })
-      const selectOptions = select.props('options') as string[]
 
-      // Should not duplicate the value
-      expect(selectOptions).toEqual(options)
-      expect(
-        selectOptions.filter((opt: string) => opt === 'option2')
-      ).toHaveLength(1)
+      // Should use the widget's placeholder since value is in options
+      expect(select.props('placeholder')).toBe('Select an option')
     })
   })
 
diff --git a/src/renderer/extensions/vueNodes/widgets/components/WidgetSelectDefault.vue b/src/renderer/extensions/vueNodes/widgets/components/WidgetSelectDefault.vue
index dced27b67..2a1fa1dba 100644
--- a/src/renderer/extensions/vueNodes/widgets/components/WidgetSelectDefault.vue
+++ b/src/renderer/extensions/vueNodes/widgets/components/WidgetSelectDefault.vue
@@ -3,6 +3,7 @@
     <Select
       v-model="localValue"
       :options="selectOptions"
+      :placeholder="selectPlaceholder"
       v-bind="combinedProps"
       :class="cn(WidgetInputBaseClass, 'w-full text-xs')"
       :aria-label="widget.name"
@@ -64,9 +65,27 @@ const selectOptions = computed(() => {
   const options = props.widget.options
 
   if (options?.values && Array.isArray(options.values)) {
-    return ensureValueInOptions(options.values, localValue.value)
+    return options.values
   }
 
   return []
 })
+
+// Show the deserialized value as placeholder when it's not in the options list
+// This preserves legacy behavior where workflow values are shown even if deleted
+const selectPlaceholder = computed(() => {
+  const currentValue = localValue.value
+
+  // If there's a current value and it's not in the options, show it as placeholder
+  if (
+    currentValue != null &&
+    currentValue !== '' &&
+    !selectOptions.value.includes(currentValue)
+  ) {
+    return String(currentValue)
+  }
+
+  // Otherwise use the default placeholder from options
+  return props.widget.options?.placeholder
+})
 </script>
diff --git a/src/renderer/extensions/vueNodes/widgets/components/WidgetSelectDropdown.vue b/src/renderer/extensions/vueNodes/widgets/components/WidgetSelectDropdown.vue
index 1c55ddf0d..cb8499b7d 100644
--- a/src/renderer/extensions/vueNodes/widgets/components/WidgetSelectDropdown.vue
+++ b/src/renderer/extensions/vueNodes/widgets/components/WidgetSelectDropdown.vue
@@ -75,12 +75,7 @@ const inputItems = computed<DropdownItem[]>(() => {
     return []
   }
 
-  const valuesWithCurrent = ensureValueInOptions(
-    values,
-    typeof localValue.value === 'string' ? localValue.value : undefined
-  )
-
-  return valuesWithCurrent.map((value: string, index: number) => ({
+  return values.map((value: string, index: number) => ({
     id: `input-${index}`,
     mediaSrc: getMediaUrl(value, 'input'),
     name: value,
@@ -134,6 +129,20 @@ const dropdownItems = computed<DropdownItem[]>(() => {
 })
 
 const mediaPlaceholder = computed(() => {
+  const currentValue = localValue.value
+  const values = props.widget.options?.values || []
+
+  // If there's a current value and it's not in the options, show it as placeholder
+  // This preserves legacy behavior where workflow values are shown even if deleted
+  if (
+    currentValue != null &&
+    currentValue !== '' &&
+    typeof currentValue === 'string' &&
+    !values.includes(currentValue)
+  ) {
+    return currentValue
+  }
+
   const options = props.widget.options
 
   if (options?.placeholder) {
diff --git a/src/renderer/extensions/vueNodes/widgets/utils/widgetOptionsUtils.test.ts b/src/renderer/extensions/vueNodes/widgets/utils/widgetOptionsUtils.test.ts
deleted file mode 100644
index 21fb2aa01..000000000
--- a/src/renderer/extensions/vueNodes/widgets/utils/widgetOptionsUtils.test.ts
+++ /dev/null
@@ -1,117 +0,0 @@
-import { describe, expect, it } from 'vitest'
-
-import { ensureValueInOptions } from './widgetOptionsUtils'
-
-describe('ensureValueInOptions', () => {
-  describe('when value exists in options', () => {
-    it('returns original options without duplicates', () => {
-      const options = ['option1', 'option2', 'option3']
-      const result = ensureValueInOptions(options, 'option2')
-
-      expect(result).toEqual(options)
-      expect(result).toHaveLength(3)
-    })
-
-    it('handles first option', () => {
-      const options = ['first', 'second', 'third']
-      const result = ensureValueInOptions(options, 'first')
-
-      expect(result).toEqual(options)
-    })
-
-    it('handles last option', () => {
-      const options = ['first', 'second', 'third']
-      const result = ensureValueInOptions(options, 'third')
-
-      expect(result).toEqual(options)
-    })
-  })
-
-  describe('when value is missing from options', () => {
-    it('prepends missing value to options array', () => {
-      const options = ['option1', 'option2', 'option3']
-      const result = ensureValueInOptions(options, 'deleted_model.safetensors')
-
-      expect(result).toEqual([
-        'deleted_model.safetensors',
-        'option1',
-        'option2',
-        'option3'
-      ])
-      expect(result).toHaveLength(4)
-    })
-
-    it('preserves deserialized workflow values', () => {
-      const options = ['current_model.ckpt']
-      const oldValue = 'old_model_from_workflow.ckpt'
-      const result = ensureValueInOptions(options, oldValue)
-
-      expect(result[0]).toBe(oldValue)
-      expect(result).toContain('current_model.ckpt')
-    })
-
-    it('handles numeric values', () => {
-      const options = [1, 2, 3]
-      const result = ensureValueInOptions(options, 99)
-
-      expect(result).toEqual([99, 1, 2, 3])
-    })
-  })
-
-  describe('when value is null or empty', () => {
-    it('returns original options for undefined', () => {
-      const options = ['option1', 'option2']
-      const result = ensureValueInOptions(options, undefined)
-
-      expect(result).toEqual(options)
-    })
-
-    it('returns original options for null', () => {
-      const options = ['option1', 'option2']
-      const result = ensureValueInOptions(options, null)
-
-      expect(result).toEqual(options)
-    })
-
-    it('returns original options for empty string', () => {
-      const options = ['option1', 'option2']
-      const result = ensureValueInOptions(options, '')
-
-      expect(result).toEqual(options)
-    })
-  })
-
-  describe('edge cases', () => {
-    it('handles empty options array', () => {
-      const result = ensureValueInOptions([], 'some_value')
-
-      expect(result).toEqual(['some_value'])
-    })
-
-    it('handles options with special characters', () => {
-      const options = ['normal.txt', 'with spaces.png', 'special@#$.jpg']
-      const result = ensureValueInOptions(
-        options,
-        'another file with spaces.png'
-      )
-
-      expect(result[0]).toBe('another file with spaces.png')
-      expect(result).toHaveLength(4)
-    })
-
-    it('creates new array instance (does not mutate input)', () => {
-      const options = ['option1', 'option2']
-      const result = ensureValueInOptions(options, 'option1')
-
-      expect(result).not.toBe(options)
-      expect(result).toEqual(options)
-    })
-
-    it('handles readonly arrays', () => {
-      const options = ['a', 'b', 'c'] as const
-      const result = ensureValueInOptions(options, 'd')
-
-      expect(result).toEqual(['d', 'a', 'b', 'c'])
-    })
-  })
-})
diff --git a/src/renderer/extensions/vueNodes/widgets/utils/widgetOptionsUtils.ts b/src/renderer/extensions/vueNodes/widgets/utils/widgetOptionsUtils.ts
deleted file mode 100644
index e9e1f2ea8..000000000
--- a/src/renderer/extensions/vueNodes/widgets/utils/widgetOptionsUtils.ts
+++ /dev/null
@@ -1,31 +0,0 @@
-/**
- * Utility functions for widget option handling
- */
-
-/**
- * Ensures the current value is included in the options array.
- * This preserves legacy behavior where deserialized workflow values
- * can be shown even if they're not in the current options list
- * (e.g., deleted models, removed files, etc.)
- *
- * @param options - The available options from widget.options.values
- * @param currentValue - The current widget value
- * @returns Options array with current value prepended if missing
- */
-export function ensureValueInOptions<T extends string | number>(
-  options: readonly T[],
-  currentValue: T | undefined | null
-): T[] {
-  // Early return for empty/null values
-  if (currentValue == null || currentValue === '') {
-    return [...options]
-  }
-
-  // If value already exists, return original options
-  if (options.includes(currentValue)) {
-    return [...options]
-  }
-
-  // Prepend missing value to options
-  return [currentValue, ...options]
-}