From bf5c7a8a038ff3b11548abdeb0a65828cd4dd2b6 Mon Sep 17 00:00:00 2001
From: Alexander Brown <448862+DrJKL@users.noreply.github.com>
Date: Mon, 26 Jan 2026 00:16:25 -0800
Subject: [PATCH] refactor(GraphCanvas): Phase 1 - critical fixes

- Fix dead isNativeWindow template condition (was impossible branch)

- Add async cancellation guards to onMounted

- Add cleanup for onSelectionChange in onUnmounted

Amp-Thread-ID: https://ampcode.com/threads/T-019bf956-6121-720f-a40b-a0388bf7fb40
Co-authored-by: Amp <amp@ampcode.com>
---
 src/components/graph/GraphCanvas.vue | 27 ++++++++++++++++++++++-----
 1 file changed, 22 insertions(+), 5 deletions(-)

diff --git a/src/components/graph/GraphCanvas.vue b/src/components/graph/GraphCanvas.vue
index 5d7e9c199..a4bef29f4 100644
--- a/src/components/graph/GraphCanvas.vue
+++ b/src/components/graph/GraphCanvas.vue
@@ -4,15 +4,15 @@
   synced with the stateStorage (localStorage). -->
   <LiteGraphCanvasSplitterOverlay v-if="comfyAppReady">
     <template v-if="showUI" #workflow-tabs>
+      <!-- Native drag area for Electron (when tabs are NOT in topbar) -->
+      <div
+        v-if="isNativeWindow() && workflowTabsPosition !== 'Topbar'"
+        class="app-drag fixed top-0 left-0 z-10 h-[var(--comfy-topbar-height)] w-full"
+      />
       <div
         v-if="workflowTabsPosition === 'Topbar'"
         class="workflow-tabs-container pointer-events-auto relative h-9.5 w-full"
       >
-        <!-- Native drag area for Electron -->
-        <div
-          v-if="isNativeWindow() && workflowTabsPosition !== 'Topbar'"
-          class="app-drag fixed top-0 left-0 z-10 h-[var(--comfy-topbar-height)] w-full"
-        />
         <div
           class="flex h-full items-center border-b border-interface-stroke bg-comfy-menu-bg shadow-interface"
         >
@@ -430,6 +430,10 @@ useEventListener(
   }
 )
 
+let disposed = false
+let prevOnSelectionChange: typeof comfyApp.canvas.onSelectionChange | null =
+  null
+
 onMounted(async () => {
   comfyApp.vueAppReady = true
   workspaceStore.spinner = true
@@ -438,6 +442,7 @@ onMounted(async () => {
   ChangeTracker.init()
 
   await until(() => isSettingsReady.value || !!settingsError.value).toBe(true)
+  if (disposed) return
 
   if (settingsError.value) {
     if (settingsError.value instanceof UnauthorizedError) {
@@ -458,6 +463,8 @@ onMounted(async () => {
     until(() => isI18nReady.value || !!i18nError.value).toBe(true),
     newUserService().initializeIfNewUser(settingStore)
   ])
+  if (disposed) return
+
   if (i18nError.value) {
     console.warn(
       '[GraphCanvas] Failed to load custom nodes i18n:',
@@ -467,6 +474,8 @@ onMounted(async () => {
 
   // @ts-expect-error fixme ts strict error
   await comfyApp.setup(canvasRef.value)
+  if (disposed) return
+
   canvasStore.canvas = comfyApp.canvas
   canvasStore.canvas.render_canvas_border = false
   workspaceStore.spinner = false
@@ -479,6 +488,7 @@ onMounted(async () => {
 
   vueNodeLifecycle.setupEmptyGraphListener()
 
+  prevOnSelectionChange = comfyApp.canvas.onSelectionChange
   comfyApp.canvas.onSelectionChange = useChainCallback(
     comfyApp.canvas.onSelectionChange,
     () => canvasStore.updateSelectedItems()
@@ -491,10 +501,13 @@ onMounted(async () => {
 
   // Restore saved workflow and workflow tabs state
   await workflowPersistence.initializeWorkflow()
+  if (disposed) return
+
   workflowPersistence.restoreWorkflowTabsState()
 
   // Load template from URL if present
   await workflowPersistence.loadTemplateFromUrlIfPresent()
+  if (disposed) return
 
   // Accept workspace invite from URL if present (e.g., ?invite=TOKEN)
   // Uses watch because feature flags load asynchronously - flag may be false initially
@@ -522,6 +535,10 @@ onMounted(async () => {
 })
 
 onUnmounted(() => {
+  disposed = true
   vueNodeLifecycle.cleanup()
+  if (prevOnSelectionChange && comfyApp.canvas) {
+    comfyApp.canvas.onSelectionChange = prevOnSelectionChange
+  }
 })
 </script>