Comfy-Org / Comfy-Org/ComfyUI_frontend

TWidgetValue cannot express null, but LGraphNode.serialize() writes null into widgets_values

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

Description

## Problem / Goal

`TWidgetValue` cannot express what `LGraphNode.serialize()` writes, so the declared type of `widgets_values` is wrong about its own on-disk payload.

At `origin/main` `296fc5cd07`:

- `src/lib/litegraph/src/types/widgets.ts:432` — `export type TWidgetValue = IWidget['value']`, which resolves to `boolean | number | string | object | undefined`. **No `null`.**
- `src/lib/litegraph/src/types/widgets.ts:434` — `isWidgetValue` returns `false` for `null`, explicitly.
- `src/lib/litegraph/src/types/serialisation.ts:108` — `widgets_values?: TWidgetValue[]`.
- `src/lib/litegraph/src/LGraphNode.ts:1084` — the serialiser writes `val ?? null`, and it assigns at the widget's full index so a skipped widget leaves a hole that `JSON.stringify` renders as `null`.

So `serialize()` emits `null` by two separate routes and neither is representable in the declared element type. The practical symptom: test fixtures that state the real payload need `as unknown as TWidgetValue[]` to compile. Three such casts were added in #15688 because `vue-tsc` rejected the honest version.

This is adjacent to the widget-value null contract settled in #15668, and to #15551 (a promoted subgraph widget set to `null` is dropped on save). A fourth meaning for `null` should not be introduced in that area while the type and the serialiser still disagree about the first three.

## Proposed Solution

Pick one and write it down; do not leave it implicit:

1. **Widen the serialisation type.** `widgets_values?: (TWidgetValue | null)[]` — honest about the payload, and removes the casts. Does not change `TWidgetValue` itself, so widget-value call sites are unaffected. Cheapest.
2. **Stop emitting `null`.** Write `undefined` instead of `val ?? null` and compact the holes. This is an on-disk format change and collides with #15688 / #14262 — out of scope for a typing fix.
3. **Widen `TWidgetValue`.** Largest blast radius; `isWidgetValue` would have to change too, which affects the subgraph promotion path.

Option 1 unless someone wants the format discussion.

## Acceptance Criteria

- [ ] `vue-tsc --noEmit` exits 0 with a fixture that writes the literal payload `[20, null, 12345]` typed as `ISerialisedNode['widgets_values']`, with no `as unknown as` cast.
- [ ] The three casts introduced in #15688 are removed, or the reason they must stay is recorded.
- [ ] No production behaviour change: full unit suite count unchanged.

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.