From 7b274b74f15edf696aafa15fb4571301934e35a1 Mon Sep 17 00:00:00 2001 From: Godwin Iheuwa Date: Sun, 11 Jan 2026 12:41:08 +0530 Subject: [PATCH] fix: add beforeChange/afterChange to convertToSubgraph for proper undo (#7791) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary - Adds missing `beforeChange()` and `afterChange()` lifecycle calls to `convertToSubgraph()` method ## Problem When converting nodes to a subgraph and then pressing Ctrl+Z to undo, the node positions were being changed from their original locations instead of being properly restored. ## Root Cause The `convertToSubgraph()` method in `LGraph.ts` was missing the `beforeChange()` and `afterChange()` lifecycle calls that are needed for proper undo/redo state tracking. These calls record the graph state before and after modifications. The inverse operation `unpackSubgraph()` already has these calls (see line 1742), so this is simply matching the existing pattern. ## Solution Add `beforeChange()` at the start of the method (after validation) and `afterChange()` before the return. ## Testing 1. Create a workflow with several nodes positioned in specific locations 2. Select 2-3 nodes 3. Right-click → "Convert Selection to Subgraph" 4. Press Ctrl+Z to undo 5. Verify nodes return to their exact original positions Fixes comfyanonymous/ComfyUI#11514 ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-7791-fix-add-beforeChange-afterChange-to-convertToSubgraph-for-proper-undo-2d86d73d36508125a2c4e4a412cced4a) by [Unito](https://www.unito.io) --------- Co-authored-by: RUiNtheExtinct --- src/lib/litegraph/src/LGraph.ts | 34 +++++++++++++++++++++++++++++++-- 1 file changed, 32 insertions(+), 2 deletions(-) diff --git a/src/lib/litegraph/src/LGraph.ts b/src/lib/litegraph/src/LGraph.ts index b3ad35c99..32494c615 100644 --- a/src/lib/litegraph/src/LGraph.ts +++ b/src/lib/litegraph/src/LGraph.ts @@ -1522,6 +1522,22 @@ export class LGraph } { if (items.size === 0) throw new Error('Cannot convert to subgraph: nothing to convert') + + // Record state before conversion for proper undo support + this.beforeChange() + + try { + return this._convertToSubgraphImpl(items) + } finally { + // Mark state change complete for proper undo support + this.afterChange() + } + } + + private _convertToSubgraphImpl(items: Set): { + subgraph: Subgraph + node: SubgraphNode + } { const { state, revision, config } = this const firstChild = [...items][0] if (items.size === 1 && firstChild instanceof LGraphGroup) { @@ -1728,6 +1744,7 @@ export class LGraph subgraphNode._setConcreteSlots() subgraphNode.arrange() + this.canvasAction((c) => c.canvas.dispatchEvent( new CustomEvent('subgraph-converted', { @@ -1746,9 +1763,23 @@ export class LGraph if (!(subgraphNode instanceof SubgraphNode)) throw new Error('Can only unpack Subgraph Nodes') + // Record state before unpacking for proper undo support + this.beforeChange() + + try { + this._unpackSubgraphImpl(subgraphNode, options) + } finally { + // Mark state change complete for proper undo support + this.afterChange() + } + } + + private _unpackSubgraphImpl( + subgraphNode: SubgraphNode, + options?: { skipMissingNodes?: boolean } + ) { const skipMissingNodes = options?.skipMissingNodes ?? false - this.beforeChange() //NOTE: Create bounds can not be called on positionables directly as the subgraph is not being displayed and boundingRect is not initialized. //NOTE: NODE_TITLE_HEIGHT is explicitly excluded here const positionables = [ @@ -2019,7 +2050,6 @@ export class LGraph } this.canvasAction((c) => c.selectItems(toSelect)) - this.afterChange() } /**