Comfy-Org/ComfyUI_frontend
1
0
Fork 0
mirror of https://github.com/Comfy-Org/ComfyUI_frontend.git synced 2026-01-26 10:59:53 +00:00
Code Issues Packages Projects Releases Wiki Activity

Fix changeTracker modified state (#1481)

Browse Source 
* Add jsondiffpatch

* Add logs

* Add graphDiff helper

* Fix changeTracker

* Add loglevel

* Add playwright test

* Fix jest test

* nit

* nit

* Fix test url

* nit
This commit is contained in:
Chenlei Hu
2024-11-08 22:24:35 -05:00
committed by GitHub
parent c12f059940
commit f8ec87ddea
8 changed files with 171 additions and 11 deletions
Download Patch File Download Diff File Expand all files Collapse all files

37
browser_tests/changeTracker.spec.ts
View File

@@ -16,6 +16,43 @@ async function afterChange(comfyPage: ComfyPage) {
}
test.describe('Change Tracker', () => {
test.describe('Undo/Redo', () => {
test.beforeEach(async ({ comfyPage }) => {
await comfyPage.setSetting('Comfy.UseNewMenu', 'Top')
})
// Flaky https://github.com/Comfy-Org/ComfyUI_frontend/pull/1481
// The collapse can be recognized as several changes.
test.skip('Can undo multiple operations', async ({ comfyPage }) => {
function isModified() {
return comfyPage.page.evaluate(async () => {
return window['app'].extensionManager.workflow.activeWorkflow
.isModified
})
}
await comfyPage.menu.topbar.saveWorkflow('undo-redo-test')
expect(await isModified()).toBe(false)
const node = (await comfyPage.getFirstNodeRef())!
await node.click('collapse')
await expect(node).toBeCollapsed()
expect(await isModified()).toBe(true)
await comfyPage.ctrlB()
await expect(node).toBeBypassed()
expect(await isModified()).toBe(true)
await comfyPage.ctrlZ()
await expect(node).not.toBeBypassed()
expect(await isModified()).toBe(true)
await comfyPage.ctrlZ()
await expect(node).not.toBeCollapsed()
expect(await isModified()).toBe(false)
})
})
test('Can group multiple change actions into a single transaction', async ({
comfyPage
}) => {

4
browser_tests/fixtures/ComfyPage.ts
View File

@@ -167,7 +167,7 @@ export class ComfyPage {
}
async setupUser(username: string) {
const res = await this.request.get(`${this.url}/users`)
const res = await this.request.get(`${this.url}/api/users`)
if (res.status() !== 200)
throw new Error(`Failed to retrieve users: ${await res.text()}`)
@@ -181,7 +181,7 @@ export class ComfyPage {
}
async createUser(username: string) {
const resp = await this.request.post(`${this.url}/users`, {
const resp = await this.request.post(`${this.url}/api/users`, {
data: { username }
})

45
package-lock.json generated
View File

@@ -17,7 +17,9 @@
"axios": "^1.7.4",
"dotenv": "^16.4.5",
"fuse.js": "^7.0.0",
"jsondiffpatch": "^0.6.0",
"lodash": "^4.17.21",
"loglevel": "^1.9.2",
"pinia": "^2.1.7",
"primeicons": "^7.0.0",
"primevue": "^4.0.5",
@@ -3890,6 +3892,12 @@
"@babel/types": "^7.20.7"
}
},
"node_modules/@types/diff-match-patch": {
"version": "1.0.36",
"resolved": "https://registry.npmjs.org/@types/diff-match-patch/-/diff-match-patch-1.0.36.tgz",
"integrity": "sha512-xFdR6tkm0MWvBfO8xXCSsinYxHcqkQUlcHeSpMC2ukzOb6lwQAfDmW+Qt0AvlGd8HpsS28qKsB+oPeJn9I39jg==",
"license": "MIT"
},
"node_modules/@types/estree": {
"version": "1.0.5",
"resolved": "https://registry.npmjs.org/@types/estree/-/estree-1.0.5.tgz",
@@ -5379,7 +5387,6 @@
"version": "5.3.0",
"resolved": "https://registry.npmjs.org/chalk/-/chalk-5.3.0.tgz",
"integrity": "sha512-dLitG79d+GV1Nb/VYcCDFivJeK1hiukt9QjRNVOsUtTy1rR1YJsmpGGTZ3qJos+uw7WmWF4wUwBd9jxjocFC2w==",
"dev": true,
"engines": {
"node": "^12.17.0 || ^14.13 || >=16.0.0"
},
@@ -5991,6 +5998,12 @@
"node": ">=0.3.1"
}
},
"node_modules/diff-match-patch": {
"version": "1.0.5",
"resolved": "https://registry.npmjs.org/diff-match-patch/-/diff-match-patch-1.0.5.tgz",
"integrity": "sha512-IayShXAgj/QMXgB0IWmKx+rOPuGMhqm5w6jvFxmVenXKIzRqTAAsbBPT3kWQeGANj3jGgvcvv4yK6SxqYmikgw==",
"license": "Apache-2.0"
},
"node_modules/diff-sequences": {
"version": "29.6.3",
"resolved": "https://registry.npmjs.org/diff-sequences/-/diff-sequences-29.6.3.tgz",
@@ -9402,6 +9415,23 @@
"node": ">=6"
}
},
"node_modules/jsondiffpatch": {
"version": "0.6.0",
"resolved": "https://registry.npmjs.org/jsondiffpatch/-/jsondiffpatch-0.6.0.tgz",
"integrity": "sha512-3QItJOXp2AP1uv7waBkao5nCvhEv+QmJAd38Ybq7wNI74Q+BBmnLn4EDKz6yI9xGAIQoUF87qHt+kc1IVxB4zQ==",
"license": "MIT",
"dependencies": {
"@types/diff-match-patch": "^1.0.36",
"chalk": "^5.3.0",
"diff-match-patch": "^1.0.5"
},
"bin": {
"jsondiffpatch": "bin/jsondiffpatch.js"
},
"engines": {
"node": "^18.0.0 || >=20.0.0"
}
},
"node_modules/jsonfile": {
"version": "6.1.0",
"resolved": "https://registry.npmjs.org/jsonfile/-/jsonfile-6.1.0.tgz",
@@ -9934,6 +9964,19 @@
"url": "https://github.com/chalk/strip-ansi?sponsor=1"
}
},
"node_modules/loglevel": {
"version": "1.9.2",
"resolved": "https://registry.npmjs.org/loglevel/-/loglevel-1.9.2.tgz",
"integrity": "sha512-HgMmCqIJSAKqo68l0rS2AanEWfkxaZ5wNiEFb5ggm08lDs9Xl2KxBlX3PTcaD2chBM1gXAYf491/M2Rv8Jwayg==",
"license": "MIT",
"engines": {
"node": ">= 0.6.0"
},
"funding": {
"type": "tidelift",
"url": "https://tidelift.com/funding/github/npm/loglevel"
}
},
"node_modules/loupe": {
"version": "3.1.1",
"resolved": "https://registry.npmjs.org/loupe/-/loupe-3.1.1.tgz",

2
package.json
View File

@@ -79,7 +79,9 @@
"axios": "^1.7.4",
"dotenv": "^16.4.5",
"fuse.js": "^7.0.0",
"jsondiffpatch": "^0.6.0",
"lodash": "^4.17.21",
"loglevel": "^1.9.2",
"pinia": "^2.1.7",
"primeicons": "^7.0.0",
"primevue": "^4.0.5",

2
src/scripts/app.ts
View File

@@ -2363,7 +2363,7 @@ export class ComfyApp {
}
await this.#invokeExtensionsAsync('afterConfigureGraph', missingNodeTypes)
// @ts-expect-error zod types issue. Will be fixed after we enable ts-strict
workflowService.afterLoadNewGraph(workflow, this.graph.serialize())
await workflowService.afterLoadNewGraph(workflow, this.graph.serialize())
requestAnimationFrame(() => {
this.graph.setDirtyCanvas(true, true)
})

80
src/scripts/changeTracker.ts
View File

@@ -7,6 +7,8 @@ import type { ComfyWorkflowJSON } from '@/types/comfyWorkflow'
import type { ExecutedWsMessage } from '@/types/apiTypes'
import { useExecutionStore } from '@/stores/executionStore'
import _ from 'lodash'
import * as jsondiffpatch from 'jsondiffpatch'
import log from 'loglevel'
function clone(obj: any) {
try {
@@ -20,6 +22,10 @@ function clone(obj: any) {
return JSON.parse(JSON.stringify(obj))
}
const logger = log.getLogger('ChangeTracker')
// Change to debug for more verbose logging
logger.setLevel('info')
export class ChangeTracker {
static MAX_HISTORY = 50
/**
@@ -29,6 +35,10 @@ export class ChangeTracker {
undoQueue: ComfyWorkflowJSON[] = []
redoQueue: ComfyWorkflowJSON[] = []
changeCount: number = 0
/**
* Whether the redo/undo restoring is in progress.
*/
private restoringState: boolean = false
ds?: { scale: number; offset: [number, number] }
nodeOutputs?: Record<string, any>
@@ -55,8 +65,12 @@ export class ChangeTracker {
* Save the current state as the initial state.
*/
reset(state?: ComfyWorkflowJSON) {
// Do not reset the state if we are restoring.
if (this.restoringState) return
logger.debug('Reset State')
this.activeState = state ?? this.activeState
this.initialState = this.activeState
this.initialState = clone(this.activeState)
}
store() {
@@ -85,6 +99,13 @@ export class ChangeTracker {
this.initialState,
this.activeState
)
if (workflow.isModified) {
const diff = ChangeTracker.graphDiff(
this.initialState,
this.activeState
)
logger.debug('Graph diff:', diff)
}
}
}
@@ -101,6 +122,8 @@ export class ChangeTracker {
if (this.undoQueue.length > ChangeTracker.MAX_HISTORY) {
this.undoQueue.shift()
}
logger.debug('Diff detected. Undo queue length:', this.undoQueue.length)
this.activeState = clone(currentState)
this.redoQueue.length = 0
api.dispatchEvent(
@@ -114,21 +137,38 @@ export class ChangeTracker {
const prevState = source.pop()
if (prevState) {
target.push(this.activeState!)
await this.app.loadGraphData(prevState, false, false, this.workflow, {
showMissingModelsDialog: false,
showMissingNodesDialog: false
})
this.activeState = prevState
this.updateModified()
this.restoringState = true
try {
await this.app.loadGraphData(prevState, false, false, this.workflow, {
showMissingModelsDialog: false,
showMissingNodesDialog: false
})
this.activeState = prevState
this.updateModified()
} finally {
this.restoringState = false
}
}
}
async undo() {
await this.updateState(this.undoQueue, this.redoQueue)
logger.debug(
'Undo. Undo queue length:',
this.undoQueue.length,
'Redo queue length:',
this.redoQueue.length
)
}
async redo() {
await this.updateState(this.redoQueue, this.undoQueue)
logger.debug(
'Redo. Undo queue length:',
this.undoQueue.length,
'Redo queue length:',
this.redoQueue.length
)
}
async undoRedo(e: KeyboardEvent) {
@@ -198,6 +238,7 @@ export class ChangeTracker {
// If our active element is some type of input then handle changes after they're done
if (ChangeTracker.bindInput(app, bindInputEl)) return
logger.debug('checkState on keydown')
changeTracker.checkState()
})
},
@@ -207,21 +248,25 @@ export class ChangeTracker {
window.addEventListener('keyup', (e) => {
if (keyIgnored) {
keyIgnored = false
logger.debug('checkState on keyup')
checkState()
}
})
// Handle clicking DOM elements (e.g. widgets)
window.addEventListener('mouseup', () => {
logger.debug('checkState on mouseup')
checkState()
})
// Handle prompt queue event for dynamic widget changes
api.addEventListener('promptQueued', () => {
logger.debug('checkState on promptQueued')
checkState()
})
api.addEventListener('graphCleared', () => {
logger.debug('checkState on graphCleared')
checkState()
})
@@ -229,12 +274,14 @@ export class ChangeTracker {
const processMouseUp = LGraphCanvas.prototype.processMouseUp
LGraphCanvas.prototype.processMouseUp = function (e) {
const v = processMouseUp.apply(this, [e])
logger.debug('checkState on processMouseUp')
checkState()
return v
}
const processMouseDown = LGraphCanvas.prototype.processMouseDown
LGraphCanvas.prototype.processMouseDown = function (e) {
const v = processMouseDown.apply(this, [e])
logger.debug('checkState on processMouseDown')
checkState()
return v
}
@@ -251,6 +298,7 @@ export class ChangeTracker {
callback(v)
checkState()
}
logger.debug('checkState on prompt')
return prompt.apply(this, [title, value, extendedCallback, event])
}
@@ -258,6 +306,7 @@ export class ChangeTracker {
const close = LiteGraph.ContextMenu.prototype.close
LiteGraph.ContextMenu.prototype.close = function (e: MouseEvent) {
const v = close.apply(this, [e])
logger.debug('checkState on contextMenuClose')
checkState()
return v
}
@@ -267,6 +316,7 @@ export class ChangeTracker {
LiteGraph.LGraph.prototype.onNodeAdded = function (node: LGraphNode) {
const v = onNodeAdded?.apply(this, [node])
if (!app?.configuringGraph) {
logger.debug('checkState on onNodeAdded')
checkState()
}
return v
@@ -357,4 +407,20 @@ export class ChangeTracker {
return false
}
static graphDiff(a: ComfyWorkflowJSON, b: ComfyWorkflowJSON) {
function sortGraphNodes(graph: ComfyWorkflowJSON) {
return {
links: graph.links,
groups: graph.groups,
nodes: graph.nodes.sort((a, b) => {
if (typeof a.id === 'number' && typeof b.id === 'number') {
return a.id - b.id
}
return 0
})
}
}
return jsondiffpatch.diff(sortGraphNodes(a), sortGraphNodes(b))
}
}

6
tests-ui/tests/fast/globalSetup.ts
View File

@@ -19,4 +19,10 @@ module.exports = async function () {
useI18n: jest.fn()
}
})
jest.mock('jsondiffpatch', () => {
return {
diff: jest.fn()
}
})
}

6
tests-ui/tests/globalSetup.ts
View File

@@ -74,4 +74,10 @@ module.exports = async function () {
useI18n: jest.fn()
}
})
jest.mock('jsondiffpatch', () => {
return {
diff: jest.fn()
}
})
}