refactor: replace PromotedDomWidgetAdapter class with Proxy

- Replace class with createPromotedDomWidgetAdapter factory using Proxy - Proxy transparently delegates all properties to the inner widget, eliminating manual forwarding of element, component, inputSpec, props - Extract widgetState getter to deduplicate WidgetValueStore lookups - Move try/catch into resolve() instead of wrapping every call site - Merge drawDisconnectedPlaceholder into draw() to reduce duplication - Remove redundant resolvedType/resolvedOptions getters - Remove redundant resolvedType assertions from tests Amp-Thread-ID: https://ampcode.com/threads/T-019c5543-f50b-77a2-bca6-2549bdc15594 Co-authored-by: Amp <amp@ampcode.com>
2026-04-20 06:20:11 +00:00 · 2026-02-12 20:38:46 -08:00
parent e8af61e25d
commit 1154dec2ff
3 changed files with 93 additions and 276 deletions
--- a/src/core/graph/subgraph/PromotedDomWidgetAdapter.ts
+++ b/src/core/graph/subgraph/PromotedDomWidgetAdapter.ts
@@ -1,195 +1,73 @@
 import type { LGraphNode } from '@/lib/litegraph/src/LGraphNode'
 import type { IBaseWidget } from '@/lib/litegraph/src/types/widgets'
-import type {
-  BaseDOMWidget,
-  ComponentWidget,
-  DOMWidgetOptions
-} from '@/scripts/domWidget'
-import { isDOMWidget, isComponentWidget } from '@/scripts/domWidget'
+import type { BaseDOMWidget } from '@/scripts/domWidget'
 import { generateUUID } from '@/utils/formatUtil'

 import type { PromotedWidgetSlot } from './PromotedWidgetSlot'

 /**
- * Adapts an interior DOM widget for display on a SubgraphNode.
+ * Properties delegated to the PromotedWidgetSlot instead of the inner widget.
+ */
+type SlotManagedKey = 'y' | 'last_y' | 'computedHeight'
+const SLOT_MANAGED = new Set<string>([
+  'y',
+  'last_y',
+  'computedHeight'
+] satisfies SlotManagedKey[])
+
+/**
+ * Creates a Proxy-based adapter that makes an interior DOM widget appear to
+ * belong to the SubgraphNode (host).
 *
- * When a DOM widget is promoted to a subgraph node, `DomWidgets.vue` positions
- * it using `widget.node.pos` and `widget.y`. This adapter overrides those to
- * reference the SubgraphNode (host) and the PromotedWidgetSlot's y position,
- * so the DOM element renders at the correct location on the parent graph.
+ * `DomWidgets.vue` positions DOM widgets using `widget.node.pos` and
+ * `widget.y`. This proxy overrides those to reference the host node and the
+ * PromotedWidgetSlot's positional state, so the DOM element renders at the
+ * correct location on the parent graph.
 *
 * Only ONE of {adapter, interior widget} should be registered in
- * `domWidgetStore` at a time.  The adapter is registered on creation and the
- * interior widget is deactivated.  On dispose the interior is reactivated.
- * `DomWidgets.vue` therefore only ever sees a single `DomWidget.vue` instance
- * per shared `HTMLElement`, avoiding the "element‑theft" race condition that
- * occurs when two instances try to `appendChild` the same element.
+ * `domWidgetStore` at a time.
 */
-export class PromotedDomWidgetAdapter<
-  V extends object | string
-> implements BaseDOMWidget<V> {
-  // IBaseWidget requires a symbol index signature for Vue reactivity tracking.
-  [symbol: symbol]: boolean
-  readonly id = generateUUID()
-  private readonly inner: BaseDOMWidget<V>
-  private readonly hostNode: LGraphNode
-  private readonly slot: PromotedWidgetSlot
+export function createPromotedDomWidgetAdapter<V extends object | string>(
+  inner: BaseDOMWidget<V>,
+  hostNode: LGraphNode,
+  slot: PromotedWidgetSlot
+): BaseDOMWidget<V> & { readonly innerWidget: BaseDOMWidget<V> } {
+  const adapterId = generateUUID()

-  constructor(
-    inner: BaseDOMWidget<V>,
-    hostNode: LGraphNode,
-    slot: PromotedWidgetSlot
-  ) {
-    this.inner = inner
-    this.hostNode = hostNode
-    this.slot = slot
-  }
+  type Adapted = BaseDOMWidget<V> & { readonly innerWidget: BaseDOMWidget<V> }

-  get node(): LGraphNode {
-    return this.hostNode
-  }
+  return new Proxy(inner as Adapted, {
+    get(target, prop, receiver) {
+      switch (prop) {
+        case 'id':
+          return adapterId
+        case 'node':
+          return hostNode
+        case 'promoted':
+        case 'serialize':
+          return false
+        case 'innerWidget':
+          return target
+        case 'isVisible':
+          return function isVisible() {
+            return !target.hidden && hostNode.isWidgetVisible(receiver)
+          }
+      }

-  get y(): number {
-    return this.slot.y
-  }
+      if (SLOT_MANAGED.has(prop as string))
+        return (slot as IBaseWidget)[prop as SlotManagedKey]

-  set y(_v: number) {
-    // Position is managed by the slot; ignore external writes.
-  }
+      return Reflect.get(target, prop, receiver)
+    },

-  get last_y(): number | undefined {
-    return this.slot.last_y
-  }
+    set(target, prop, value) {
+      if (SLOT_MANAGED.has(prop as string)) {
+        const widget: IBaseWidget = slot
+        widget[prop as SlotManagedKey] = value
+        return true
+      }

-  set last_y(_v: number | undefined) {
-    // Managed by the slot.
-  }
-
-  get name(): string {
-    return this.inner.name
-  }
-
-  get type(): string {
-    return this.inner.type
-  }
-
-  get options(): DOMWidgetOptions<V> {
-    return this.inner.options
-  }
-
-  get value(): V {
-    return this.inner.value
-  }
-
-  set value(v: V) {
-    this.inner.value = v
-  }
-
-  get promoted(): boolean {
-    return false
-  }
-
-  get margin(): number {
-    return this.inner.margin
-  }
-
-  get width(): number | undefined {
-    return (this.inner as IBaseWidget).width
-  }
-
-  get computedHeight(): number | undefined {
-    return this.slot.computedHeight
-  }
-
-  set computedHeight(v: number | undefined) {
-    this.slot.computedHeight = v
-  }
-
-  get computedDisabled(): boolean | undefined {
-    return (this.inner as IBaseWidget).computedDisabled
-  }
-
-  get hidden(): boolean | undefined {
-    return (this.inner as IBaseWidget).hidden
-  }
-
-  get serialize(): boolean | undefined {
-    return false
-  }
-
-  isVisible(): boolean {
-    return (
-      !this.hidden &&
-      this.hostNode.isWidgetVisible(this as unknown as IBaseWidget)
-    )
-  }
-
-  get callback(): BaseDOMWidget<V>['callback'] {
-    return this.inner.callback
-  }
-
-  set callback(v: BaseDOMWidget<V>['callback']) {
-    this.inner.callback = v
-  }
-
-  /** The interior DOM widget this adapter wraps. */
-  get innerWidget(): BaseDOMWidget<V> {
-    return this.inner
-  }
+      return Reflect.set(target, prop, value)
+    }
+  })
 }
-
-/**
- * Expose the `element` property so `DomWidget.vue` can access it via
- * the `isDOMWidget` type guard.
- */
-Object.defineProperty(PromotedDomWidgetAdapter.prototype, 'element', {
-  get(this: PromotedDomWidgetAdapter<object | string>) {
-    const inner = this.innerWidget
-    if (isDOMWidget(inner)) return inner.element
-    return undefined
-  },
-  enumerable: true,
-  configurable: true
-})
-
-/**
- * Expose the `component` property so `DomWidget.vue` can access it via
- * the `isComponentWidget` type guard.
- */
-Object.defineProperty(PromotedDomWidgetAdapter.prototype, 'component', {
-  get(this: PromotedDomWidgetAdapter<object | string>) {
-    const inner = this.innerWidget
-    if (isComponentWidget(inner)) return inner.component
-    return undefined
-  },
-  enumerable: true,
-  configurable: true
-})
-
-/**
- * Expose the `inputSpec` property for component widgets.
- */
-Object.defineProperty(PromotedDomWidgetAdapter.prototype, 'inputSpec', {
-  get(this: PromotedDomWidgetAdapter<object | string>) {
-    const inner = this.innerWidget
-    if (isComponentWidget(inner))
-      return (inner as ComponentWidget<object | string>).inputSpec
-    return undefined
-  },
-  enumerable: true,
-  configurable: true
-})
-
-/**
- * Expose the `props` property for component widgets.
- */
-Object.defineProperty(PromotedDomWidgetAdapter.prototype, 'props', {
-  get(this: PromotedDomWidgetAdapter<object | string>) {
-    const inner = this.innerWidget
-    if (isComponentWidget(inner))
-      return (inner as ComponentWidget<object | string>).props
-    return undefined
-  },
-  enumerable: true,
-  configurable: true
-})
--- a/src/core/graph/subgraph/PromotedWidgetSlot.test.ts
+++ b/src/core/graph/subgraph/PromotedWidgetSlot.test.ts
@@ -84,7 +84,6 @@ describe('PromotedWidgetSlot', () => {
      const slot = new PromotedWidgetSlot(subNode, '5', 'seed')

      expect(slot.type).toBe('number')
-      expect(slot.resolvedType).toBe('number')
    })

    it('returns button type when interior node is missing', () => {
@@ -92,7 +91,6 @@ describe('PromotedWidgetSlot', () => {
      const slot = new PromotedWidgetSlot(subNode, '5', 'seed')

      expect(slot.type).toBe('button')
-      expect(slot.resolvedType).toBe('button')
    })

    it('returns button type when interior widget is missing', () => {
@@ -105,7 +103,6 @@ describe('PromotedWidgetSlot', () => {
      const slot = new PromotedWidgetSlot(subNode, '5', 'seed')

      expect(slot.type).toBe('button')
-      expect(slot.resolvedType).toBe('button')
    })
  })

--- a/src/core/graph/subgraph/PromotedWidgetSlot.ts
+++ b/src/core/graph/subgraph/PromotedWidgetSlot.ts
@@ -16,25 +16,20 @@ import {
  useWidgetValueStore
 } from '@/stores/widgetValueStore'

-import { PromotedDomWidgetAdapter } from './PromotedDomWidgetAdapter'
+import { createPromotedDomWidgetAdapter } from './PromotedDomWidgetAdapter'

 type WidgetValue = IBaseWidget['value']

 /**
 * A lightweight widget slot for canvas rendering of promoted subgraph widgets.
 *
- * Unlike the old ProxyWidget (a JavaScript Proxy), this is a plain class that
- * implements IBaseWidget. It owns positional state (y, last_y, width) and
- * delegates value/type/drawing to the resolved interior widget via the
- * WidgetValueStore.
+ * Owns positional state (y, last_y, width) and delegates value/type/drawing
+ * to the resolved interior widget via the WidgetValueStore.
 *
 * When the interior node/widget no longer exists (disconnected state),
 * it renders a "Disconnected" placeholder.
 */
-export class PromotedWidgetSlot
-  extends BaseWidget<IBaseWidget>
-  implements IBaseWidget
-{
+export class PromotedWidgetSlot extends BaseWidget<IBaseWidget> {
  override readonly isPromotedSlot = true
  readonly sourceNodeId: NodeId
  readonly sourceWidgetName: string
@@ -45,7 +40,7 @@ export class PromotedWidgetSlot
   * `domWidgetStore` so that `DomWidgets.vue` positions the DOM element on the
   * SubgraphNode rather than the interior node.
   */
-  private domAdapter?: PromotedDomWidgetAdapter<object | string>
+  private domAdapter?: BaseDOMWidget<object | string>

  constructor(
    subgraphNode: SubgraphNode,
@@ -72,12 +67,12 @@ export class PromotedWidgetSlot
    // data properties. Override them with instance-level accessors that
    // delegate to the resolved interior widget.
    Object.defineProperty(this, 'type', {
-      get: () => this.resolvedType,
+      get: () => this.resolve()?.widget.type ?? 'button',
      configurable: true,
      enumerable: true
    })
    Object.defineProperty(this, 'options', {
-      get: () => this.resolvedOptions,
+      get: () => this.resolve()?.widget.options ?? {},
      configurable: true,
      enumerable: true
    })
@@ -89,7 +84,6 @@ export class PromotedWidgetSlot
    }

    this.syncDomAdapter()
-    this.syncLayoutSize()
  }

  /**
@@ -114,14 +108,7 @@ export class PromotedWidgetSlot
   * correct growable height on the SubgraphNode.
   */
  private syncLayoutSize(): void {
-    let resolved: ReturnType<PromotedWidgetSlot['resolve']>
-    try {
-      resolved = this.resolve()
-    } catch {
-      return
-    }
-
-    const interiorWidget = resolved?.widget
+    const interiorWidget = this.resolve()?.widget
    if (interiorWidget?.computeLayoutSize) {
      this.computeLayoutSize = (node) => interiorWidget.computeLayoutSize!(node)
    } else {
@@ -133,60 +120,43 @@ export class PromotedWidgetSlot
    node: LGraphNode
    widget: IBaseWidget
  } | null {
-    const node = this.subgraphNode.subgraph.getNodeById(this.sourceNodeId)
-    if (!node) return null
-    const widget = node.widgets?.find((w) => w.name === this.sourceWidgetName)
-    if (!widget) return null
-    return { node, widget }
+    try {
+      const node = this.subgraphNode.subgraph.getNodeById(this.sourceNodeId)
+      if (!node) return null
+      const widget = node.widgets?.find((w) => w.name === this.sourceWidgetName)
+      if (!widget) return null
+      return { node, widget }
+    } catch {
+      // May fail during construction if the subgraph is not yet fully wired
+      // (e.g. in tests or during deserialization).
+      return null
+    }
  }

-  /**
-   * Resolves to the interior widget's type, or 'button' if disconnected.
-   * Uses defineProperty to dynamically override the `type` field set by BaseWidget.
-   */
-  get resolvedType(): string {
-    return this.resolve()?.widget.type ?? 'button'
-  }
-
-  get resolvedOptions(): IBaseWidget['options'] {
-    return this.resolve()?.widget.options ?? {}
+  private get widgetState() {
+    return useWidgetValueStore().getWidget(
+      stripGraphPrefix(this.sourceNodeId),
+      this.sourceWidgetName
+    )
  }

  override get value(): WidgetValue {
-    const store = useWidgetValueStore()
-    const state = store.getWidget(
-      stripGraphPrefix(this.sourceNodeId),
-      this.sourceWidgetName
-    )
-    return state?.value as WidgetValue
+    return this.widgetState?.value as WidgetValue
  }

  override set value(v: WidgetValue) {
-    const store = useWidgetValueStore()
-    const state = store.getWidget(
-      stripGraphPrefix(this.sourceNodeId),
-      this.sourceWidgetName
-    )
+    const state = this.widgetState
    if (!state) return

    state.value = v
  }

  override get label(): string | undefined {
-    const store = useWidgetValueStore()
-    const state = store.getWidget(
-      stripGraphPrefix(this.sourceNodeId),
-      this.sourceWidgetName
-    )
-    return state?.label ?? this.name
+    return this.widgetState?.label ?? this.name
  }

  override set label(v: string | undefined) {
-    const store = useWidgetValueStore()
-    const state = store.getWidget(
-      stripGraphPrefix(this.sourceNodeId),
-      this.sourceWidgetName
-    )
+    const state = this.widgetState
    if (!state) return

    state.label = v
@@ -223,24 +193,16 @@ export class PromotedWidgetSlot
   * `HTMLElement`.
   */
  syncDomAdapter(): void {
-    // resolve() may fail during construction if the subgraph is not yet
-    // fully wired (e.g. in tests or during deserialization).
-    let resolved: ReturnType<PromotedWidgetSlot['resolve']>
-    try {
-      resolved = this.resolve()
-    } catch {
-      return
-    }
+    const resolved = this.resolve()
    if (!resolved) return

    const interiorWidget = resolved.widget
-
    const isDom =
      isDOMWidget(interiorWidget) || isComponentWidget(interiorWidget)

    if (isDom && !this.domAdapter) {
      const domWidget = interiorWidget as BaseDOMWidget<object | string>
-      const adapter = new PromotedDomWidgetAdapter(
+      const adapter = createPromotedDomWidgetAdapter(
        domWidget,
        this.subgraphNode,
        this
@@ -248,19 +210,16 @@ export class PromotedWidgetSlot
      this.domAdapter = adapter

      const store = useDomWidgetStore()
-      // The adapter satisfies BaseDOMWidget but TypeScript cannot verify
-      // the IBaseWidget symbol index signature on plain classes.
      // Start invisible — `updateWidgets()` will set `visible: true` on the
      // first canvas draw when the SubgraphNode is in the current graph.
      // This prevents a race where both adapter and interior DomWidget.vue
      // instances try to mount the same HTMLElement during `onMounted`.
-      store.registerWidget(
-        adapter as unknown as BaseDOMWidget<object | string>,
-        { visible: false }
-      )
+      store.registerWidget(adapter, { visible: false })
    } else if (!isDom && this.domAdapter) {
      this.disposeDomAdapter()
    }
+
+    this.syncLayoutSize()
  }

  /**
@@ -280,16 +239,14 @@ export class PromotedWidgetSlot
    // on every draw until it succeeds.
    if (!this.domAdapter) {
      this.syncDomAdapter()
-      this.syncLayoutSize()
    }

    const resolved = this.resolve()
-    if (!resolved) {
-      this.drawDisconnectedPlaceholder(ctx, options)
-      return
-    }

-    const concrete = toConcreteWidget(resolved.widget, resolved.node, false)
+    const concrete = resolved
+      ? toConcreteWidget(resolved.widget, resolved.node, false)
+      : null
+
    if (concrete) {
      // Suppress promoted border: the purple outline should only appear on
      // the source node inside the subgraph, not on the SubgraphNode.
@@ -306,6 +263,7 @@ export class PromotedWidgetSlot
    } else {
      this.drawWidgetShape(ctx, options)
      if (options.showText !== false) {
+        if (!resolved) ctx.fillStyle = LiteGraph.WIDGET_DISABLED_TEXT_COLOR
        this.drawTruncatingText({
          ctx,
          ...options,
@@ -316,22 +274,6 @@ export class PromotedWidgetSlot
    }
  }

-  private drawDisconnectedPlaceholder(
-    ctx: CanvasRenderingContext2D,
-    options: DrawWidgetOptions
-  ): void {
-    this.drawWidgetShape(ctx, options)
-    if (options.showText !== false) {
-      ctx.fillStyle = LiteGraph.WIDGET_DISABLED_TEXT_COLOR
-      this.drawTruncatingText({
-        ctx,
-        ...options,
-        leftPadding: 0,
-        rightPadding: 0
-      })
-    }
-  }
-
  onClick(options: WidgetEventOptions): void {
    const resolved = this.resolve()
    if (!resolved) return