cleanup: Remove JS private fields from LGraphNode, remove update requirement for selectedItems

Author: DrJKL

src/components/graph/GraphCanvas.vue

@@ -44,7 +44,7 @@
     <template #graph-canvas-panel>
       <GraphCanvasMenu v-if="canvasMenuEnabled" class="pointer-events-auto" />
       <MiniMap
-        v-if="comfyAppReady && minimapEnabled"
+        v-if="comfyAppReady && minimapEnabled && betaMenuEnabled"
         class="pointer-events-auto"
       />
     </template>
@@ -119,7 +119,6 @@ import NodeSearchboxPopover from '@/components/searchbox/NodeSearchBoxPopover.vu
 import SideToolbar from '@/components/sidebar/SideToolbar.vue'
 import TopbarBadges from '@/components/topbar/TopbarBadges.vue'
 import WorkflowTabs from '@/components/topbar/WorkflowTabs.vue'
-import { useChainCallback } from '@/composables/functional/useChainCallback'
 import type { VueNodeData } from '@/composables/graph/useGraphNodeManager'
 import { useVueNodeLifecycle } from '@/composables/graph/useVueNodeLifecycle'
 import { useNodeBadge } from '@/composables/node/useNodeBadge'
@@ -445,11 +444,6 @@ onMounted(async () => {
 
   vueNodeLifecycle.setupEmptyGraphListener()
 
-  comfyApp.canvas.onSelectionChange = useChainCallback(
-    comfyApp.canvas.onSelectionChange,
-    () => canvasStore.updateSelectedItems()
-  )
-
   // Load color palette
   colorPaletteStore.customPalettes = settingStore.get(
     'Comfy.CustomColorPalettes'

src/components/graph/SelectionToolbox.test.ts

@@ -1,15 +1,32 @@
 import { mount } from '@vue/test-utils'
-import { createPinia, setActivePinia } from 'pinia'
+import { setActivePinia } from 'pinia'
 import PrimeVue from 'primevue/config'
 import { beforeEach, describe, expect, it, vi } from 'vitest'
 import { createI18n } from 'vue-i18n'
 
 import SelectionToolbox from '@/components/graph/SelectionToolbox.vue'
-import { useCanvasStore } from '@/renderer/core/canvas/canvasStore'
 import { useCanvasInteractions } from '@/renderer/core/canvas/useCanvasInteractions'
 import { useExtensionService } from '@/services/extensionService'
+import type { Positionable } from '@/lib/litegraph/src/interfaces'
+import { ref } from 'vue'
+import { createTestingPinia } from '@pinia/testing'
+
+const mockData = vi.hoisted(() => {
+  return { canvas: undefined, selectedItems: [] as Positionable[] }
+})
+
+vi.mock('@/renderer/core/canvas/canvasStore', () => ({
+  useCanvasStore: vi.fn(() => ({
+    canvas: {
+      setDirty: vi.fn(),
+      state: {
+        selectionChanged: false
+      }
+    },
+    selectedItems: ref(mockData.selectedItems)
+  }))
+}))
 
-// Mock the composables and services
 vi.mock('@/renderer/core/canvas/useCanvasInteractions', () => ({
   useCanvasInteractions: vi.fn(() => ({
     handleWheel: vi.fn()
@@ -84,8 +101,6 @@ vi.mock('@/stores/nodeDefStore', () => ({
 }))
 
 describe('SelectionToolbox', () => {
-  let canvasStore: ReturnType<typeof useCanvasStore>
-
   const i18n = createI18n({
     legacy: false,
     locale: 'en',
@@ -108,16 +123,7 @@ describe('SelectionToolbox', () => {
   }
 
   beforeEach(() => {
-    setActivePinia(createPinia())
-    canvasStore = useCanvasStore()
-
-    // Mock the canvas to avoid "getCanvas: canvas is null" errors
-    canvasStore.canvas = {
-      setDirty: vi.fn(),
-      state: {
-        selectionChanged: false
-      }
-    } as any
+    setActivePinia(createTestingPinia({ createSpy: vi.fn }))
 
     vi.resetAllMocks()
   })
@@ -191,12 +197,12 @@ describe('SelectionToolbox', () => {
 
     it('should show info button only for single selections', () => {
       // Single node selection
-      canvasStore.selectedItems = [{ type: 'TestNode' }] as any
+      mockData.selectedItems = [{ type: 'TestNode' }] as any
       const wrapper = mountComponent()
       expect(wrapper.find('.info-button').exists()).toBe(true)
 
       // Multiple node selection
-      canvasStore.selectedItems = [
+      mockData.selectedItems = [
         { type: 'TestNode1' },
         { type: 'TestNode2' }
       ] as any
@@ -206,7 +212,7 @@ describe('SelectionToolbox', () => {
     })
 
     it('should not show info button when node definition is not found', () => {
-      canvasStore.selectedItems = [{ type: 'TestNode' }] as any
+      mockData.selectedItems = [{ type: 'TestNode' }] as any
       // mock nodedef and return null
       nodeDefMock = null
       // remount component
@@ -216,14 +222,14 @@ describe('SelectionToolbox', () => {
 
     it('should show color picker for all selections', () => {
       // Single node selection
-      canvasStore.selectedItems = [{ type: 'TestNode' }] as any
+      mockData.selectedItems = [{ type: 'TestNode' }] as any
       const wrapper = mountComponent()
       expect(wrapper.find('[data-testid="color-picker-button"]').exists()).toBe(
         true
       )
 
       // Multiple node selection
-      canvasStore.selectedItems = [
+      mockData.selectedItems = [
         { type: 'TestNode1' },
         { type: 'TestNode2' }
       ] as any
@@ -236,12 +242,12 @@ describe('SelectionToolbox', () => {
 
     it('should show frame nodes only for multiple selections', () => {
       // Single node selection
-      canvasStore.selectedItems = [{ type: 'TestNode' }] as any
+      mockData.selectedItems = [{ type: 'TestNode' }] as any
       const wrapper = mountComponent()
       expect(wrapper.find('.frame-nodes').exists()).toBe(false)
 
       // Multiple node selection
-      canvasStore.selectedItems = [
+      mockData.selectedItems = [
         { type: 'TestNode1' },
         { type: 'TestNode2' }
       ] as any
@@ -252,12 +258,12 @@ describe('SelectionToolbox', () => {
 
     it('should show bypass button for appropriate selections', () => {
       // Single node selection
-      canvasStore.selectedItems = [{ type: 'TestNode' }] as any
+      mockData.selectedItems = [{ type: 'TestNode' }] as any
       const wrapper = mountComponent()
       expect(wrapper.find('[data-testid="bypass-button"]').exists()).toBe(true)
 
       // Multiple node selection
-      canvasStore.selectedItems = [
+      mockData.selectedItems = [
         { type: 'TestNode1' },
         { type: 'TestNode2' }
       ] as any
@@ -267,7 +273,7 @@ describe('SelectionToolbox', () => {
     })
 
     it('should show common buttons for all selections', () => {
-      canvasStore.selectedItems = [{ type: 'TestNode' }] as any
+      mockData.selectedItems = [{ type: 'TestNode' }] as any
       const wrapper = mountComponent()
 
       expect(wrapper.find('[data-testid="delete-button"]').exists()).toBe(true)
@@ -285,13 +291,13 @@ describe('SelectionToolbox', () => {
 
       // Single image node
       isImageNodeSpy.mockReturnValue(true)
-      canvasStore.selectedItems = [{ type: 'ImageNode' }] as any
+      mockData.selectedItems = [{ type: 'ImageNode' }] as any
       const wrapper = mountComponent()
       expect(wrapper.find('.mask-editor-button').exists()).toBe(true)
 
       // Single non-image node
       isImageNodeSpy.mockReturnValue(false)
-      canvasStore.selectedItems = [{ type: 'TestNode' }] as any
+      mockData.selectedItems = [{ type: 'TestNode' }] as any
       wrapper.unmount()
       const wrapper2 = mountComponent()
       expect(wrapper2.find('.mask-editor-button').exists()).toBe(false)
@@ -303,13 +309,13 @@ describe('SelectionToolbox', () => {
 
       // Single Load3D node
       isLoad3dNodeSpy.mockReturnValue(true)
-      canvasStore.selectedItems = [{ type: 'Load3DNode' }] as any
+      mockData.selectedItems = [{ type: 'Load3DNode' }] as any
       const wrapper = mountComponent()
       expect(wrapper.find('.load-3d-viewer-button').exists()).toBe(true)
 
       // Single non-Load3D node
       isLoad3dNodeSpy.mockReturnValue(false)
-      canvasStore.selectedItems = [{ type: 'TestNode' }] as any
+      mockData.selectedItems = [{ type: 'TestNode' }] as any
       wrapper.unmount()
       const wrapper2 = mountComponent()
       expect(wrapper2.find('.load-3d-viewer-button').exists()).toBe(false)
@@ -326,7 +332,7 @@ describe('SelectionToolbox', () => {
       // With output node selected
       isOutputNodeSpy.mockReturnValue(true)
       filterOutputNodesSpy.mockReturnValue([{ type: 'SaveImage' }] as any)
-      canvasStore.selectedItems = [
+      mockData.selectedItems = [
         { type: 'SaveImage', constructor: { nodeData: { output_node: true } } }
       ] as any
       const wrapper = mountComponent()
@@ -335,13 +341,13 @@ describe('SelectionToolbox', () => {
       // Without output node selected
       isOutputNodeSpy.mockReturnValue(false)
       filterOutputNodesSpy.mockReturnValue([])
-      canvasStore.selectedItems = [{ type: 'TestNode' }] as any
+      mockData.selectedItems = [{ type: 'TestNode' }] as any
       wrapper.unmount()
       const wrapper2 = mountComponent()
       expect(wrapper2.find('.execute-button').exists()).toBe(false)
 
       // No selection at all
-      canvasStore.selectedItems = []
+      mockData.selectedItems = []
       wrapper2.unmount()
       const wrapper3 = mountComponent()
       expect(wrapper3.find('.execute-button').exists()).toBe(false)
@@ -351,7 +357,7 @@ describe('SelectionToolbox', () => {
   describe('Divider Visibility Logic', () => {
     it('should show dividers between button groups when both groups have buttons', () => {
       // Setup single node to show info + other buttons
-      canvasStore.selectedItems = [{ type: 'TestNode' }] as any
+      mockData.selectedItems = [{ type: 'TestNode' }] as any
       const wrapper = mountComponent()
 
       const dividers = wrapper.findAll('.vertical-divider')
@@ -360,7 +366,7 @@ describe('SelectionToolbox', () => {
 
     it('should not show dividers when adjacent groups are empty', () => {
       // No selection should show minimal buttons and dividers
-      canvasStore.selectedItems = []
+      mockData.selectedItems = []
       const wrapper = mountComponent()
 
       const buttons = wrapper.find('.panel').element.children
@@ -380,7 +386,7 @@ describe('SelectionToolbox', () => {
         invokeExtensions: vi.fn(() => ['test-command'])
       } as any)
 
-      canvasStore.selectedItems = [{ type: 'TestNode' }] as any
+      mockData.selectedItems = [{ type: 'TestNode' }] as any
       const wrapper = mountComponent()
 
       expect(wrapper.find('.extension-command-button').exists()).toBe(true)
@@ -393,7 +399,7 @@ describe('SelectionToolbox', () => {
         invokeExtensions: vi.fn(() => [])
       } as any)
 
-      canvasStore.selectedItems = [{ type: 'TestNode' }] as any
+      mockData.selectedItems = [{ type: 'TestNode' }] as any
       const wrapper = mountComponent()
 
       expect(wrapper.find('.extension-command-button').exists()).toBe(false)
@@ -408,7 +414,7 @@ describe('SelectionToolbox', () => {
         invokeExtensions: vi.fn(() => [])
       } as any)
 
-      canvasStore.selectedItems = [{ type: 'TestNode' }] as any
+      mockData.selectedItems = [{ type: 'TestNode' }] as any
       const wrapper = mountComponent()
 
       const panel = wrapper.find('.panel')
@@ -422,7 +428,7 @@ describe('SelectionToolbox', () => {
         invokeExtensions: vi.fn(() => [])
       } as any)
 
-      canvasStore.selectedItems = [{ type: 'TestNode' }] as any
+      mockData.selectedItems = [{ type: 'TestNode' }] as any
       const wrapper = mountComponent()
 
       const panel = wrapper.find('.panel')
@@ -439,7 +445,7 @@ describe('SelectionToolbox', () => {
         invokeExtensions: vi.fn(() => [])
       } as any)
 
-      canvasStore.selectedItems = [{ type: 'TestNode' }] as any
+      mockData.selectedItems = [{ type: 'TestNode' }] as any
       const wrapper = mountComponent()
 
       const panel = wrapper.find('.panel')
@@ -461,7 +467,7 @@ describe('SelectionToolbox', () => {
         invokeExtensions: vi.fn(() => [])
       } as any)
 
-      canvasStore.selectedItems = [{ type: 'TestNode' }] as any
+      mockData.selectedItems = [{ type: 'TestNode' }] as any
       const wrapper = mountComponent()
 
       const panel = wrapper.find('.panel')
@@ -481,7 +487,7 @@ describe('SelectionToolbox', () => {
     })
 
     it('should hide most buttons when no items selected', () => {
-      canvasStore.selectedItems = []
+      mockData.selectedItems = []
       const wrapper = mountComponent()
 
       expect(wrapper.find('.info-button').exists()).toBe(false)

src/components/graph/SelectionToolbox.vue

@@ -45,6 +45,7 @@
 </template>
 
 <script setup lang="ts">
+import { storeToRefs } from 'pinia'
 import Panel from 'primevue/panel'
 import { computed, ref } from 'vue'
 
@@ -80,9 +81,11 @@ const canvasInteractions = useCanvasInteractions()
 const toolboxRef = ref<HTMLElement | undefined>()
 const { visible } = useSelectionToolboxPosition(toolboxRef)
 
+const { selectedItems } = storeToRefs(canvasStore)
+
 const extensionToolboxCommands = computed<ComfyCommandImpl[]>(() => {
   const commandIds = new Set<string>(
-    canvasStore.selectedItems
+    selectedItems.value
       .map(
         (item) =>
           extensionService