fix: show load widget inputs in media dropdown#9670
Merged
Conversation
Replace computed itemsKey with a Symbol ref and add a watcher that re-runs the searcher whenever the items prop changes. This fixes the Load Video node not showing videos in the Media Assets panel dropdown when switching between asset types with identical or empty item sets.
Addresses review feedback: #9551 (comment)
- keep combo widget options object identity while providing dynamic values - remove temporary dropdown debug logs and stale search prop plumbing - update dropdown/combo tests and mock typing compatibility Amp-Thread-ID: https://ampcode.com/threads/T-019cd3ac-3866-73df-93be-226ebe67fa09 Co-authored-by: Amp <amp@ampcode.com>
dosubot
bot
added
the
size:L
🎨 Storybook: ✅ Built — View Storybook |
🎭 Playwright: ✅ 556 passed, 0 failed · 2 flaky📊 Browser Reports
|
Contributor
📝 WalkthroughWalkthroughRefactors FormDropdown filtering from synchronous local search to debounced asynchronous filtering via Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
DrJKL
changed the base branch from
ticket/bug-load-video-node-doesnt-show-videos-in-media-as
to
main
The getter was discarding non-array values like 'Loading...' and returning empty fallback array instead. Amp-Thread-ID: https://ampcode.com/threads/T-019cd3ec-0132-730b-b6e7-b30df01bd775 Co-authored-by: Amp <amp@ampcode.com>
📦 Bundle: 4.57 MB gzip 🔴 +464 BDetailsSummary
Category Glance App Entry Points — 28.9 kB (baseline 28.9 kB) • ⚪ 0 BMain entry bundles and manifests
Status: 1 added / 1 removed Graph Workspace — 967 kB (baseline 967 kB) • 🔴 +1 BGraph editor runtime, canvas, workflow orchestration
Status: 1 added / 1 removed Views & Navigation — 72.4 kB (baseline 72.4 kB) • ⚪ 0 BTop-level views, pages, and routed surfaces
Status: 9 added / 9 removed Panels & Settings — 436 kB (baseline 436 kB) • ⚪ 0 BConfiguration panels, inspectors, and settings screens
Status: 10 added / 10 removed User & Accounts — 16.1 kB (baseline 16.1 kB) • ⚪ 0 BAuthentication, profile, and account management bundles
Status: 5 added / 5 removed Editors & Dialogs — 77.5 kB (baseline 77.5 kB) • ⚪ 0 BModals, dialogs, drawers, and in-app editors
Status: 2 added / 2 removed UI Components — 56.5 kB (baseline 56.5 kB) • ⚪ 0 BReusable component library chunks
Status: 7 added / 7 removed Data & Services — 2.77 MB (baseline 2.77 MB) • 🔴 +253 BStores, services, APIs, and repositories
Status: 14 added / 14 removed Utilities & Hooks — 56.8 kB (baseline 56.8 kB) • ⚪ 0 BHelpers, composables, and utility bundles
Status: 12 added / 12 removed Vendor & Third-Party — 8.88 MB (baseline 8.88 MB) • 🔴 +1.36 kBExternal libraries and shared vendor chunks
Status: 1 added / 1 removed Other — 8.04 MB (baseline 8.04 MB) • 🟢 -435 BBundles that do not match a named category
Status: 57 added / 57 removed |
⚡ Performance Report
Raw data{
"timestamp": "2026-03-09T19:15:31.128Z",
"gitSha": "c0f586f625b2bff34f6becc4eac9d386906a6fe2",
"branch": "drjkl/computed-async-filtered-items",
"measurements": [
{
"name": "canvas-idle",
"durationMs": 2042.6889999999958,
"styleRecalcs": 124,
"styleRecalcDurationMs": 26.066000000000003,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 460.22200000000004,
"heapDeltaBytes": -3774536
},
{
"name": "canvas-idle",
"durationMs": 2026.9720000000007,
"styleRecalcs": 123,
"styleRecalcDurationMs": 28.012,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 459.68199999999996,
"heapDeltaBytes": -3808168
},
{
"name": "canvas-idle",
"durationMs": 2068.1040000000053,
"styleRecalcs": 128,
"styleRecalcDurationMs": 31.392999999999997,
"layouts": 1,
"layoutDurationMs": 0.192,
"taskDurationMs": 471.52699999999993,
"heapDeltaBytes": -3069764
},
{
"name": "canvas-mouse-sweep",
"durationMs": 2207.3579999999993,
"styleRecalcs": 193,
"styleRecalcDurationMs": 68.44399999999999,
"layouts": 12,
"layoutDurationMs": 3.954,
"taskDurationMs": 1185.4959999999999,
"heapDeltaBytes": -2671804
},
{
"name": "canvas-mouse-sweep",
"durationMs": 1900.622999999996,
"styleRecalcs": 175,
"styleRecalcDurationMs": 55.68000000000001,
"layouts": 13,
"layoutDurationMs": 4.162,
"taskDurationMs": 860.341,
"heapDeltaBytes": -2346512
},
{
"name": "canvas-mouse-sweep",
"durationMs": 1853.9030000000025,
"styleRecalcs": 171,
"styleRecalcDurationMs": 52.894999999999996,
"layouts": 13,
"layoutDurationMs": 3.7160000000000006,
"taskDurationMs": 848.3159999999999,
"heapDeltaBytes": -3105708
},
{
"name": "dom-widget-clipping",
"durationMs": 638.2830000000013,
"styleRecalcs": 46,
"styleRecalcDurationMs": 15.125000000000002,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 408.66400000000004,
"heapDeltaBytes": 7366236
},
{
"name": "dom-widget-clipping",
"durationMs": 570.3800000000001,
"styleRecalcs": 42,
"styleRecalcDurationMs": 13.524999999999999,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 358.71,
"heapDeltaBytes": 6543356
},
{
"name": "dom-widget-clipping",
"durationMs": 576.6249999999786,
"styleRecalcs": 42,
"styleRecalcDurationMs": 14.583,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 372.52699999999993,
"heapDeltaBytes": 6878712
},
{
"name": "subgraph-dom-widget-clipping",
"durationMs": 625.3900000000385,
"styleRecalcs": 75,
"styleRecalcDurationMs": 17.256,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 435.008,
"heapDeltaBytes": -8969984
},
{
"name": "subgraph-dom-widget-clipping",
"durationMs": 621.9110000000114,
"styleRecalcs": 74,
"styleRecalcDurationMs": 16.286,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 432.78700000000003,
"heapDeltaBytes": -8868972
},
{
"name": "subgraph-dom-widget-clipping",
"durationMs": 613.4860000000231,
"styleRecalcs": 74,
"styleRecalcDurationMs": 16.216,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 434.47400000000005,
"heapDeltaBytes": -8518148
},
{
"name": "subgraph-idle",
"durationMs": 2020.8959999999934,
"styleRecalcs": 122,
"styleRecalcDurationMs": 29.583,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 502.66300000000007,
"heapDeltaBytes": -2714048
},
{
"name": "subgraph-idle",
"durationMs": 1998.817000000031,
"styleRecalcs": 121,
"styleRecalcDurationMs": 25.153,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 428.781,
"heapDeltaBytes": -4675696
},
{
"name": "subgraph-idle",
"durationMs": 2003.5890000000336,
"styleRecalcs": 121,
"styleRecalcDurationMs": 25.998,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 441.62,
"heapDeltaBytes": -3823768
},
{
"name": "subgraph-mouse-sweep",
"durationMs": 1714.2939999999953,
"styleRecalcs": 156,
"styleRecalcDurationMs": 53.854,
"layouts": 16,
"layoutDurationMs": 5.182,
"taskDurationMs": 843.6779999999999,
"heapDeltaBytes": -6167072
},
{
"name": "subgraph-mouse-sweep",
"durationMs": 1969.1520000000082,
"styleRecalcs": 171,
"styleRecalcDurationMs": 58.84400000000001,
"layouts": 16,
"layoutDurationMs": 4.713,
"taskDurationMs": 1002.248,
"heapDeltaBytes": -5187976
},
{
"name": "subgraph-mouse-sweep",
"durationMs": 1708.9320000000043,
"styleRecalcs": 155,
"styleRecalcDurationMs": 46.597,
"layouts": 16,
"layoutDurationMs": 4.249,
"taskDurationMs": 763.76,
"heapDeltaBytes": -5151788
}
]
} |
![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: 2
🧹 Nitpick comments (2)
src/renderer/extensions/vueNodes/widgets/composables/useComboWidget.test.ts (1)
338-353: Turn this into a live-update regression test.This only proves the
optionsobject identity is preserved. A stale snapshot implementation would still pass. Please mutatemockAssetsStoreState.inputAssetsafter construction and assertwidget.options.valueschanges, then add the same coverage for theinputSpec.remotepath sincebindDynamicValuesOption()now backs both call sites.As per coding guidelines, do not write tests that just test the mocks; aim for behavioral coverage of critical and new features.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/extensions/vueNodes/widgets/composables/useComboWidget.test.ts` around lines 338 - 353, Update the test for useComboWidget to be a live-update regression: after constructing the widget via constructor(mockNode, inputSpec) (and for a second case with inputSpec.remote = true), mutate mockAssetsStoreState.inputAssets and then assert that widget.options.values reflects the updated assets (verifying dynamic binding via bindDynamicValuesOption); keep the original identity assertion but replace static identity-only verification with the mutation + expectation for both the local options path and the inputSpec.remote path so the test proves live updates rather than snapshot/mocked identity.src/renderer/extensions/vueNodes/widgets/composables/useComboWidget.ts (1)
47-67: The setter narrowing is asymmetric but not a practical issue in current use.Line 64's setter accepts
unknown[]and only preserves arrays infallbackValues, but the getter returns any non-null value fromgetValues(). While ComboWidget historically supports function-valuedoptions.values, this pattern is deprecated (see ComboWidget.ts:131) and no code in the codebase writes non-array values to this property. The actual dynamic use case (useRemoteWidget.ts) bypassesoptions.valuesentirely, updatingwidget.valuedirectly instead. Consider storing the original value descriptor or converting non-array values to a stable representation if defensive preservation is needed, but this is not required for current functionality.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/extensions/vueNodes/widgets/composables/useComboWidget.ts` around lines 47 - 67, The current bindDynamicValuesOption redefines options.values asymmetrically (setter accepts unknown[] and preserves only arrays while getter may return any non-null getValues() result); to fix, capture the original descriptor via Object.getOwnPropertyDescriptor(options, 'values') before redefining and either (a) delegate to the original getter/setter when non-array or function values are encountered, or (b) normalize non-array sets into a stable array fallback (e.g., wrap single values or ignore non-arrays consistently) so setter/getter behavior is symmetric; update bindDynamicValuesOption (and ensure compatibility with ComboWidget and useRemoteWidget) to use the preserved descriptor or normalization approach.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@src/renderer/extensions/vueNodes/widgets/components/form/dropdown/FormDropdown.test.ts`:
- Around line 65-119: The tests use stacked await nextTick() calls which don't
wait for promises produced by the component's computedAsync to settle, causing
flakiness; replace each pair of await nextTick(); await nextTick() in the test
cases ("updates displayed items when items prop changes", "updates when items
change but IDs stay the same", and "updates when switching between empty and
non-empty items") with a single await flushPromises() and add an import for
flushPromises from 'flush-promises' at the top of the test file; update
occurrences around the mountDropdown/getMenuItems/createItem interactions so the
assertions run after promise resolution.
In
`@src/renderer/extensions/vueNodes/widgets/components/form/dropdown/FormDropdown.vue`:
- Around line 107-114: The computedAsync block that defines filteredItems awaits
the public async prop searcher without handling rejections; wrap the searcher
call inside a try-catch in the filteredItems computedAsync callback (preserving
onCancel and cleanupFn behavior) and on error call the app error toast (either
via useErrorHandling().toastErrorHandler or toastStore.add(t('...'))) to show a
user-facing, localized message using t(), then return an empty/fallback result
so the dropdown doesn't fail silently; update references to searcher,
filteredItems, onCancel and cleanupFn accordingly.
---
Nitpick comments:
In `@src/renderer/extensions/vueNodes/widgets/composables/useComboWidget.test.ts`:
- Around line 338-353: Update the test for useComboWidget to be a live-update
regression: after constructing the widget via constructor(mockNode, inputSpec)
(and for a second case with inputSpec.remote = true), mutate
mockAssetsStoreState.inputAssets and then assert that widget.options.values
reflects the updated assets (verifying dynamic binding via
bindDynamicValuesOption); keep the original identity assertion but replace
static identity-only verification with the mutation + expectation for both the
local options path and the inputSpec.remote path so the test proves live updates
rather than snapshot/mocked identity.
In `@src/renderer/extensions/vueNodes/widgets/composables/useComboWidget.ts`:
- Around line 47-67: The current bindDynamicValuesOption redefines
options.values asymmetrically (setter accepts unknown[] and preserves only
arrays while getter may return any non-null getValues() result); to fix, capture
the original descriptor via Object.getOwnPropertyDescriptor(options, 'values')
before redefining and either (a) delegate to the original getter/setter when
non-array or function values are encountered, or (b) normalize non-array sets
into a stable array fallback (e.g., wrap single values or ignore non-arrays
consistently) so setter/getter behavior is symmetric; update
bindDynamicValuesOption (and ensure compatibility with ComboWidget and
useRemoteWidget) to use the preserved descriptor or normalization approach.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d7894fee-3a92-4b97-8436-59952885d899
📒 Files selected for processing (6)
src/renderer/extensions/vueNodes/widgets/components/form/dropdown/FormDropdown.test.tssrc/renderer/extensions/vueNodes/widgets/components/form/dropdown/FormDropdown.vuesrc/renderer/extensions/vueNodes/widgets/components/form/dropdown/FormDropdownMenu.vuesrc/renderer/extensions/vueNodes/widgets/components/form/dropdown/FormDropdownMenuActions.vuesrc/renderer/extensions/vueNodes/widgets/composables/useComboWidget.test.tssrc/renderer/extensions/vueNodes/widgets/composables/useComboWidget.ts
💤 Files with no reviewable changes (1)
- src/renderer/extensions/vueNodes/widgets/components/form/dropdown/FormDropdownMenuActions.vue
src/renderer/extensions/vueNodes/widgets/components/form/dropdown/FormDropdown.test.ts
Show resolved
Hide resolved
src/renderer/extensions/vueNodes/widgets/components/form/dropdown/FormDropdown.vue
Show resolved
Hide resolved
computedAsync produces promises that nextTick alone does not reliably drain. Use flushPromises from @vue/test-utils to properly settle the async computed before asserting. Amp-Thread-ID: https://ampcode.com/threads/T-019cd3ff-0573-71c3-8e8f-0bb8e52870cb Co-authored-by: Amp <amp@ampcode.com>
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/renderer/extensions/vueNodes/widgets/components/form/dropdown/FormDropdown.test.ts (1)
10-12: Consider usingsatisfiesfor explicit shape validation.Per test helper conventions,
satisfiesat the return site provides shape validation at the point of construction rather than relying on the function signature.♻️ Optional refinement
function createItem(id: string, name: string): FormDropdownItem { - return { id, preview_url: '', name, label: name } + return { id, preview_url: '', name, label: name } satisfies FormDropdownItem }Based on learnings: "In test files matching **/*.test.ts under src, when creating test helper functions that construct mock objects implementing an interface, prefer using satisfies InterfaceType for shape validation."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/extensions/vueNodes/widgets/components/form/dropdown/FormDropdown.test.ts` around lines 10 - 12, The createItem helper returns a FormDropdownItem but currently relies on the function signature for shape validation; update the return expression to use TypeScript's `satisfies FormDropdownItem` at the construction site (the object literal returned in createItem) so the literal is validated against FormDropdownItem without changing the function signature—locate the createItem function and append the `satisfies FormDropdownItem` assertion to the returned object literal to enforce shape validation in the test helper.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@src/renderer/extensions/vueNodes/widgets/components/form/dropdown/FormDropdown.test.ts`:
- Around line 10-12: The createItem helper returns a FormDropdownItem but
currently relies on the function signature for shape validation; update the
return expression to use TypeScript's `satisfies FormDropdownItem` at the
construction site (the object literal returned in createItem) so the literal is
validated against FormDropdownItem without changing the function
signature—locate the createItem function and append the `satisfies
FormDropdownItem` assertion to the returned object literal to enforce shape
validation in the test helper.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: b0626d5f-eeae-4586-bf07-1c941a6831e1
📒 Files selected for processing (1)
src/renderer/extensions/vueNodes/widgets/components/form/dropdown/FormDropdown.test.ts
AustinMroz
approved these changes
Mar 9, 2026
Collaborator
AustinMroz
left a comment
AustinMroz
left a comment
There was a problem hiding this comment.
Some potential overlap with #8775?
Changes to searcher seem very healthy.
Contributor
Author
Very probably |
DrJKL
added
needs-backport
github-actions bot
pushed a commit
that referenced
this pull request
Mar 9, 2026
Main targeted, built on #9551 ## Summary Fix Load Image/Load Video input dropdown tabs not showing available input assets in Vue node select dropdown. ## Changes - **What**: Keep combo widget `options` object identity while exposing dynamic `values` for cloud/remote combos. - **What**: Remove temporary debug logging and restore clearer dropdown filter branching. - **What**: Remove stale `searcher`/`updateKey` prop plumbing in dropdown menu/actions and update related tests. ## Review Focus Verify `Load Image` / `Load Video` Inputs tab behavior and confirm cloud/remote combo option values still update correctly. Relates to #9551 ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-9670-fix-show-load-widget-inputs-in-media-dropdown-31e6d73d36508148b845e18268a60c2a) by [Unito](https://www.unito.io) --------- Co-authored-by: bymyself <cbyrne@comfy.org> Co-authored-by: Amp <amp@ampcode.com>
Member
github-actions
bot
removed
the
needs-backport
DrJKL
added
core/1.40
DrJKL
added a commit
that referenced
this pull request
Mar 10, 2026
Main targeted, built on #9551 ## Summary Fix Load Image/Load Video input dropdown tabs not showing available input assets in Vue node select dropdown. ## Changes - **What**: Keep combo widget `options` object identity while exposing dynamic `values` for cloud/remote combos. - **What**: Remove temporary debug logging and restore clearer dropdown filter branching. - **What**: Remove stale `searcher`/`updateKey` prop plumbing in dropdown menu/actions and update related tests. ## Review Focus Verify `Load Image` / `Load Video` Inputs tab behavior and confirm cloud/remote combo option values still update correctly. Relates to #9551 ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-9670-fix-show-load-widget-inputs-in-media-dropdown-31e6d73d36508148b845e18268a60c2a) by [Unito](https://www.unito.io) --------- Co-authored-by: bymyself <cbyrne@comfy.org> Co-authored-by: Amp <amp@ampcode.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
5 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.
Main targeted, built on #9551
Summary
Fix Load Image/Load Video input dropdown tabs not showing available input assets in Vue node select dropdown.
Changes
optionsobject identity while exposing dynamicvaluesfor cloud/remote combos.searcher/updateKeyprop plumbing in dropdown menu/actions and update related tests.Review Focus
Verify
Load Image/Load VideoInputs tab behavior and confirm cloud/remote combo option values still update correctly.Relates to #9551
┆Issue is synchronized with this Notion page by Unito