mirror of
https://github.com/Comfy-Org/ComfyUI_frontend.git
synced 2026-04-26 01:09:46 +00:00
docs: document PrimitiveNode copy/paste semantics and widgets_values loss (#9119)
## Summary Documents the PrimitiveNode copy/paste bug mechanism and connection lifecycle semantics in `WIDGET_SERIALIZATION.md`. This is tribal knowledge from debugging [#1757](https://github.com/Comfy-Org/ComfyUI_frontend/issues/1757) and the related [Slack discussion](https://comfy-organization.slack.com/archives/C09AQRB49QX/p1771806268469089). ## What's documented - **The clone→serialize gap**: `_serializeItems()` calls `item.clone()?.serialize()`. The clone has no `this.widgets` (PrimitiveNode creates them on connection), so `serialize()` silently drops `widgets_values`. - **Why seed survives but control_after_generate doesn't**: Primary widget value is copied from the target on reconnect; secondary widgets read from `this.widgets_values` which was lost. - **Current vs. proposed lifecycle**: Empty-on-copy → morph-on-connect (current) vs. clone-configured-instance → empty-on-disconnect (proposed). - **Design considerations**: `input.widget` override flexibility, deserialization ordering, and the minimal `serialize()` override fix. ## Related - Issue: #1757 - Fix PR: #8938 - Companion: #9102 (initial WIDGET_SERIALIZATION.md), #9105 (type/JSDoc improvements) - Notion: COM-15282 ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-9119-docs-document-PrimitiveNode-copy-paste-semantics-and-widgets_values-loss-3106d73d3650816ba7f7d9e6f3bb3868) by [Unito](https://www.unito.io)
This commit is contained in:
45
docs/adr/0006-primitive-node-copy-paste-lifecycle.md
Normal file
45
docs/adr/0006-primitive-node-copy-paste-lifecycle.md
Normal file
@@ -0,0 +1,45 @@
|
||||
# 6. PrimitiveNode Copy/Paste Lifecycle
|
||||
|
||||
Date: 2026-02-22
|
||||
|
||||
## Status
|
||||
|
||||
Proposed
|
||||
|
||||
## Context
|
||||
|
||||
PrimitiveNode creates widgets dynamically on connection. When copied, the clone has no `this.widgets`, so `LGraphNode.serialize()` drops `widgets_values` from the clipboard data. This causes secondary widget values (e.g., `control_after_generate`) to be lost on paste. See [WIDGET_SERIALIZATION.md](../WIDGET_SERIALIZATION.md#primitiveno-and-copypaste) for the full mechanism.
|
||||
|
||||
Related: [#1757](https://github.com/Comfy-Org/ComfyUI_frontend/issues/1757), [#8938](https://github.com/Comfy-Org/ComfyUI_frontend/pull/8938)
|
||||
|
||||
## Options
|
||||
|
||||
### A. Minimal fix: override `serialize()` on PrimitiveNode
|
||||
|
||||
Override `serialize()` to fall back to `this.widgets_values` (set during `configure()`) when the base implementation omits it due to missing `this.widgets`.
|
||||
|
||||
- **Pro**: No change to connection lifecycle semantics. Lowest risk.
|
||||
- **Pro**: Doesn't affect workflow save/load (which already works via `onAfterGraphConfigured`).
|
||||
- **Con**: Doesn't address the deeper design issue — primitives are still empty on copy.
|
||||
|
||||
### B. Clone-configured-instance lifecycle
|
||||
|
||||
On copy, the primitive is a clone of the configured instance (with widgets intact). On disconnect or paste without connections, it returns to empty state.
|
||||
|
||||
- **Pro**: Copy→serialize captures `widgets_values` correctly. Matches OOP expectations.
|
||||
- **Pro**: Secondary widget state survives round-trips without special-casing.
|
||||
- **Con**: `input.widget[CONFIG]` allows extensions to make PrimitiveNode create a _different_ widget than the target. Widget config is derived at connection time, not stored, so cloning the configured state may not be faithful.
|
||||
- **Con**: Deserialization ordering — `configure()` runs before links are restored. PrimitiveNode needs links to know what widgets to create. `onAfterGraphConfigured()` handles this for workflow load, but copy/paste uses a different code path.
|
||||
- **Con**: Higher risk of regressions in extension compatibility.
|
||||
|
||||
### C. Projection model (like Subgraph widgets)
|
||||
|
||||
Primitives act as a synchronization mechanism — no own state, just a projection of the target widget's resolved value.
|
||||
|
||||
- **Pro**: Cleanest conceptual model. Eliminates state duplication.
|
||||
- **Con**: Primitives can connect to multiple targets. Projection with multiple targets is ambiguous.
|
||||
- **Con**: Major architectural change with broad impact.
|
||||
|
||||
## Decision
|
||||
|
||||
Pending. Option A is the most pragmatic first step. Option B can be revisited after Option A ships and stabilizes.
|
||||
Reference in New Issue
Block a user