Comfy-Org / Comfy-Org/ComfyUI_frontend

AUDIT-LG: deprecate-then-delete LGraphEventMode.ON_EVENT (and LiteGraph.ON_EVENT reexport)

Open
#12,225 1 comment 0 reactions 0 assignees View on GitHub
documentation Public API
Dominant language
TypeScript
Stars
2k
Forks
699
Avg merge
1d 7h
Merged PRs (30d)
490

Description

**Source audit:** `research/architecture/audit-litegraph-pruning.md (cross-repo workspace)` — see **§AUDIT-LG.4 — `ON_EVENT` verdict** and **§AUDIT-LG.7 — synthesis**.

## Verdict

**DEPRECATE** the *symbol* in release N, **DELETE** in release N+1. **Keep numeric slot `1` for v2 ABI compatibility** — v2 `NodeMode = 0 | 1 | 2 | 3 | 4` is a numeric union, so any extension calling `node.setMode(1)` must keep working.

## Surfaces in this batch (2 surfaces)

| id | kind | symbol | int | ext_rep | tier |
|---|---|---|---:|---:|---|
| `lgraph_event_mode_on_event` | enum_value | `LGraphEventMode.ON_EVENT` | 0 | 11 | top-10 |
| `litegraphglobal_on_event` | reexport | `LiteGraph.ON_EVENT` | 0 | 11 | top-10 |

## Why deprecate, not delete-immediately

Per AUDIT-LG.4:

- **History**: dormant since the 2024 subtree import from `jagenjo/litegraph.js`. Zero PRs added behavioural references after the merge.
- **Semantics**: `node.mode = LGraphEventMode.ON_EVENT` (value `1`) is **functionally a no-op** today — scheduler skips it (only `ALWAYS` executes), action path gates on `ON_TRIGGER` not `ON_EVENT`, bypass/mute paths check `=== NEVER` and `=== BYPASS`, canvas tinting ignores it. The `case LGraphEventMode.ON_EVENT: break` in `changeMode()` is a literal no-op slot in the switch.
- **Internal callers**: 0 functional. Only one `.d.ts` type-level mention.
- **External callers**: 0 functional. The CSV row shows `15/11/top-10` but that is a coarse all-`LGraphEventMode.*` roll-up; targeted `grep ON_EVENT` against the touch-points DB returns 2 hits, both inside one vendored `.d.ts` type union.

The deprecation cycle (rather than direct delete) protects against the unmeasured edge case where a custom-node pack reads the symbolic name (e.g. `if (mode === LiteGraph.ON_EVENT)`) — those reads silently no-op today and would silently throw `undefined` reads after rename without a deprecation warning.

## Replacement

There is none — `ON_EVENT` is not paired with any execution behaviour. Custom nodes that meant to mute a node should use `LGraphEventMode.NEVER` (which has visual tinting + documented mute semantics).

## Action items

**Release N (deprecate):**
- [ ] Rename `LGraphEventMode.ON_EVENT` → `LGraphEventMode.UNUSED_1` (or `_DEPRECATED_ON_EVENT`) in `src/lib/litegraph/src/types/globalEnums.ts:85`.
- [ ] Add a getter on `window.LiteGraph.ON_EVENT` that fires `console.warn('LiteGraph.ON_EVENT is deprecated; the symbol will be removed in release N+1. Numeric slot 1 is preserved for v2 ABI but ON_EVENT is a no-op mode — use LGraphEventMode.NEVER to mute a node.')` then returns `1`.
- [ ] Delete the no-op `case LGraphEventMode.ON_EVENT: break` in `LGraphNode.changeMode()` (or update the switch to widen to `default → return false` for unknown numeric values without losing `setMode(1)` acceptance).

**Release N+1 (delete):**
- [ ] Drop the deprecated symbol entirely.
- [ ] Drop the `window.LiteGraph.ON_EVENT` deprecation getter.

## Open questions for Christian

1. **Confirm the deprecation message copy** — current suggestion above. Acceptable?
2. **Should the warning fire on read or only on write?** Reads (`if (mode === LiteGraph.ON_EVENT)`) are harmless; writes (`node.mode = LiteGraph.ON_EVENT`) are the misuse worth catching. Getter-based warnings fire on both. Cheap to live with for one release.

## Adjacent (already shipped, FYI)

The v2 `NodeMode` doc-comments in `src/extension-api/node.ts:48-61` had wrong numeric mappings — fixed in foundation `#12142` commit `8564a19dc7`. This issue is the LiteGraph-side cleanup of the same root cause.

cc @drjkl @christian-byrne

┆Issue is synchronized with this [Notion page](https://www.notion.so/Issue-12225-AUDIT-LG-deprecate-then-delete-LGraphEventMode-ON_EVENT-and-LiteGraph-ON_EVENT-reex-35f6d73d3650816c9af7f85c3715040c) by [Unito](https://www.unito.io)

Contributor guide

Open the contributing guide

Research direction

Read research/architecture/audit-litegraph-pruning.md sections AUDIT-LG.4 and AUDIT-LG.7, then inspect src/lib/litegraph/src/types/globalEnums.ts:85, LGraphNode.changeMode(), and the LiteGraph global reexport. Confirm the deprecation message and read-versus-write warning decision, preserve numeric slot 1, and verify the release N deprecation and N+1 removal plan.

Written by the indexing model from the issue text.

Assessment

Tech stack
typescript
Domain
api, frontend
Issue type
Refactor
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
48/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.