Comfy-Org / Comfy-Org/ComfyUI_frontend
A promoted subgraph widget set to null is dropped on save
- Dominant language
- TypeScript
- Stars
- 2k
- Forks
- 704
- Avg merge
- 1d 8h
- Merged PRs (30d)
- 512
Description
## Summary
A promoted subgraph widget whose value is `null` is dropped by `SubgraphNode.serialize()`. When it is the only promoted value on the node, `widgets_values` is deleted from the serialized node entirely and the value is lost.
Present on `main`. Not introduced by the ECS branch — but the ECS work makes `widgetValueStore` the authority for exactly this value, so it is worth resolving before that lands.
## Root cause
`isWidgetValue` treats `undefined` as a valid widget value and `null` as invalid:
```ts
// src/lib/litegraph/src/types/widgets.ts:434
export function isWidgetValue(value: unknown): value is TWidgetValue {
if (value === undefined) return true
if (typeof value === 'string') return true
if (typeof value === 'number') return true
if (typeof value === 'boolean') return true
return value !== null && typeof value === 'object'
}
```
`SubgraphNode.serialize()` then uses it as a filter:
```ts
// src/lib/litegraph/src/subgraph/SubgraphNode.ts:951
const widgetValues = this.inputs.flatMap((input) => {
if (!input.widgetId) return []
const value = useWidgetValueStore().getWidget(input.widgetId)?.value
return [isWidgetValue(value) ? value : undefined]
})
if (widgetValues.some((value) => value !== undefined)) {
serialized.widgets_values = widgetValues
} else {
delete serialized.widgets_values
}
```
So `null` becomes `undefined`, and an all-`null` set deletes the key.
## Why it usually looks fine
Restore is asymmetric and more permissive: `_applyPromotedWidgetValues` guards on `value !== undefined`, so it *does* accept `null` on the way back in.
And a file with a mixed set round-trips **by accident** — the in-memory `undefined` hole becomes `null` when the array is JSON-stringified. So `[, 42]` is written as `[null, 42]` and reads back as `null`. It looks correct because JSON has no `undefined`.
The accident does not save the single-value case, where the key is removed before serialization.
## Impact
`0` is falsy and serializes correctly as `[0]`. `null` does not. The difference matters because absence and `null` mean different things on reload, and this is asserted directly in the new tests:
| Saved | Restores as |
| --- | --- |
| `[null, 42]` | `null` — correct |
| `[42]`, entry genuinely missing | interior widget default `5` |
So a lost `null` does not restore as `null`; it silently becomes the interior default. For a user, a promoted control they had deliberately cleared comes back populated.
## Reproduction
`src/core/graph/subgraph/promotionAfterReplacement.test.ts` on #15550 covers this — 6 tests, currently asserting the behaviour as-is rather than red. The relevant ones:
- single promoted `null` → `widgets_values` key absent, contrasted against `0` serializing as `[0]`
- `[null, 42]` restores `null`; truncated `[42]` falls back to the interior default
A grep across `SubgraphWidgetPromotion.test.ts`, `promotionUtils.test.ts` and `proxyWidgetMigration.test.ts` found **zero** existing assertions on a `null` widget value, which is why this has not surfaced.
## Suggested fix
Narrowly: accept `null` in `isWidgetValue`, or stop using it as the serialization filter and check for `undefined` explicitly. The second is probably right — `isWidgetValue` is a type guard being used as a validity filter, and those are different questions.
Either way the two halves should agree. Restore already accepts `null`; serialize should too.
## Context
Found while writing QA-7 coverage ahead of #14246. The subgraph promotion docs are explicit that `null` is a value rather than absence, so the serializer disagreeing with that is worth closing regardless of the migration.
Contributor guide
Assessment
This issue has not been assessed yet.