Make ChangeTracker detect changes in workflow.extra (Except ds) (#1692)

* Compare workflow.extra content

* Add tests

browser_tests/changeTracker.spec.ts

@@ -3,9 +3,6 @@ import {
   comfyPageFixture as test,
   comfyExpect as expect
 } from './fixtures/ComfyPage'
-import type { useWorkspaceStore } from '../src/stores/workspaceStore'
-
-type WorkspaceStore = ReturnType<typeof useWorkspaceStore>
 
 async function beforeChange(comfyPage: ComfyPage) {
   await comfyPage.page.evaluate(() => {
@@ -26,64 +23,41 @@ test.describe('Change Tracker', () => {
     })
 
     test('Can undo multiple operations', async ({ comfyPage }) => {
-      function isModified() {
-        return comfyPage.page.evaluate(async () => {
-          return !!(window['app'].extensionManager as WorkspaceStore).workflow
-            .activeWorkflow?.isModified
-        })
-      }
-
-      function getUndoQueueSize() {
-        return comfyPage.page.evaluate(() => {
-          const workflow = (window['app'].extensionManager as WorkspaceStore)
-            .workflow.activeWorkflow
-          return workflow?.changeTracker.undoQueue.length
-        })
-      }
-
-      function getRedoQueueSize() {
-        return comfyPage.page.evaluate(() => {
-          const workflow = (window['app'].extensionManager as WorkspaceStore)
-            .workflow.activeWorkflow
-          return workflow?.changeTracker.redoQueue.length
-        })
-      }
-      expect(await getUndoQueueSize()).toBe(0)
-      expect(await getRedoQueueSize()).toBe(0)
+      expect(await comfyPage.getUndoQueueSize()).toBe(0)
+      expect(await comfyPage.getRedoQueueSize()).toBe(0)
 
       // Save, confirm no errors & workflow modified flag removed
       await comfyPage.menu.topbar.saveWorkflow('undo-redo-test')
       expect(await comfyPage.getToastErrorCount()).toBe(0)
-      expect(await isModified()).toBe(false)
-
-      expect(await getUndoQueueSize()).toBe(0)
-      expect(await getRedoQueueSize()).toBe(0)
+      expect(await comfyPage.isCurrentWorkflowModified()).toBe(false)
+      expect(await comfyPage.getUndoQueueSize()).toBe(0)
+      expect(await comfyPage.getRedoQueueSize()).toBe(0)
 
       const node = (await comfyPage.getFirstNodeRef())!
       await node.click('title')
       await node.click('collapse')
       await expect(node).toBeCollapsed()
-      expect(await isModified()).toBe(true)
-      expect(await getUndoQueueSize()).toBe(1)
-      expect(await getRedoQueueSize()).toBe(0)
+      expect(await comfyPage.isCurrentWorkflowModified()).toBe(true)
+      expect(await comfyPage.getUndoQueueSize()).toBe(1)
+      expect(await comfyPage.getRedoQueueSize()).toBe(0)
 
       await comfyPage.ctrlB()
       await expect(node).toBeBypassed()
-      expect(await isModified()).toBe(true)
-      expect(await getUndoQueueSize()).toBe(2)
-      expect(await getRedoQueueSize()).toBe(0)
+      expect(await comfyPage.isCurrentWorkflowModified()).toBe(true)
+      expect(await comfyPage.getUndoQueueSize()).toBe(2)
+      expect(await comfyPage.getRedoQueueSize()).toBe(0)
 
       await comfyPage.ctrlZ()
       await expect(node).not.toBeBypassed()
-      expect(await isModified()).toBe(true)
-      expect(await getUndoQueueSize()).toBe(1)
-      expect(await getRedoQueueSize()).toBe(1)
+      expect(await comfyPage.isCurrentWorkflowModified()).toBe(true)
+      expect(await comfyPage.getUndoQueueSize()).toBe(1)
+      expect(await comfyPage.getRedoQueueSize()).toBe(1)
 
       await comfyPage.ctrlZ()
       await expect(node).not.toBeCollapsed()
-      expect(await isModified()).toBe(false)
-      expect(await getUndoQueueSize()).toBe(0)
-      expect(await getRedoQueueSize()).toBe(2)
+      expect(await comfyPage.isCurrentWorkflowModified()).toBe(false)
+      expect(await comfyPage.getUndoQueueSize()).toBe(0)
+      expect(await comfyPage.getRedoQueueSize()).toBe(2)
     })
   })
 
@@ -174,4 +148,20 @@ test.describe('Change Tracker', () => {
     await expect(node).toBePinned()
     await expect(node).toBeCollapsed()
   })
+
+  test('Can detect changes in workflow.extra', async ({ comfyPage }) => {
+    expect(await comfyPage.getUndoQueueSize()).toBe(0)
+    await comfyPage.page.evaluate(() => {
+      window['app'].graph.extra.foo = 'bar'
+    })
+    // Click empty space to trigger a change detection.
+    await comfyPage.clickEmptySpace()
+    expect(await comfyPage.getUndoQueueSize()).toBe(1)
+  })
+
+  test('Ignores changes in workflow.ds', async ({ comfyPage }) => {
+    expect(await comfyPage.getUndoQueueSize()).toBe(0)
+    await comfyPage.pan({ x: 10, y: 10 })
+    expect(await comfyPage.getUndoQueueSize()).toBe(0)
+  })
 })

browser_tests/fixtures/ComfyPage.ts

@@ -17,8 +17,11 @@ import {
 import { Topbar } from './components/Topbar'
 import { NodeReference } from './utils/litegraphUtils'
 import type { Position, Size } from './types'
+import type { useWorkspaceStore } from '../../src/stores/workspaceStore'
 import { SettingDialog } from './components/SettingDialog'
 
+type WorkspaceStore = ReturnType<typeof useWorkspaceStore>
+
 class ComfyMenu {
   public readonly sideToolbar: Locator
   public readonly themeToggleButton: Locator
@@ -788,6 +791,26 @@ export class ComfyPage {
   async moveMouseToEmptyArea() {
     await this.page.mouse.move(10, 10)
   }
+  async getUndoQueueSize() {
+    return this.page.evaluate(() => {
+      const workflow = (window['app'].extensionManager as WorkspaceStore)
+        .workflow.activeWorkflow
+      return workflow?.changeTracker.undoQueue.length
+    })
+  }
+  async getRedoQueueSize() {
+    return this.page.evaluate(() => {
+      const workflow = (window['app'].extensionManager as WorkspaceStore)
+        .workflow.activeWorkflow
+      return workflow?.changeTracker.redoQueue.length
+    })
+  }
+  async isCurrentWorkflowModified() {
+    return this.page.evaluate(() => {
+      return (window['app'].extensionManager as WorkspaceStore).workflow
+        .activeWorkflow?.isModified
+    })
+  }
 }
 
 export const comfyPageFixture = base.extend<{ comfyPage: ComfyPage }>({

src/scripts/changeTracker.ts

@@ -388,8 +388,11 @@ export class ChangeTracker {
         return false
       }
 
-      // Reroutes (schema 0.4)
-      if (!_.isEqual(a.extra?.reroutes, b.extra?.reroutes)) return false
+      // Compare extra properties ignoring ds
+      if (
+        !_.isEqual(_.omit(a.extra ?? {}, ['ds']), _.omit(b.extra ?? {}, ['ds']))
+      )
+        return false
 
       // Compare other properties normally
       for (const key of ['links', 'reroutes', 'groups']) {