[feat] Add containerNode property for DOM widget positioning in subgraphs (#1128)

Co-authored-by: Claude <[email protected]>

Author: christian-byrne

CLAUDE.md

@@ -24,3 +24,39 @@
 
 - Be sure to typecheck when you’re done making a series of code changes
 - Prefer running single tests, and not the whole test suite, for performance
+
+# Testing Guidelines
+
+## Avoiding Circular Dependencies in Tests
+
+**CRITICAL**: When writing tests for subgraph-related code, always import from the barrel export to avoid circular dependency issues:
+
+```typescript
+// ✅ CORRECT - Use barrel import
+import { LGraph, Subgraph, SubgraphNode } from "@/litegraph"
+
+// ❌ WRONG - Direct imports cause circular dependency
+import { LGraph } from "@/LGraph"
+import { Subgraph } from "@/subgraph/Subgraph" 
+import { SubgraphNode } from "@/subgraph/SubgraphNode"
+```
+
+**Root cause**: `LGraph` and `Subgraph` have a circular dependency:
+- `LGraph.ts` imports `Subgraph` (creates instances with `new Subgraph()`)
+- `Subgraph.ts` extends `LGraph` 
+
+The barrel export (`@/litegraph`) handles this properly, but direct imports cause module loading failures.
+
+## Test Setup for Subgraphs
+
+Use the provided test helpers for consistent setup:
+
+```typescript
+import { createTestSubgraph, createTestSubgraphNode } from "./fixtures/subgraphHelpers"
+
+function createTestSetup() {
+  const subgraph = createTestSubgraph()
+  const subgraphNode = createTestSubgraphNode(subgraph)
+  return { subgraph, subgraphNode }
+}
+```

src/infrastructure/SubgraphEventMap.ts

@@ -1,6 +1,8 @@
 import type { LGraphEventMap } from "./LGraphEventMap"
 import type { SubgraphInput } from "@/subgraph/SubgraphInput"
+import type { SubgraphNode } from "@/subgraph/SubgraphNode"
 import type { SubgraphOutput } from "@/subgraph/SubgraphOutput"
+import type { IBaseWidget } from "@/types/widgets"
 
 export interface SubgraphEventMap extends LGraphEventMap {
   "adding-input": {
@@ -40,4 +42,13 @@ export interface SubgraphEventMap extends LGraphEventMap {
     oldName: string
     newName: string
   }
+
+  "widget-promoted": {
+    widget: IBaseWidget
+    subgraphNode: SubgraphNode
+  }
+  "widget-unpromoted": {
+    widget: IBaseWidget
+    subgraphNode: SubgraphNode
+  }
 }

src/subgraph/SubgraphNode.ts

@@ -187,8 +187,13 @@ export class SubgraphNode extends LGraphNode implements BaseLGraph {
   }
 
   #setWidget(subgraphInput: Readonly<SubgraphInput>, input: INodeInputSlot, widget: Readonly<IBaseWidget>) {
-    // Use the first matching widget
-    const promotedWidget = toConcreteWidget(widget, this).createCopyForNode(this)
+    const concreteWidget = toConcreteWidget(widget, this)
+
+    const promotedWidget = concreteWidget.createCopyForNode(this)
+
+    // Set parentSubgraphNode for all promoted widgets to track their origin
+    promotedWidget.parentSubgraphNode = this
+
     Object.assign(promotedWidget, {
       get name() {
         return subgraphInput.name
@@ -212,6 +217,9 @@ export class SubgraphNode extends LGraphNode implements BaseLGraph {
 
     this.widgets.push(promotedWidget)
 
+    // Dispatch widget-promoted event
+    this.subgraph.events.dispatch("widget-promoted", { widget: promotedWidget, subgraphNode: this })
+
     input.widget = { name: subgraphInput.name }
     input._widget = promotedWidget
   }
@@ -307,7 +315,28 @@ export class SubgraphNode extends LGraphNode implements BaseLGraph {
     return nodes
   }
 
+  override removeWidgetByName(name: string): void {
+    const widget = this.widgets.find(w => w.name === name)
+    if (widget) {
+      this.subgraph.events.dispatch("widget-unpromoted", { widget, subgraphNode: this })
+    }
+    super.removeWidgetByName(name)
+  }
+
+  override ensureWidgetRemoved(widget: IBaseWidget): void {
+    if (this.widgets.includes(widget)) {
+      this.subgraph.events.dispatch("widget-unpromoted", { widget, subgraphNode: this })
+    }
+    super.ensureWidgetRemoved(widget)
+  }
+
   override onRemoved(): void {
+    // Clean up all promoted widgets
+    for (const widget of this.widgets) {
+      widget.parentSubgraphNode = undefined
+      this.subgraph.events.dispatch("widget-unpromoted", { widget, subgraphNode: this })
+    }
+
     for (const input of this.inputs) {
       input._listenerController?.abort()
     }

src/types/widgets.ts

@@ -201,6 +201,12 @@ export interface IBaseWidget<
 
   tooltip?: string
 
+  /**
+   * Reference to the subgraph container node when this widget is promoted from a subgraph.
+   * This allows the widget to know which SubgraphNode it belongs to in the parent graph.
+   */
+  parentSubgraphNode?: LGraphNode
+
   // TODO: Confirm this format
   callback?(
     value: any,

src/widgets/BaseWidget.ts

@@ -56,6 +56,12 @@ export abstract class BaseWidget<TWidget extends IBaseWidget = IBaseWidget> impl
     return this.#node
   }
 
+  /**
+   * Reference to the subgraph container node when this widget is promoted from a subgraph.
+   * This allows the widget to know which SubgraphNode it belongs to in the parent graph.
+   */
+  parentSubgraphNode?: LGraphNode
+
   linkedWidgets?: IBaseWidget[]
   name: string
   options: TWidget["options"]
@@ -297,4 +303,12 @@ export abstract class BaseWidget<TWidget extends IBaseWidget = IBaseWidget> impl
     cloned.value = this.value
     return cloned
   }
+
+  /**
+   * Type guard to check if this widget has a DOM element.
+   * @returns True if the widget has a DOM element attached
+   */
+  isDOMWidget(): this is this & { element: HTMLElement } {
+    return this.element instanceof HTMLElement
+  }
 }

test/subgraph/SubgraphEdgeCases.test.ts

@@ -67,9 +67,9 @@ describe("SubgraphEdgeCases - Recursion Detection", () => {
   it("should respect MAX_NESTED_SUBGRAPHS constant", () => {
     // Verify the constant exists and is a reasonable positive number
     expect(Subgraph.MAX_NESTED_SUBGRAPHS).toBeDefined()
-    expect(typeof Subgraph.MAX_NESTED_SUBGRAPHS).toBe('number')
+    expect(typeof Subgraph.MAX_NESTED_SUBGRAPHS).toBe("number")
     expect(Subgraph.MAX_NESTED_SUBGRAPHS).toBeGreaterThan(0)
-    expect(Subgraph.MAX_NESTED_SUBGRAPHS).toBeLessThanOrEqual(10000) // Reasonable upper bound
+    expect(Subgraph.MAX_NESTED_SUBGRAPHS).toBeLessThanOrEqual(10_000) // Reasonable upper bound
 
     // Note: Currently not enforced in implementation
     // This test documents the intended behavior
@@ -313,7 +313,7 @@ describe("SubgraphEdgeCases - Performance and Scale", () => {
     const flattened = subgraphNode.getInnerNodes(executableNodes)
 
     expect(flattened).toHaveLength(50)
-    
+
     // Performance is acceptable for 50 nodes (typically < 1ms)
   })