Refactor: More state management simplification (#5721)

## Summary Remove more procedural synchronization in favor of using reactive references. > Note: Also includes some fixes for issues caused during HMR. ## Review Focus In testing it seems to work the same, but let me know if I missed something. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-5721-Refactor-More-state-management-simplification-2766d73d3650819b8d7ddc047c460f2b) by [Unito](https://www.unito.io)
2026-01-26 19:09:52 +00:00 · 2025-09-22 13:15:33 -07:00
parent f086377307
commit e5d4d07d32
9 changed files with 61 additions and 239 deletions
--- a/src/components/graph/GraphCanvas.vue
+++ b/src/components/graph/GraphCanvas.vue
@@ -43,8 +43,6 @@
      v-for="nodeData in allNodes"
      :key="nodeData.id"
      :node-data="nodeData"
-      :position="nodePositions.get(nodeData.id)"
-      :size="nodeSizes.get(nodeData.id)"
      :readonly="false"
      :error="
        executionStore.lastExecutionError?.node_id === nodeData.id
@@ -189,15 +187,8 @@ watch(
  }
 )

-const nodePositions = vueNodeLifecycle.nodePositions
-const nodeSizes = vueNodeLifecycle.nodeSizes
 const allNodes = viewportCulling.allNodes
-
-const handleTransformUpdate = () => {
-  viewportCulling.handleTransformUpdate()
-  // TODO: Fix paste position sync in separate PR
-  vueNodeLifecycle.detectChangesInRAF.value()
-}
+const handleTransformUpdate = viewportCulling.handleTransformUpdate

 watchEffect(() => {
  nodeDefStore.showDeprecated = settingStore.get('Comfy.Node.ShowDeprecated')
--- a/src/components/graph/NodeTooltip.vue
+++ b/src/components/graph/NodeTooltip.vue
@@ -33,9 +33,11 @@ const tooltipText = ref('')
 const left = ref<string>()
 const top = ref<string>()

-const hideTooltip = () => (tooltipText.value = '')
+function hideTooltip() {
+  return (tooltipText.value = '')
+}

-const showTooltip = async (tooltip: string | null | undefined) => {
+async function showTooltip(tooltip: string | null | undefined) {
  if (!tooltip) return

  left.value = comfyApp.canvas.mouse[0] + 'px'
@@ -56,9 +58,9 @@ const showTooltip = async (tooltip: string | null | undefined) => {
  }
 }

-const onIdle = () => {
+function onIdle() {
  const { canvas } = comfyApp
-  const node = canvas.node_over
+  const node = canvas?.node_over
  if (!node) return

  const ctor = node.constructor as { title_mode?: 0 | 1 | 2 | 3 }
--- a/src/components/searchbox/NodeSearchBoxPopover.vue
+++ b/src/components/searchbox/NodeSearchBoxPopover.vue
@@ -64,31 +64,29 @@ const litegraphService = useLitegraphService()

 const { visible, newSearchBoxEnabled } = storeToRefs(searchBoxStore)
 const dismissable = ref(true)
-const getNewNodeLocation = (): Point => {
+function getNewNodeLocation(): Point {
  return triggerEvent
    ? [triggerEvent.canvasX, triggerEvent.canvasY]
    : litegraphService.getCanvasCenter()
 }
 const nodeFilters = ref<FuseFilterWithValue<ComfyNodeDefImpl, string>[]>([])
-const addFilter = (filter: FuseFilterWithValue<ComfyNodeDefImpl, string>) => {
+function addFilter(filter: FuseFilterWithValue<ComfyNodeDefImpl, string>) {
  nodeFilters.value.push(filter)
 }
-const removeFilter = (
-  filter: FuseFilterWithValue<ComfyNodeDefImpl, string>
-) => {
+function removeFilter(filter: FuseFilterWithValue<ComfyNodeDefImpl, string>) {
  nodeFilters.value = nodeFilters.value.filter(
    (f) => toRaw(f) !== toRaw(filter)
  )
 }
-const clearFilters = () => {
+function clearFilters() {
  nodeFilters.value = []
 }
-const closeDialog = () => {
+function closeDialog() {
  visible.value = false
 }
 const canvasStore = useCanvasStore()

-const addNode = (nodeDef: ComfyNodeDefImpl) => {
+function addNode(nodeDef: ComfyNodeDefImpl) {
  const node = litegraphService.addNodeOnGraph(nodeDef, {
    pos: getNewNodeLocation()
  })
@@ -106,7 +104,7 @@ const addNode = (nodeDef: ComfyNodeDefImpl) => {
  window.requestAnimationFrame(closeDialog)
 }

-const showSearchBox = (e: CanvasPointerEvent | null) => {
+function showSearchBox(e: CanvasPointerEvent | null) {
  if (newSearchBoxEnabled.value) {
    if (e?.pointerType === 'touch') {
      setTimeout(() => {
@@ -120,11 +118,12 @@ const showSearchBox = (e: CanvasPointerEvent | null) => {
  }
 }

-const getFirstLink = () =>
-  canvasStore.getCanvas().linkConnector.renderLinks.at(0)
+function getFirstLink() {
+  return canvasStore.getCanvas().linkConnector.renderLinks.at(0)
+}

 const nodeDefStore = useNodeDefStore()
-const showNewSearchBox = (e: CanvasPointerEvent | null) => {
+function showNewSearchBox(e: CanvasPointerEvent | null) {
  const firstLink = getFirstLink()
  if (firstLink) {
    const filter =
@@ -149,7 +148,7 @@ const showNewSearchBox = (e: CanvasPointerEvent | null) => {
  }, 300)
 }

-const showContextMenu = (e: CanvasPointerEvent) => {
+function showContextMenu(e: CanvasPointerEvent) {
  const firstLink = getFirstLink()
  if (!firstLink) return

@@ -226,7 +225,7 @@ watchEffect(() => {
  )
 })

-const canvasEventHandler = (e: LiteGraphCanvasEvent) => {
+function canvasEventHandler(e: LiteGraphCanvasEvent) {
  if (e.detail.subType === 'empty-double-click') {
    showSearchBox(e.detail.originalEvent)
  } else if (e.detail.subType === 'group-double-click') {
@@ -249,8 +248,10 @@ const linkReleaseActionShift = computed(() =>
 )

 // Prevent normal LinkConnector reset (called by CanvasPointer.finally)
-const preventDefault = (e: Event) => e.preventDefault()
-const cancelNextReset = (e: CustomEvent<CanvasPointerEvent>) => {
+function preventDefault(e: Event) {
+  return e.preventDefault()
+}
+function cancelNextReset(e: CustomEvent<CanvasPointerEvent>) {
  e.preventDefault()

  const canvas = canvasStore.getCanvas()
@@ -260,7 +261,7 @@ const cancelNextReset = (e: CustomEvent<CanvasPointerEvent>) => {
  })
 }

-const handleDroppedOnCanvas = (e: CustomEvent<CanvasPointerEvent>) => {
+function handleDroppedOnCanvas(e: CustomEvent<CanvasPointerEvent>) {
  disconnectOnReset = true
  const action = e.detail.shiftKey
    ? linkReleaseActionShift.value
@@ -281,7 +282,7 @@ const handleDroppedOnCanvas = (e: CustomEvent<CanvasPointerEvent>) => {
 }

 // Resets litegraph state
-const reset = () => {
+function reset() {
  listenerController?.abort()
  listenerController = null
  triggerEvent = null
--- a/src/composables/graph/useGraphNodeManager.ts
+++ b/src/composables/graph/useGraphNodeManager.ts
@@ -72,8 +72,6 @@ export interface GraphNodeManager {
  // Reactive state - safe data extracted from LiteGraph nodes
  vueNodeData: ReadonlyMap<string, VueNodeData>
  nodeState: ReadonlyMap<string, NodeState>
-  nodePositions: ReadonlyMap<string, { x: number; y: number }>
-  nodeSizes: ReadonlyMap<string, { width: number; height: number }>

  // Access to original LiteGraph nodes (non-reactive)
  getNode(id: string): LGraphNode | undefined
@@ -88,7 +86,6 @@ export interface GraphNodeManager {
    priority?: 'critical' | 'normal' | 'low'
  ): void
  forceSync(): void
-  detectChangesInRAF(): void

  // Spatial queries
  getVisibleNodeIds(viewportBounds: Bounds): Set<string>
@@ -101,17 +98,12 @@ export interface GraphNodeManager {
  getSpatialIndexDebugInfo(): SpatialIndexDebugInfo | null
 }

-export const useGraphNodeManager = (graph: LGraph): GraphNodeManager => {
+export function useGraphNodeManager(graph: LGraph): GraphNodeManager {
  // Get layout mutations composable
-  const { moveNode, resizeNode, createNode, deleteNode, setSource } =
-    useLayoutMutations()
+  const { createNode, deleteNode, setSource } = useLayoutMutations()
  // Safe reactive data extracted from LiteGraph nodes
  const vueNodeData = reactive(new Map<string, VueNodeData>())
  const nodeState = reactive(new Map<string, NodeState>())
-  const nodePositions = reactive(new Map<string, { x: number; y: number }>())
-  const nodeSizes = reactive(
-    new Map<string, { width: number; height: number }>()
-  )

  // Non-reactive storage for original LiteGraph nodes
  const nodeRefs = new Map<string, LGraphNode>()
@@ -365,7 +357,6 @@ export const useGraphNodeManager = (graph: LGraph): GraphNodeManager => {
  //   }
  //   return metadata
  // }
-
  const scheduleUpdate = (
    nodeId?: string,
    priority: 'critical' | 'normal' | 'low' = 'normal'
@@ -432,8 +423,6 @@ export const useGraphNodeManager = (graph: LGraph): GraphNodeManager => {
        nodeRefs.delete(id)
        vueNodeData.delete(id)
        nodeState.delete(id)
-        nodePositions.delete(id)
-        nodeSizes.delete(id)
        lastNodesSnapshot.delete(id)
        spatialIndex.remove(id)
      }
@@ -459,8 +448,6 @@ export const useGraphNodeManager = (graph: LGraph): GraphNodeManager => {
          lastUpdate: performance.now(),
          culled: false
        })
-        nodePositions.set(id, { x: node.pos[0], y: node.pos[1] })
-        nodeSizes.set(id, { width: node.size[0], height: node.size[1] })
        attachMetadata(node)

        // Add to spatial index
@@ -496,120 +483,6 @@ export const useGraphNodeManager = (graph: LGraph): GraphNodeManager => {
    return visibleIds
  }

-  /**
-   * Detects position changes for a single node and updates reactive state
-   */
-  const detectPositionChanges = (node: LGraphNode, id: string): boolean => {
-    const currentPos = nodePositions.get(id)
-
-    if (
-      !currentPos ||
-      currentPos.x !== node.pos[0] ||
-      currentPos.y !== node.pos[1]
-    ) {
-      nodePositions.set(id, { x: node.pos[0], y: node.pos[1] })
-
-      // Push position change to layout store
-      // Source is already set to 'canvas' in detectChangesInRAF
-      void moveNode(id, { x: node.pos[0], y: node.pos[1] })
-
-      return true
-    }
-    return false
-  }
-
-  /**
-   * Detects size changes for a single node and updates reactive state
-   */
-  const detectSizeChanges = (node: LGraphNode, id: string): boolean => {
-    const currentSize = nodeSizes.get(id)
-
-    if (
-      !currentSize ||
-      currentSize.width !== node.size[0] ||
-      currentSize.height !== node.size[1]
-    ) {
-      nodeSizes.set(id, { width: node.size[0], height: node.size[1] })
-
-      // Push size change to layout store
-      // Source is already set to 'canvas' in detectChangesInRAF
-      void resizeNode(id, {
-        width: node.size[0],
-        height: node.size[1]
-      })
-
-      return true
-    }
-    return false
-  }
-
-  /**
-   * Updates spatial index for a node if bounds changed
-   */
-  const updateSpatialIndex = (node: LGraphNode, id: string): void => {
-    const bounds: Bounds = {
-      x: node.pos[0],
-      y: node.pos[1],
-      width: node.size[0],
-      height: node.size[1]
-    }
-    spatialIndex.update(id, bounds)
-  }
-
-  /**
-   * Updates performance metrics after change detection
-   */
-  const updatePerformanceMetrics = (
-    startTime: number,
-    positionUpdates: number,
-    sizeUpdates: number
-  ): void => {
-    const endTime = performance.now()
-    performanceMetrics.updateTime = endTime - startTime
-    performanceMetrics.nodeCount = vueNodeData.size
-    performanceMetrics.culledCount = Array.from(nodeState.values()).filter(
-      (state) => state.culled
-    ).length
-    spatialMetrics.nodesInIndex = spatialIndex.size
-
-    if (positionUpdates > 0 || sizeUpdates > 0) {
-      performanceMetrics.rafUpdateCount++
-    }
-  }
-
-  /**
-   * Main RAF change detection function
-   */
-  const detectChangesInRAF = () => {
-    const startTime = performance.now()
-
-    if (!graph?._nodes) return
-
-    let positionUpdates = 0
-    let sizeUpdates = 0
-
-    // Set source for all canvas-driven updates
-    setSource(LayoutSource.Canvas)
-
-    // Process each node for changes
-    for (const node of graph._nodes) {
-      const id = String(node.id)
-
-      const posChanged = detectPositionChanges(node, id)
-      const sizeChanged = detectSizeChanges(node, id)
-
-      if (posChanged) positionUpdates++
-      if (sizeChanged) sizeUpdates++
-
-      // Update spatial index if geometry changed
-      if (posChanged || sizeChanged) {
-        updateSpatialIndex(node, id)
-      }
-    }
-
-    updatePerformanceMetrics(startTime, positionUpdates, sizeUpdates)
-  }
-
  /**
   * Handles node addition to the graph - sets up Vue state and spatial indexing
   * Defers position extraction until after potential configure() calls
@@ -642,8 +515,6 @@ export const useGraphNodeManager = (graph: LGraph): GraphNodeManager => {
      const nodePosition = { x: node.pos[0], y: node.pos[1] }
      const nodeSize = { width: node.size[0], height: node.size[1] }

-      nodePositions.set(id, nodePosition)
-      nodeSizes.set(id, nodeSize)
      attachMetadata(node)

      // Add to spatial index for viewport culling with final positions
@@ -709,8 +580,6 @@ export const useGraphNodeManager = (graph: LGraph): GraphNodeManager => {
    nodeRefs.delete(id)
    vueNodeData.delete(id)
    nodeState.delete(id)
-    nodePositions.delete(id)
-    nodeSizes.delete(id)
    lastNodesSnapshot.delete(id)

    // Call original callback if provided
@@ -743,8 +612,6 @@ export const useGraphNodeManager = (graph: LGraph): GraphNodeManager => {
      nodeRefs.clear()
      vueNodeData.clear()
      nodeState.clear()
-      nodePositions.clear()
-      nodeSizes.clear()
      lastNodesSnapshot.clear()
      pendingUpdates.clear()
      criticalUpdates.clear()
@@ -846,14 +713,11 @@ export const useGraphNodeManager = (graph: LGraph): GraphNodeManager => {
  return {
    vueNodeData,
    nodeState,
-    nodePositions,
-    nodeSizes,
    getNode,
    setupEventListeners,
    cleanup,
    scheduleUpdate,
    forceSync: syncWithGraph,
-    detectChangesInRAF,
    getVisibleNodeIds,
    performanceMetrics,
    spatialMetrics,
--- a/src/composables/graph/useViewportCulling.ts
+++ b/src/composables/graph/useViewportCulling.ts
@@ -6,21 +6,18 @@
 * 2. Set display none on element to avoid cascade resolution overhead
 * 3. Only run when transform changes (event driven)
 */
+import { useThrottleFn } from '@vueuse/core'
 import { computed } from 'vue'

 import { useVueNodeLifecycle } from '@/composables/graph/useVueNodeLifecycle'
-import { useVueFeatureFlags } from '@/composables/useVueFeatureFlags'
 import { useCanvasStore } from '@/renderer/core/canvas/canvasStore'
 import { app as comfyApp } from '@/scripts/app'

 export function useViewportCulling() {
  const canvasStore = useCanvasStore()
-  const { shouldRenderVueNodes } = useVueFeatureFlags()
-  const { vueNodeData, nodeDataTrigger, nodeManager } = useVueNodeLifecycle()
+  const { vueNodeData, nodeManager } = useVueNodeLifecycle()

  const allNodes = computed(() => {
-    if (!shouldRenderVueNodes.value) return []
-    void nodeDataTrigger.value // Force re-evaluation when nodeManager initializes
    return Array.from(vueNodeData.value.values())
  })

@@ -28,7 +25,7 @@ export function useViewportCulling() {
   * Update visibility of all nodes based on viewport
   * Queries DOM directly - no cache maintenance needed
   */
-  const updateVisibility = () => {
+  function updateVisibility() {
    if (!nodeManager.value || !canvasStore.canvas || !comfyApp.canvas) return

    const canvas = canvasStore.canvas
@@ -70,31 +67,17 @@ export function useViewportCulling() {
    }
  }

+  const updateVisibilityDebounced = useThrottleFn(updateVisibility, 20)
+
  // RAF throttling for smooth updates during continuous panning
-  let rafId: number | null = null
-
-  /**
-   * Handle transform update - called by TransformPane event
-   * Uses RAF to batch updates for smooth performance
-   */
-  const handleTransformUpdate = () => {
-    if (!shouldRenderVueNodes.value) return
-
-    // Cancel previous RAF if still pending
-    if (rafId !== null) {
-      cancelAnimationFrame(rafId)
-    }
-
-    // Schedule update in next animation frame
-    rafId = requestAnimationFrame(() => {
-      updateVisibility()
-      rafId = null
+  function handleTransformUpdate() {
+    requestAnimationFrame(async () => {
+      await updateVisibilityDebounced()
    })
  }

  return {
    allNodes,
-    handleTransformUpdate,
-    updateVisibility
+    handleTransformUpdate
  }
 }
--- a/src/composables/graph/useVueNodeLifecycle.ts
+++ b/src/composables/graph/useVueNodeLifecycle.ts
@@ -43,15 +43,6 @@ function useVueNodeLifecycleIndividual() {
  // Vue node data state
  const vueNodeData = ref<ReadonlyMap<string, VueNodeData>>(new Map())
  const nodeState = ref<ReadonlyMap<string, NodeState>>(new Map())
-  const nodePositions = ref<ReadonlyMap<string, { x: number; y: number }>>(
-    new Map()
-  )
-  const nodeSizes = ref<ReadonlyMap<string, { width: number; height: number }>>(
-    new Map()
-  )
-
-  // Change detection function
-  const detectChangesInRAF = ref<() => void>(() => {})

  // Trigger for forcing computed re-evaluation
  const nodeDataTrigger = ref(0)
@@ -71,9 +62,6 @@ function useVueNodeLifecycleIndividual() {
    // Use the manager's data maps
    vueNodeData.value = manager.vueNodeData
    nodeState.value = manager.nodeState
-    nodePositions.value = manager.nodePositions
-    nodeSizes.value = manager.nodeSizes
-    detectChangesInRAF.value = manager.detectChangesInRAF

    // Initialize layout system with existing nodes from active graph
    const nodes = activeGraph._nodes.map((node: LGraphNode) => ({
@@ -137,11 +125,6 @@ function useVueNodeLifecycleIndividual() {
    // Reset reactive maps to clean state
    vueNodeData.value = new Map()
    nodeState.value = new Map()
-    nodePositions.value = new Map()
-    nodeSizes.value = new Map()
-
-    // Reset change detection function
-    detectChangesInRAF.value = () => {}
  }

  // Watch for Vue nodes enabled state changes
@@ -236,11 +219,8 @@ function useVueNodeLifecycleIndividual() {
  return {
    vueNodeData,
    nodeState,
-    nodePositions,
-    nodeSizes,
    nodeDataTrigger: readonly(nodeDataTrigger),
    nodeManager: readonly(nodeManager),
-    detectChangesInRAF: readonly(detectChangesInRAF),
    isNodeManagerReady,

    // Lifecycle methods
--- a/src/renderer/extensions/vueNodes/components/LGraphNode.vue
+++ b/src/renderer/extensions/vueNodes/components/LGraphNode.vue
@@ -31,7 +31,7 @@
    "
    :style="[
      {
-        transform: `translate(${layoutPosition.x ?? position?.x ?? 0}px, ${(layoutPosition.y ?? position?.y ?? 0) - LiteGraph.NODE_TITLE_HEIGHT}px)`,
+        transform: `translate(${position.x ?? 0}px, ${(position.y ?? 0) - LiteGraph.NODE_TITLE_HEIGHT}px)`,
        zIndex: zIndex
      },
      dragStyle
@@ -172,8 +172,6 @@ import SlotConnectionDot from './SlotConnectionDot.vue'
 // Extended props for main node component
 interface LGraphNodeProps {
  nodeData: VueNodeData
-  position?: { x: number; y: number }
-  size?: { width: number; height: number }
  readonly?: boolean
  error?: string | null
  zoomLevel?: number
@@ -181,8 +179,6 @@ interface LGraphNodeProps {

 const {
  nodeData,
-  position = { x: 0, y: 0 },
-  size = { width: 100, height: 50 },
  error = null,
  readonly = false,
  zoomLevel = 1
@@ -245,11 +241,7 @@ onErrorCaptured((error) => {
 })

 // Use layout system for node position and dragging
-const {
-  position: layoutPosition,
-  zIndex,
-  resize
-} = useNodeLayout(() => nodeData.id)
+const { position, size, zIndex, resize } = useNodeLayout(() => nodeData.id)
 const {
  handlePointerDown,
  handlePointerUp,
@@ -259,11 +251,11 @@ const {
 } = useNodePointerInteractions(() => nodeData, handleNodeSelect)

 onMounted(() => {
-  if (size && transformState?.camera) {
+  if (size.value && transformState?.camera) {
    const scale = transformState.camera.z
    const screenSize = {
-      width: size.width * scale,
-      height: size.height * scale
+      width: size.value.width * scale,
+      height: size.value.height * scale
    }
    resize(screenSize)
  }
--- a/src/renderer/extensions/vueNodes/layout/useNodeLayout.ts
+++ b/src/renderer/extensions/vueNodes/layout/useNodeLayout.ts
@@ -1,5 +1,11 @@
 import { storeToRefs } from 'pinia'
-import { type MaybeRefOrGetter, computed, inject, toValue } from 'vue'
+import {
+  type CSSProperties,
+  type MaybeRefOrGetter,
+  computed,
+  inject,
+  toValue
+} from 'vue'

 import { useCanvasStore } from '@/renderer/core/canvas/canvasStore'
 import { TransformStateKey } from '@/renderer/core/layout/injectionKeys'
@@ -182,14 +188,16 @@ export function useNodeLayout(nodeIdMaybe: MaybeRefOrGetter<string>) {
    endDrag,

    // Computed styles for Vue templates
-    nodeStyle: computed(() => ({
-      position: 'absolute' as const,
-      left: `${position.value.x}px`,
-      top: `${position.value.y}px`,
-      width: `${size.value.width}px`,
-      height: `${size.value.height}px`,
-      zIndex: zIndex.value,
-      cursor: isDragging ? 'grabbing' : 'grab'
-    }))
+    nodeStyle: computed(
+      (): CSSProperties => ({
+        position: 'absolute' as const,
+        left: `${position.value.x}px`,
+        top: `${position.value.y}px`,
+        width: `${size.value.width}px`,
+        height: `${size.value.height}px`,
+        zIndex: zIndex.value,
+        cursor: isDragging ? 'grabbing' : 'grab'
+      })
+    )
  }
 }