Comfy-Org / Comfy-Org/ComfyUI_frontend

ECS branch: widget.options whole-value assignment is discarded, and the branch removed the render-time merge that used to mask it

Open
#15,632 0 comments 0 reactions 1 assignee Claimed by @DrJKL View on GitHub
area:widgets Potential Bug Public API
Dominant language
TypeScript
Stars
2k
Forks
699
Avg merge
1d 7h
Merged PRs (30d)
490

Description

`BaseWidget.options` is a plain field whose value is *also* seeded into the store's `WidgetState`. In-place mutation is honoured because both sides share one object reference; whole-value assignment is discarded because it only rebinds the class field. The branch made the discarded case worse by removing the render-time merge.

Reviewed at `5002fae1b12d44831a21367afa7c0f798f7e7a2c` (PR #14246), merge base `6532665db947acb61ed044fe91a1d4fe1fb84c8b`.

## Mechanism

- `BaseWidget.options` is a plain field — `src/lib/litegraph/src/widgets/BaseWidget.ts:81`.
- `_state.options` is seeded with the **same object reference** at `BaseWidget.ts:207-215`.

So:

- `widget.options.values.push(x)` mutates the object the store hands out and is visible at `src/renderer/extensions/vueNodes/composables/useProcessedWidgets.ts:367`. **Honoured.**
- `widget.options = {...}` rebinds only the class field. `_state.options` keeps the old object. **Discarded.**

That is the asymmetry: mutate-in-place works, replace-wholesale does not.

## The branch narrowed it from partial to total

On `main` the render path merged both:

```ts
// main:src/renderer/extensions/vueNodes/composables/useProcessedWidgets.ts:271-274
{ ...widget.options, ...widgetState.options }
```

so keys added by a whole-value assignment still surfaced. On the branch it is:

```ts
// useProcessedWidgets.ts:367
{ ...(widgetState.options ?? {}) }
```

The live object is now **entirely ignored**. This is the behaviour change; the underlying plain-field/store split predates the branch.

## Second divergence, same field

`registerWidget`'s existing-state branch (`src/stores/widgetValueStore.ts:105-109`) returns the previously registered state **without refreshing `options`**, so a re-created widget adopts stale options. Pre-existing (`main:widgetValueStore.ts:39-42` is the same), but the loss of the merge above makes it user-visible for the first time.

## Reachability, measured

Corpus: 29 custom-node packs on one host, 157 frontend source files (`.js`/`.ts` under `*/web/*`, excluding minified bundles, `litegraph.d.ts` and `node_modules`). Control `/registerExtension/` matched **84 files**.

- `/\.options\s*=[^=]/` — **5 files, 7 sites, 1 pack**
- `/options\.values\s*=[^=]/` — **3 files, 4 sites, 2 packs**:
- `rgthree-comfy/web/comfyui/base_power_prompt.js:172` — `this.combos[key].options.values = values`
- `ComfyUI-Custom-Scripts/web/js/betterCombos.js:274,282`
- `ComfyUI-Custom-Scripts/web/js/presetText.js:147`

Note these particular four sites assign `options.values` (a nested key on the shared object), not `options` wholesale, so they take the honoured path. The `\.options\s*=` hits are the ones at risk; I have not traced each of the 7 to a user gesture, so **treat the reachability of the wholesale-assignment case as established-in-principle but not path-confirmed.**

Separate, pre-existing, and not a branch regression: `bindDynamicValuesOption` (`src/composables/widgets/useComboWidget.ts:49-70`) makes `options.values` a getter over `getValues()` whose setter only writes `fallbackValues`, consulted **only** when `getValues()` returns null/undefined. So `widget.options.values = [...]` is silently discarded whenever the remote/asset source is live. Byte-identical to `main`. Same class, different vintage — noting it so the umbrella rule covers it.

Also worth recording: `options.values.push(...)` mutates the raw object, so no reactive effect fires and Vue will not re-render on its own. Unverified whether any consumer depends on that.

## Suggested fix

Give `options` a getter/setter pair over `_state.options` so both write forms commit, or restore the render-time merge. Either makes it fully honoured.

Part of a branch-wide audit of compatibility-accessor asymmetry — see the umbrella issue. Related: #15620, #15618, #15577, #15594.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.