Comfy-Org / Comfy-Org/ComfyUI_frontend
refactor(litegraph): add deprecation warning for direct graph._version writes
- Dominant language
- TypeScript
- Stars
- 2k
- Forks
- 699
- Avg merge
- 1d 7h
- Merged PRs (30d)
- 490
Description
## Summary
Follow-up to #11698 (ECS Migration Phase 0a). That PR centralized internal `LGraph._version` increments behind `incrementVersion()`. This issue tracks adding a deprecation warning for **external** writes to `graph._version` from the custom-node ecosystem, so authors migrate to `incrementVersion()` ahead of the future `VersionSystem` (ECS).
Per AGENTS.md ADR 0008, changes that affect 40+ custom node repos require migration guidance. A dedup'd deprecation warning is the right tool to deliver that guidance without breaking existing nodes.
## Proposed Implementation
Replace the public `_version: number` field on [`LGraph`](https://github.com/Comfy-Org/ComfyUI_frontend/blob/main/src/lib/litegraph/src/LGraph.ts) with a getter/setter pair plus a private backing field. The setter is **write-through** (zero behavior change) and emits a deduped warning via the existing [`warnDeprecated`](https://github.com/Comfy-Org/ComfyUI_frontend/blob/main/src/lib/litegraph/src/utils/feedback.ts) helper.
```ts
private __version = -1
get _version(): number {
return this.__version
}
set _version(v: number) {
warnDeprecated(
'Direct write to graph._version is deprecated. Use graph.incrementVersion() instead.',
this
)
this.__version = v // write-through: existing behavior preserved
}
incrementVersion(): void {
this.__version++
}
// Internal resets (e.g. clear()) write the backing field directly.
clear() {
// ...
this.__version = -1
}
```
### Key properties
- **Reads stay silent.** `LGraphCanvas.renderInfo()` and any custom node read paths don't trigger warnings.
- **Write-through, not redirect.** `graph._version++` and `graph._version = N` still do exactly what they did before — only the warning is new.
- **Dedup'd.** [`warnDeprecated`](https://github.com/Comfy-Org/ComfyUI_frontend/blob/main/src/lib/litegraph/src/utils/feedback.ts#L13-L27) emits each unique message at most once per session.
- **Internal call sites unaffected.** All in-tree writes already go through `incrementVersion()` after #11698; `clear()`'s `-1` reset uses the backing field.
- **Forward-compatible with ECS.** When `VersionSystem` lands, the setter is removed (or replaced with a thrown error) and `_version` becomes a getter that delegates to the World. Custom nodes will have had several versions of warnings to migrate.
## Alternatives Considered
- **Setter that redirects to `incrementVersion()`** (ignoring the assigned value). Rejected — silently changes behavior for nodes doing snapshot/restore.
- **ESLint `no-restricted-syntax` rule** on `_version` writes outside `LGraph.ts`. Useful internally but doesn't reach the custom-node ecosystem, which is the actual drift surface.
- **Do nothing.** The remaining drift surface is bounded and the ECS migration will eventually replace `_version` regardless. Acceptable but loses a cheap migration signal.
## Acceptance Criteria
- [ ] `LGraph._version` is implemented as a getter/setter over a private `__version` backing field
- [ ] Setter emits a deduped `warnDeprecated` message naming `incrementVersion()` as the replacement
- [ ] All in-tree writes that should not warn (`clear()` reset) use the backing field directly
- [ ] Reads do not emit warnings
- [ ] Unit test: writing `graph._version = N` triggers `LiteGraph.onDeprecationWarning`
- [ ] Unit test: calling `graph.incrementVersion()` does not trigger the warning
- [ ] Unit test: write-through preserves the assigned value (`graph._version = 5; expect(graph._version).toBe(5)`)
## Context
- Predecessor PR: #11698 (centralizes internal increments)
- Reviewer suggestion: https://github.com/Comfy-Org/ComfyUI_frontend/pull/11698#pullrequestreview-4184690993
- Linear: FE-165 (ECS Migration Phase 0a)
- Related: `docs/architecture/ecs-migration-plan.md`
┆Issue is synchronized with this [Notion page](https://www.notion.so/Issue-11701-refactor-litegraph-add-deprecation-warning-for-direct-graph-_version-writes-34f6d73d365081259521f5274677d6fd) by [Unito](https://www.unito.io)
Contributor guide
Assessment
This issue has not been assessed yet.