From 97c59f3adbbabe7678df9ccebcbcaf92ed708564 Mon Sep 17 00:00:00 2001 From: Christian Byrne Date: Thu, 25 Jun 2026 16:50:20 -0700 Subject: [PATCH] refactor(litegraph): delete dead onGetNodeMenuOptions, deprecate onBeforeChange (#12230) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Stacks on #12228. Part of the LGraph dead-hook cleanup per AUDIT-LG.9. ## What - `LGraph.onGetNodeMenuOptions` — **deleted** (field + dispatcher in `LGraphCanvas.getNodeMenuOptions`). Zero ecosystem consumers. - `LGraph.onBeforeChange` — **deprecated, not deleted**. The field and the dispatch in `LGraph.beforeChange()` are kept, but assigning a handler now emits a one-time `warnDeprecated` nudging migration to `LGraphCanvas.onBeforeChange`. ## Why onBeforeChange is preserved The W2F-1 re-audit found `bmad4ever/ComfyUI-Bmad-DirtyUndoRedo` assigns `app.graph.onBeforeChange = fn` (the listener-assignment pattern). Deleting the field outright would silently turn those handlers into no-ops. Keeping it as a deprecated shim preserves backward compatibility during a grace period while signaling the intended replacement. `onAfterChange` is untouched (so `BennyKok/comfyui-deploy`'s `onAfterChange` wrapper keeps working). `LGraphCanvas.onBeforeChange` remains a separate field, and the canvas dispatch chain `this.canvasAction((c) => c.onBeforeChange?.(this))` is unchanged. ## Tests `LGraph.test.ts` covers the shim: the assigned listener is still invoked, the deprecation warning fires when used, and no warning fires when no listener is assigned. ## Sequencing - Stacks on #12228 - Sequences behind Alex's Phase B (#11939, #11811) --------- Co-authored-by: Connor Byrne Co-authored-by: Claude Opus 4.5 Co-authored-by: Alexander Brown --- src/lib/litegraph/src/LGraph.test.ts | 48 ++++++++++++++++++++++++++- src/lib/litegraph/src/LGraph.ts | 17 ++++++---- src/lib/litegraph/src/LGraphCanvas.ts | 2 -- 3 files changed, 58 insertions(+), 9 deletions(-) diff --git a/src/lib/litegraph/src/LGraph.test.ts b/src/lib/litegraph/src/LGraph.test.ts index 21d3b31dca..b120023a2d 100644 --- a/src/lib/litegraph/src/LGraph.test.ts +++ b/src/lib/litegraph/src/LGraph.test.ts @@ -1,6 +1,6 @@ import { createTestingPinia } from '@pinia/testing' import { setActivePinia } from 'pinia' -import { beforeEach, describe, expect, it, vi } from 'vitest' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import type { NodeId, Subgraph } from '@/lib/litegraph/src/litegraph' import { @@ -529,6 +529,52 @@ describe('Subgraph Definition Garbage Collection', () => { }) }) +describe('beforeChange deprecated onBeforeChange shim', () => { + beforeEach(() => { + LiteGraph.onDeprecationWarning = [] + LiteGraph.alwaysRepeatWarnings = true + }) + + afterEach(() => { + LiteGraph.alwaysRepeatWarnings = false + }) + + it('still invokes a listener assigned to onBeforeChange', () => { + const graph = new LGraph() + const node = new LGraphNode('test') + const onBeforeChange = vi.fn() + graph.onBeforeChange = onBeforeChange + + graph.beforeChange(node) + + expect(onBeforeChange).toHaveBeenCalledWith(graph, node) + }) + + it('warns that onBeforeChange is deprecated when used', () => { + const graph = new LGraph() + const deprecationCallback = vi.fn() + LiteGraph.onDeprecationWarning = [deprecationCallback] + graph.onBeforeChange = vi.fn() + + graph.beforeChange() + + expect(deprecationCallback).toHaveBeenCalledWith( + expect.stringContaining('LGraph.onBeforeChange is deprecated'), + undefined + ) + }) + + it('does not warn when no listener is assigned', () => { + const graph = new LGraph() + const deprecationCallback = vi.fn() + LiteGraph.onDeprecationWarning = [deprecationCallback] + + graph.beforeChange() + + expect(deprecationCallback).not.toHaveBeenCalled() + }) +}) + describe('Legacy LGraph Compatibility Layer', () => { test('can be extended via prototype', ({ expect, minimalGraph }) => { // @ts-expect-error Should always be an error. diff --git a/src/lib/litegraph/src/LGraph.ts b/src/lib/litegraph/src/LGraph.ts index ae8413873a..cc4e1d68be 100644 --- a/src/lib/litegraph/src/LGraph.ts +++ b/src/lib/litegraph/src/LGraph.ts @@ -38,7 +38,6 @@ import type { DefaultConnectionColors, Dictionary, HasBoundingRect, - IContextMenuValue, INodeInputSlot, INodeOutputSlot, LinkNetwork, @@ -56,6 +55,7 @@ import { createBounds, snapPoint } from './measure' +import { warnDeprecated } from './utils/feedback' import { SubgraphInput } from './subgraph/SubgraphInput' import { SubgraphInputNode } from './subgraph/SubgraphInputNode' import { SubgraphOutput } from './subgraph/SubgraphOutput' @@ -331,16 +331,16 @@ export class LGraph onNodeAdded?(node: LGraphNode): void onNodeRemoved?(node: LGraphNode): void onTrigger?: LGraphTriggerHandler + /** + * @deprecated Assign a listener to {@link LGraphCanvas.onBeforeChange} instead. + * This graph-level hook will be removed in a future version. + */ onBeforeChange?(graph: LGraph, info?: LGraphNode): void onAfterChange?(graph: LGraph, info?: LGraphNode | null): void onConnectionChange?(node: LGraphNode): void on_change?(graph: LGraph): void onSerialize?(data: ISerialisedGraph | SerialisableGraph): void onConfigure?(data: ISerialisedGraph | SerialisableGraph): void - onGetNodeMenuOptions?( - options: (IContextMenuValue | null)[], - node: LGraphNode - ): void // @ts-expect-error - Private property type needs fixing private _input_nodes?: LGraphNode[] @@ -1357,7 +1357,12 @@ export class LGraph // used for undo, called before any change is made to the graph beforeChange(info?: LGraphNode): void { - this.onBeforeChange?.(this, info) + if (this.onBeforeChange) { + warnDeprecated( + 'LGraph.onBeforeChange is deprecated and will be removed in a future version. Assign a listener to LGraphCanvas.onBeforeChange instead.' + ) + this.onBeforeChange(this, info) + } this.canvasAction((c) => c.onBeforeChange?.(this)) } diff --git a/src/lib/litegraph/src/LGraphCanvas.ts b/src/lib/litegraph/src/LGraphCanvas.ts index 6269f0f422..9dfc53b250 100644 --- a/src/lib/litegraph/src/LGraphCanvas.ts +++ b/src/lib/litegraph/src/LGraphCanvas.ts @@ -8642,8 +8642,6 @@ export class LGraphCanvas implements CustomEventDispatcher callback: LGraphCanvas.onMenuNodeRemove }) - node.graph?.onGetNodeMenuOptions?.(options, node) - return options }