Comfy-Org / Comfy-Org/ComfyUI_frontend

A promoted subgraph widget set to null is dropped on save

Open
#15,551 6 comments 0 reactions 1 assignee Claimed by @christian-byrne View on GitHub
area:subgraph Bug claimed Potential Bug
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

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.