mirror of
https://github.com/Comfy-Org/ComfyUI_frontend.git
synced 2026-04-20 06:20:11 +00:00
refactor: rename checkState to captureCanvasState with active-tracker assertion
Add self-defending invariant to captureCanvasState (formerly checkState): only the active workflow's tracker may read from the canvas. Calling it on an inactive tracker logs an error in DEV and returns early, preventing silent cross-workflow data corruption. Mechanical rename across all 26 production/test files and docs.
This commit is contained in:
@@ -57,7 +57,7 @@ test.describe('Actionbar', { tag: '@ui' }, () => {
|
||||
|
||||
;(
|
||||
window.app!.extensionManager as WorkspaceStore
|
||||
).workflow.activeWorkflow?.changeTracker.checkState()
|
||||
).workflow.activeWorkflow?.changeTracker.captureCanvasState()
|
||||
}, value)
|
||||
}
|
||||
|
||||
|
||||
@@ -34,7 +34,7 @@ test.describe('Browser tab title', { tag: '@smoke' }, () => {
|
||||
window.app!.graph!.setDirtyCanvas(true, true)
|
||||
;(
|
||||
window.app!.extensionManager as WorkspaceStore
|
||||
).workflow.activeWorkflow?.changeTracker?.checkState()
|
||||
).workflow.activeWorkflow?.changeTracker?.captureCanvasState()
|
||||
})
|
||||
await expect
|
||||
.poll(() => comfyPage.page.title())
|
||||
|
||||
@@ -12,7 +12,7 @@ test.describe(
|
||||
await comfyPage.workflow.setupWorkflowsDirectory({})
|
||||
})
|
||||
|
||||
test('Prevents checkState from corrupting workflow state during tab switch', async ({
|
||||
test('Prevents captureCanvasState from corrupting workflow state during tab switch', async ({
|
||||
comfyPage
|
||||
}) => {
|
||||
// Tab 0: default workflow (7 nodes)
|
||||
@@ -21,9 +21,9 @@ test.describe(
|
||||
// Save tab 0 so it has a unique name for tab switching
|
||||
await comfyPage.menu.topbar.saveWorkflow('workflow-a')
|
||||
|
||||
// Register an extension that forces checkState during graph loading.
|
||||
// Register an extension that forces captureCanvasState during graph loading.
|
||||
// This simulates the bug scenario where a user clicks during graph loading
|
||||
// which triggers a checkState call on the wrong graph, corrupting the activeState.
|
||||
// which triggers a captureCanvasState call on the wrong graph, corrupting the activeState.
|
||||
await comfyPage.page.evaluate(() => {
|
||||
window.app!.registerExtension({
|
||||
name: 'TestCheckStateDuringLoad',
|
||||
@@ -35,7 +35,7 @@ test.describe(
|
||||
// ; (workflow.changeTracker.constructor as unknown as { isLoadingGraph: boolean }).isLoadingGraph = false
|
||||
|
||||
// Simulate the user clicking during graph loading
|
||||
workflow.changeTracker.checkState()
|
||||
workflow.changeTracker.captureCanvasState()
|
||||
}
|
||||
})
|
||||
})
|
||||
|
||||
@@ -124,9 +124,9 @@ test.describe('Workflow Persistence', () => {
|
||||
await comfyPage.page.evaluate(() => {
|
||||
const em = window.app!.extensionManager as unknown as Record<
|
||||
string,
|
||||
{ activeWorkflow?: { changeTracker?: { checkState(): void } } }
|
||||
{ activeWorkflow?: { changeTracker?: { captureCanvasState(): void } } }
|
||||
>
|
||||
em.workflow?.activeWorkflow?.changeTracker?.checkState()
|
||||
em.workflow?.activeWorkflow?.changeTracker?.captureCanvasState()
|
||||
})
|
||||
|
||||
await expect.poll(() => getNodeOutputImageCount(comfyPage, nodeId)).toBe(1)
|
||||
@@ -398,7 +398,7 @@ test.describe('Workflow Persistence', () => {
|
||||
test.info().annotations.push({
|
||||
type: 'regression',
|
||||
description:
|
||||
'PR #10745 — saveWorkflow called checkState on inactive tab, serializing the active graph instead'
|
||||
'PR #10745 — saveWorkflow called captureCanvasState on inactive tab, serializing the active graph instead'
|
||||
})
|
||||
|
||||
await comfyPage.settings.setSetting(
|
||||
@@ -428,13 +428,13 @@ test.describe('Workflow Persistence', () => {
|
||||
const nodeCountB = await comfyPage.nodeOps.getNodeCount()
|
||||
expect(nodeCountB).toBe(nodeCountA + 1)
|
||||
|
||||
// Trigger checkState so isModified is set
|
||||
// Trigger captureCanvasState so isModified is set
|
||||
await comfyPage.page.evaluate(() => {
|
||||
const em = window.app!.extensionManager as unknown as Record<
|
||||
string,
|
||||
{ activeWorkflow?: { changeTracker?: { checkState(): void } } }
|
||||
{ activeWorkflow?: { changeTracker?: { captureCanvasState(): void } } }
|
||||
>
|
||||
em.workflow?.activeWorkflow?.changeTracker?.checkState()
|
||||
em.workflow?.activeWorkflow?.changeTracker?.captureCanvasState()
|
||||
})
|
||||
|
||||
// Switch to A via topbar tab (making B inactive)
|
||||
@@ -479,7 +479,7 @@ test.describe('Workflow Persistence', () => {
|
||||
test.info().annotations.push({
|
||||
type: 'regression',
|
||||
description:
|
||||
'PR #10745 — saveWorkflowAs called checkState on inactive temp tab, serializing the active graph'
|
||||
'PR #10745 — saveWorkflowAs called captureCanvasState on inactive temp tab, serializing the active graph'
|
||||
})
|
||||
|
||||
await comfyPage.settings.setSetting(
|
||||
@@ -504,13 +504,13 @@ test.describe('Workflow Persistence', () => {
|
||||
})
|
||||
await comfyPage.nextFrame()
|
||||
|
||||
// Trigger checkState so isModified is set
|
||||
// Trigger captureCanvasState so isModified is set
|
||||
await comfyPage.page.evaluate(() => {
|
||||
const em = window.app!.extensionManager as unknown as Record<
|
||||
string,
|
||||
{ activeWorkflow?: { changeTracker?: { checkState(): void } } }
|
||||
{ activeWorkflow?: { changeTracker?: { captureCanvasState(): void } } }
|
||||
>
|
||||
em.workflow?.activeWorkflow?.changeTracker?.checkState()
|
||||
em.workflow?.activeWorkflow?.changeTracker?.captureCanvasState()
|
||||
})
|
||||
|
||||
const nodeCountB = await comfyPage.nodeOps.getNodeCount()
|
||||
|
||||
@@ -5,14 +5,18 @@ history by comparing serialized graph snapshots.
|
||||
|
||||
## How It Works
|
||||
|
||||
`checkState()` is the core method. It:
|
||||
`captureCanvasState()` is the core method. It:
|
||||
|
||||
1. Serializes the current graph via `app.rootGraph.serialize()`
|
||||
2. Deep-compares the result against the last known `activeState`
|
||||
3. If different, pushes `activeState` onto `undoQueue` and replaces it
|
||||
|
||||
**It is not reactive.** Changes to the graph (widget values, node positions,
|
||||
links, etc.) are only captured when `checkState()` is explicitly triggered.
|
||||
links, etc.) are only captured when `captureCanvasState()` is explicitly triggered.
|
||||
|
||||
**INVARIANT:** `captureCanvasState()` asserts that it is called on the active
|
||||
workflow's tracker. Calling it on an inactive tracker logs an error in DEV and
|
||||
returns early, preventing cross-workflow data corruption.
|
||||
|
||||
## Automatic Triggers
|
||||
|
||||
@@ -31,7 +35,7 @@ These are set up once in `ChangeTracker.init()`:
|
||||
| Graph cleared | `api` `graphCleared` event | Full graph clear |
|
||||
| Transaction end | `litegraph:canvas` `after-change` event | Batched operations via `beforeChange`/`afterChange` |
|
||||
|
||||
## When You Must Call `checkState()` Manually
|
||||
## When You Must Call `captureCanvasState()` Manually
|
||||
|
||||
The automatic triggers above are designed around LiteGraph's native DOM
|
||||
rendering. They **do not cover**:
|
||||
@@ -50,17 +54,17 @@ rendering. They **do not cover**:
|
||||
import { useWorkflowStore } from '@/platform/workflow/management/stores/workflowStore'
|
||||
|
||||
// After mutating the graph:
|
||||
useWorkflowStore().activeWorkflow?.changeTracker?.checkState()
|
||||
useWorkflowStore().activeWorkflow?.changeTracker?.captureCanvasState()
|
||||
```
|
||||
|
||||
### Existing Manual Call Sites
|
||||
|
||||
These locations already call `checkState()` explicitly:
|
||||
These locations already call `captureCanvasState()` explicitly:
|
||||
|
||||
- `WidgetSelectDropdown.vue` — After dropdown selection and file upload
|
||||
- `ColorPickerButton.vue` — After changing node colors
|
||||
- `NodeSearchBoxPopover.vue` — After adding a node from search
|
||||
- `useAppSetDefaultView.ts` — After setting default view
|
||||
- `builderViewOptions.ts` — After setting default view
|
||||
- `useSelectionOperations.ts` — After align, copy, paste, duplicate, group
|
||||
- `useSelectedNodeActions.ts` — After pin, bypass, collapse
|
||||
- `useGroupMenuOptions.ts` — After group operations
|
||||
@@ -76,7 +80,7 @@ For operations that make multiple changes that should be a single undo entry:
|
||||
```typescript
|
||||
changeTracker.beforeChange()
|
||||
// ... multiple graph mutations ...
|
||||
changeTracker.afterChange() // calls checkState() when nesting count hits 0
|
||||
changeTracker.afterChange() // calls captureCanvasState() when nesting count hits 0
|
||||
```
|
||||
|
||||
The `litegraph:canvas` custom event also supports this with `before-change` /
|
||||
@@ -84,8 +88,10 @@ The `litegraph:canvas` custom event also supports this with `before-change` /
|
||||
|
||||
## Key Invariants
|
||||
|
||||
- `checkState()` is a no-op during `loadGraphData` (guarded by
|
||||
- `captureCanvasState()` asserts it is called on the active workflow's tracker;
|
||||
inactive trackers get an early return (and a DEV error log)
|
||||
- `captureCanvasState()` is a no-op during `loadGraphData` (guarded by
|
||||
`isLoadingGraph`) to prevent cross-workflow corruption
|
||||
- `checkState()` is a no-op when `changeCount > 0` (inside a transaction)
|
||||
- `captureCanvasState()` is a no-op when `changeCount > 0` (inside a transaction)
|
||||
- `undoQueue` is capped at 50 entries (`MAX_HISTORY`)
|
||||
- `graphEqual` ignores node order and `ds` (pan/zoom) when comparing
|
||||
|
||||
@@ -46,7 +46,7 @@ const mockActiveWorkflow = ref<{
|
||||
isTemporary: boolean
|
||||
initialMode?: string
|
||||
isModified?: boolean
|
||||
changeTracker?: { checkState: () => void }
|
||||
changeTracker?: { captureCanvasState: () => void }
|
||||
} | null>({
|
||||
isTemporary: true,
|
||||
initialMode: 'app'
|
||||
|
||||
@@ -49,10 +49,10 @@ describe('setWorkflowDefaultView', () => {
|
||||
expect(app.rootGraph.extra.linearMode).toBe(false)
|
||||
})
|
||||
|
||||
it('calls changeTracker.checkState', () => {
|
||||
it('calls changeTracker.captureCanvasState', () => {
|
||||
const workflow = createMockLoadedWorkflow()
|
||||
setWorkflowDefaultView(workflow, true)
|
||||
expect(workflow.changeTracker.checkState).toHaveBeenCalledOnce()
|
||||
expect(workflow.changeTracker.captureCanvasState).toHaveBeenCalledOnce()
|
||||
})
|
||||
|
||||
it('tracks telemetry with correct default_view', () => {
|
||||
|
||||
@@ -9,7 +9,7 @@ export function setWorkflowDefaultView(
|
||||
workflow.initialMode = openAsApp ? 'app' : 'graph'
|
||||
const extra = (app.rootGraph.extra ??= {})
|
||||
extra.linearMode = openAsApp
|
||||
workflow.changeTracker?.checkState()
|
||||
workflow.changeTracker?.captureCanvasState()
|
||||
useTelemetry()?.trackDefaultViewSet({
|
||||
default_view: openAsApp ? 'app' : 'graph'
|
||||
})
|
||||
|
||||
@@ -31,7 +31,7 @@ function createMockWorkflow(
|
||||
const changeTracker = Object.assign(
|
||||
new ChangeTracker(workflow, structuredClone(defaultGraph)),
|
||||
{
|
||||
checkState: vi.fn() as Mock
|
||||
captureCanvasState: vi.fn() as Mock
|
||||
}
|
||||
)
|
||||
|
||||
|
||||
@@ -125,7 +125,7 @@ const applyColor = (colorOption: ColorOption | null) => {
|
||||
canvasStore.canvas?.setDirty(true, true)
|
||||
currentColorOption.value = canvasColorOption
|
||||
showColorPicker.value = false
|
||||
workflowStore.activeWorkflow?.changeTracker.checkState()
|
||||
workflowStore.activeWorkflow?.changeTracker?.captureCanvasState()
|
||||
}
|
||||
|
||||
const currentColorOption = ref<CanvasColorOption | null>(null)
|
||||
|
||||
@@ -143,7 +143,7 @@ function addNode(nodeDef: ComfyNodeDefImpl, dragEvent?: MouseEvent) {
|
||||
disconnectOnReset = false
|
||||
|
||||
// Notify changeTracker - new step should be added
|
||||
useWorkflowStore().activeWorkflow?.changeTracker?.checkState()
|
||||
useWorkflowStore().activeWorkflow?.changeTracker?.captureCanvasState()
|
||||
window.requestAnimationFrame(closeDialog)
|
||||
}
|
||||
|
||||
|
||||
@@ -13,7 +13,7 @@ export function useCanvasRefresh() {
|
||||
canvasStore.canvas?.setDirty(true, true)
|
||||
canvasStore.canvas?.graph?.afterChange()
|
||||
canvasStore.canvas?.emitAfterChange()
|
||||
workflowStore.activeWorkflow?.changeTracker?.checkState()
|
||||
workflowStore.activeWorkflow?.changeTracker?.captureCanvasState()
|
||||
}
|
||||
|
||||
return {
|
||||
|
||||
@@ -36,7 +36,7 @@ export function useGroupMenuOptions() {
|
||||
groupContext.resizeTo(groupContext.children, padding)
|
||||
groupContext.graph?.change()
|
||||
canvasStore.canvas?.setDirty(true, true)
|
||||
workflowStore.activeWorkflow?.changeTracker?.checkState()
|
||||
workflowStore.activeWorkflow?.changeTracker?.captureCanvasState()
|
||||
}
|
||||
})
|
||||
|
||||
@@ -119,7 +119,7 @@ export function useGroupMenuOptions() {
|
||||
})
|
||||
canvasStore.canvas?.setDirty(true, true)
|
||||
groupContext.graph?.change()
|
||||
workflowStore.activeWorkflow?.changeTracker?.checkState()
|
||||
workflowStore.activeWorkflow?.changeTracker?.captureCanvasState()
|
||||
bump()
|
||||
}
|
||||
})
|
||||
|
||||
@@ -23,7 +23,7 @@ export function useSelectedNodeActions() {
|
||||
})
|
||||
|
||||
app.canvas.setDirty(true, true)
|
||||
workflowStore.activeWorkflow?.changeTracker?.checkState()
|
||||
workflowStore.activeWorkflow?.changeTracker?.captureCanvasState()
|
||||
}
|
||||
|
||||
const toggleNodeCollapse = () => {
|
||||
@@ -33,7 +33,7 @@ export function useSelectedNodeActions() {
|
||||
})
|
||||
|
||||
app.canvas.setDirty(true, true)
|
||||
workflowStore.activeWorkflow?.changeTracker?.checkState()
|
||||
workflowStore.activeWorkflow?.changeTracker?.captureCanvasState()
|
||||
}
|
||||
|
||||
const toggleNodePin = () => {
|
||||
@@ -43,7 +43,7 @@ export function useSelectedNodeActions() {
|
||||
})
|
||||
|
||||
app.canvas.setDirty(true, true)
|
||||
workflowStore.activeWorkflow?.changeTracker?.checkState()
|
||||
workflowStore.activeWorkflow?.changeTracker?.captureCanvasState()
|
||||
}
|
||||
|
||||
const toggleNodeBypass = () => {
|
||||
|
||||
@@ -47,7 +47,7 @@ export function useSelectionOperations() {
|
||||
canvas.pasteFromClipboard({ connectInputs: false })
|
||||
|
||||
// Trigger change tracking
|
||||
workflowStore.activeWorkflow?.changeTracker?.checkState()
|
||||
workflowStore.activeWorkflow?.changeTracker?.captureCanvasState()
|
||||
}
|
||||
|
||||
const duplicateSelection = () => {
|
||||
@@ -73,7 +73,7 @@ export function useSelectionOperations() {
|
||||
canvas.pasteFromClipboard({ connectInputs: false })
|
||||
|
||||
// Trigger change tracking
|
||||
workflowStore.activeWorkflow?.changeTracker?.checkState()
|
||||
workflowStore.activeWorkflow?.changeTracker?.captureCanvasState()
|
||||
}
|
||||
|
||||
const deleteSelection = () => {
|
||||
@@ -92,7 +92,7 @@ export function useSelectionOperations() {
|
||||
canvas.setDirty(true, true)
|
||||
|
||||
// Trigger change tracking
|
||||
workflowStore.activeWorkflow?.changeTracker?.checkState()
|
||||
workflowStore.activeWorkflow?.changeTracker?.captureCanvasState()
|
||||
}
|
||||
|
||||
const renameSelection = async () => {
|
||||
@@ -122,7 +122,7 @@ export function useSelectionOperations() {
|
||||
const titledItem = item as { title: string }
|
||||
titledItem.title = newTitle
|
||||
app.canvas.setDirty(true, true)
|
||||
workflowStore.activeWorkflow?.changeTracker?.checkState()
|
||||
workflowStore.activeWorkflow?.changeTracker?.captureCanvasState()
|
||||
}
|
||||
}
|
||||
return
|
||||
@@ -145,7 +145,7 @@ export function useSelectionOperations() {
|
||||
}
|
||||
})
|
||||
app.canvas.setDirty(true, true)
|
||||
workflowStore.activeWorkflow?.changeTracker?.checkState()
|
||||
workflowStore.activeWorkflow?.changeTracker?.captureCanvasState()
|
||||
}
|
||||
return
|
||||
}
|
||||
|
||||
@@ -31,7 +31,7 @@ export function useSubgraphOperations() {
|
||||
canvas.select(node)
|
||||
canvasStore.updateSelectedItems()
|
||||
// Trigger change tracking
|
||||
workflowStore.activeWorkflow?.changeTracker?.checkState()
|
||||
workflowStore.activeWorkflow?.changeTracker?.captureCanvasState()
|
||||
}
|
||||
|
||||
const doUnpack = (
|
||||
@@ -46,7 +46,7 @@ export function useSubgraphOperations() {
|
||||
nodeOutputStore.revokeSubgraphPreviews(subgraphNode)
|
||||
graph.unpackSubgraph(subgraphNode, { skipMissingNodes })
|
||||
}
|
||||
workflowStore.activeWorkflow?.changeTracker?.checkState()
|
||||
workflowStore.activeWorkflow?.changeTracker?.captureCanvasState()
|
||||
}
|
||||
|
||||
const unpackSubgraph = () => {
|
||||
|
||||
@@ -94,7 +94,7 @@ vi.mock('@/stores/toastStore', () => ({
|
||||
}))
|
||||
|
||||
const mockChangeTracker = vi.hoisted(() => ({
|
||||
checkState: vi.fn()
|
||||
captureCanvasState: vi.fn()
|
||||
}))
|
||||
const mockWorkflowStore = vi.hoisted(() => ({
|
||||
activeWorkflow: {
|
||||
@@ -382,7 +382,7 @@ describe('useCoreCommands', () => {
|
||||
|
||||
expect(mockDialogService.prompt).toHaveBeenCalled()
|
||||
expect(mockSubgraph.extra.BlueprintDescription).toBe('Test description')
|
||||
expect(mockChangeTracker.checkState).toHaveBeenCalled()
|
||||
expect(mockChangeTracker.captureCanvasState).toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('should not set description when user cancels', async () => {
|
||||
@@ -397,7 +397,7 @@ describe('useCoreCommands', () => {
|
||||
await setDescCommand.function()
|
||||
|
||||
expect(mockSubgraph.extra.BlueprintDescription).toBeUndefined()
|
||||
expect(mockChangeTracker.checkState).not.toHaveBeenCalled()
|
||||
expect(mockChangeTracker.captureCanvasState).not.toHaveBeenCalled()
|
||||
})
|
||||
})
|
||||
|
||||
@@ -432,7 +432,7 @@ describe('useCoreCommands', () => {
|
||||
'alias2',
|
||||
'alias3'
|
||||
])
|
||||
expect(mockChangeTracker.checkState).toHaveBeenCalled()
|
||||
expect(mockChangeTracker.captureCanvasState).toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('should trim whitespace and filter empty strings', async () => {
|
||||
@@ -478,7 +478,7 @@ describe('useCoreCommands', () => {
|
||||
await setAliasesCommand.function()
|
||||
|
||||
expect(mockSubgraph.extra.BlueprintSearchAliases).toBeUndefined()
|
||||
expect(mockChangeTracker.checkState).not.toHaveBeenCalled()
|
||||
expect(mockChangeTracker.captureCanvasState).not.toHaveBeenCalled()
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
@@ -1164,7 +1164,7 @@ export function useCoreCommands(): ComfyCommand[] {
|
||||
if (description === null) return
|
||||
|
||||
extra.BlueprintDescription = description.trim() || undefined
|
||||
workflowStore.activeWorkflow?.changeTracker?.checkState()
|
||||
workflowStore.activeWorkflow?.changeTracker?.captureCanvasState()
|
||||
}
|
||||
},
|
||||
{
|
||||
@@ -1201,7 +1201,7 @@ export function useCoreCommands(): ComfyCommand[] {
|
||||
}
|
||||
|
||||
extra.BlueprintSearchAliases = aliases.length > 0 ? aliases : undefined
|
||||
workflowStore.activeWorkflow?.changeTracker?.checkState()
|
||||
workflowStore.activeWorkflow?.changeTracker?.captureCanvasState()
|
||||
}
|
||||
},
|
||||
{
|
||||
|
||||
@@ -139,7 +139,8 @@ export const useWorkflowService = () => {
|
||||
}
|
||||
|
||||
if (isSelfOverwrite) {
|
||||
if (workflowStore.isActive(workflow)) workflow.changeTracker?.checkState()
|
||||
if (workflowStore.isActive(workflow))
|
||||
workflow.changeTracker?.captureCanvasState()
|
||||
await saveWorkflow(workflow)
|
||||
} else {
|
||||
let target: ComfyWorkflow
|
||||
@@ -156,7 +157,8 @@ export const useWorkflowService = () => {
|
||||
app.rootGraph.extra.linearMode = isApp
|
||||
target.initialMode = isApp ? 'app' : 'graph'
|
||||
}
|
||||
if (workflowStore.isActive(target)) target.changeTracker?.checkState()
|
||||
if (workflowStore.isActive(target))
|
||||
target.changeTracker?.captureCanvasState()
|
||||
|
||||
await workflowStore.saveWorkflow(target)
|
||||
}
|
||||
@@ -173,7 +175,8 @@ export const useWorkflowService = () => {
|
||||
if (workflow.isTemporary) {
|
||||
await saveWorkflowAs(workflow)
|
||||
} else {
|
||||
if (workflowStore.isActive(workflow)) workflow.changeTracker?.checkState()
|
||||
if (workflowStore.isActive(workflow))
|
||||
workflow.changeTracker?.captureCanvasState()
|
||||
|
||||
const isApp = workflow.initialMode === 'app'
|
||||
const expectedPath =
|
||||
|
||||
@@ -15,7 +15,7 @@ import type { ComboInputSpec } from '@/schemas/nodeDef/nodeDefSchemaV2'
|
||||
import type { SimplifiedWidget } from '@/types/simplifiedWidget'
|
||||
import { createMockWidget } from './widgetTestUtils'
|
||||
|
||||
const mockCheckState = vi.hoisted(() => vi.fn())
|
||||
const mockCaptureCanvasState = vi.hoisted(() => vi.fn())
|
||||
const mockAssetsData = vi.hoisted(() => ({ items: [] as AssetItem[] }))
|
||||
|
||||
vi.mock('@/platform/workflow/management/stores/workflowStore', async () => {
|
||||
@@ -27,7 +27,7 @@ vi.mock('@/platform/workflow/management/stores/workflowStore', async () => {
|
||||
useWorkflowStore: () => ({
|
||||
activeWorkflow: {
|
||||
changeTracker: {
|
||||
checkState: mockCheckState
|
||||
captureCanvasState: mockCaptureCanvasState
|
||||
}
|
||||
}
|
||||
})
|
||||
@@ -767,10 +767,10 @@ describe('WidgetSelectDropdown undo tracking', () => {
|
||||
}
|
||||
|
||||
beforeEach(() => {
|
||||
mockCheckState.mockClear()
|
||||
mockCaptureCanvasState.mockClear()
|
||||
})
|
||||
|
||||
it('calls checkState after dropdown selection changes modelValue', () => {
|
||||
it('calls captureCanvasState after dropdown selection changes modelValue', () => {
|
||||
const widget = createMockWidget<string | undefined>({
|
||||
value: 'img_001.png',
|
||||
name: 'test_image',
|
||||
@@ -782,10 +782,10 @@ describe('WidgetSelectDropdown undo tracking', () => {
|
||||
wrapper.vm.updateSelectedItems(new Set(['input-1']))
|
||||
|
||||
expect(wrapper.emitted('update:modelValue')?.[0]).toEqual(['photo_abc.jpg'])
|
||||
expect(mockCheckState).toHaveBeenCalledOnce()
|
||||
expect(mockCaptureCanvasState).toHaveBeenCalledOnce()
|
||||
})
|
||||
|
||||
it('calls checkState after file upload completes', async () => {
|
||||
it('calls captureCanvasState after file upload completes', async () => {
|
||||
const { api } = await import('@/scripts/api')
|
||||
vi.mocked(api.fetchApi).mockResolvedValue({
|
||||
status: 200,
|
||||
@@ -804,6 +804,6 @@ describe('WidgetSelectDropdown undo tracking', () => {
|
||||
await wrapper.vm.handleFilesUpdate([file])
|
||||
|
||||
expect(wrapper.emitted('update:modelValue')?.[0]).toEqual(['uploaded.png'])
|
||||
expect(mockCheckState).toHaveBeenCalledOnce()
|
||||
expect(mockCaptureCanvasState).toHaveBeenCalledOnce()
|
||||
})
|
||||
})
|
||||
|
||||
@@ -432,7 +432,7 @@ function updateSelectedItems(selectedItems: Set<string>) {
|
||||
return
|
||||
}
|
||||
modelValue.value = name
|
||||
useWorkflowStore().activeWorkflow?.changeTracker?.checkState()
|
||||
useWorkflowStore().activeWorkflow?.changeTracker?.captureCanvasState()
|
||||
}
|
||||
|
||||
const uploadFile = async (
|
||||
@@ -509,7 +509,7 @@ async function handleFilesUpdate(files: File[]) {
|
||||
}
|
||||
|
||||
// 5. Snapshot undo state so the image change gets its own undo entry
|
||||
useWorkflowStore().activeWorkflow?.changeTracker?.checkState()
|
||||
useWorkflowStore().activeWorkflow?.changeTracker?.captureCanvasState()
|
||||
} catch (error) {
|
||||
console.error('Upload error:', error)
|
||||
toastStore.addAlert(`Upload failed: ${error}`)
|
||||
|
||||
@@ -29,11 +29,11 @@ logger.setLevel('info')
|
||||
export class ChangeTracker {
|
||||
static MAX_HISTORY = 50
|
||||
/**
|
||||
* Guard flag to prevent checkState from running during loadGraphData.
|
||||
* Guard flag to prevent captureCanvasState from running during loadGraphData.
|
||||
* Between rootGraph.configure() and afterLoadNewGraph(), the rootGraph
|
||||
* contains the NEW workflow's data while activeWorkflow still points to
|
||||
* the OLD workflow. Any checkState call in that window would serialize
|
||||
* the wrong graph into the old workflow's activeState, corrupting it.
|
||||
* the OLD workflow. Any captureCanvasState call in that window would
|
||||
* serialize the wrong graph into the old workflow's activeState, corrupting it.
|
||||
*/
|
||||
static isLoadingGraph = false
|
||||
/**
|
||||
@@ -138,8 +138,25 @@ export class ChangeTracker {
|
||||
}
|
||||
}
|
||||
|
||||
checkState() {
|
||||
/**
|
||||
* Snapshot the current canvas state into activeState and push undo.
|
||||
* INVARIANT: only the active workflow's tracker may read from the canvas.
|
||||
* Calling this on an inactive tracker would capture the wrong graph.
|
||||
*/
|
||||
captureCanvasState() {
|
||||
if (!app.graph || this.changeCount || ChangeTracker.isLoadingGraph) return
|
||||
|
||||
const activeTracker = useWorkflowStore().activeWorkflow?.changeTracker
|
||||
if (activeTracker !== this) {
|
||||
if (import.meta.env.DEV) {
|
||||
logger.error(
|
||||
'captureCanvasState called on inactive tracker for:',
|
||||
this.workflow.path
|
||||
)
|
||||
}
|
||||
return
|
||||
}
|
||||
|
||||
const currentState = clone(app.rootGraph.serialize()) as ComfyWorkflowJSON
|
||||
if (!this.activeState) {
|
||||
this.activeState = currentState
|
||||
@@ -217,14 +234,14 @@ export class ChangeTracker {
|
||||
|
||||
afterChange() {
|
||||
if (!--this.changeCount) {
|
||||
this.checkState()
|
||||
this.captureCanvasState()
|
||||
}
|
||||
}
|
||||
|
||||
static init() {
|
||||
const getCurrentChangeTracker = () =>
|
||||
useWorkflowStore().activeWorkflow?.changeTracker
|
||||
const checkState = () => getCurrentChangeTracker()?.checkState()
|
||||
const captureState = () => getCurrentChangeTracker()?.captureCanvasState()
|
||||
|
||||
let keyIgnored = false
|
||||
window.addEventListener(
|
||||
@@ -268,8 +285,8 @@ export class ChangeTracker {
|
||||
|
||||
// If our active element is some type of input then handle changes after they're done
|
||||
if (ChangeTracker.bindInput(bindInputEl)) return
|
||||
logger.debug('checkState on keydown')
|
||||
changeTracker.checkState()
|
||||
logger.debug('captureCanvasState on keydown')
|
||||
changeTracker.captureCanvasState()
|
||||
})
|
||||
},
|
||||
true
|
||||
@@ -278,34 +295,34 @@ export class ChangeTracker {
|
||||
window.addEventListener('keyup', () => {
|
||||
if (keyIgnored) {
|
||||
keyIgnored = false
|
||||
logger.debug('checkState on keyup')
|
||||
checkState()
|
||||
logger.debug('captureCanvasState on keyup')
|
||||
captureState()
|
||||
}
|
||||
})
|
||||
|
||||
// Handle clicking DOM elements (e.g. widgets)
|
||||
window.addEventListener('mouseup', () => {
|
||||
logger.debug('checkState on mouseup')
|
||||
checkState()
|
||||
logger.debug('captureCanvasState on mouseup')
|
||||
captureState()
|
||||
})
|
||||
|
||||
// Handle prompt queue event for dynamic widget changes
|
||||
api.addEventListener('promptQueued', () => {
|
||||
logger.debug('checkState on promptQueued')
|
||||
checkState()
|
||||
logger.debug('captureCanvasState on promptQueued')
|
||||
captureState()
|
||||
})
|
||||
|
||||
api.addEventListener('graphCleared', () => {
|
||||
logger.debug('checkState on graphCleared')
|
||||
checkState()
|
||||
logger.debug('captureCanvasState on graphCleared')
|
||||
captureState()
|
||||
})
|
||||
|
||||
// Handle litegraph clicks
|
||||
const processMouseUp = LGraphCanvas.prototype.processMouseUp
|
||||
LGraphCanvas.prototype.processMouseUp = function (e) {
|
||||
const v = processMouseUp.apply(this, [e])
|
||||
logger.debug('checkState on processMouseUp')
|
||||
checkState()
|
||||
logger.debug('captureCanvasState on processMouseUp')
|
||||
captureState()
|
||||
return v
|
||||
}
|
||||
|
||||
@@ -319,9 +336,9 @@ export class ChangeTracker {
|
||||
) {
|
||||
const extendedCallback = (v: string) => {
|
||||
callback(v)
|
||||
checkState()
|
||||
captureState()
|
||||
}
|
||||
logger.debug('checkState on prompt')
|
||||
logger.debug('captureCanvasState on prompt')
|
||||
return prompt.apply(this, [title, value, extendedCallback, event])
|
||||
}
|
||||
|
||||
@@ -329,8 +346,8 @@ 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()
|
||||
logger.debug('captureCanvasState on contextMenuClose')
|
||||
captureState()
|
||||
return v
|
||||
}
|
||||
|
||||
@@ -382,7 +399,7 @@ export class ChangeTracker {
|
||||
const htmlElement = activeEl as HTMLElement
|
||||
if (`on${evt}` in htmlElement) {
|
||||
const listener = () => {
|
||||
useWorkflowStore().activeWorkflow?.changeTracker?.checkState?.()
|
||||
useWorkflowStore().activeWorkflow?.changeTracker?.captureCanvasState?.()
|
||||
htmlElement.removeEventListener(evt, listener)
|
||||
}
|
||||
htmlElement.addEventListener(evt, listener)
|
||||
|
||||
@@ -351,29 +351,29 @@ describe('appModeStore', () => {
|
||||
})
|
||||
})
|
||||
|
||||
it('calls checkState when input is selected', async () => {
|
||||
it('calls captureCanvasState when input is selected', async () => {
|
||||
const workflow = createBuilderWorkflow()
|
||||
workflowStore.activeWorkflow = workflow
|
||||
await nextTick()
|
||||
vi.mocked(workflow.changeTracker!.checkState).mockClear()
|
||||
vi.mocked(workflow.changeTracker!.captureCanvasState).mockClear()
|
||||
|
||||
store.selectedInputs.push([42, 'prompt'])
|
||||
await nextTick()
|
||||
|
||||
expect(workflow.changeTracker!.checkState).toHaveBeenCalled()
|
||||
expect(workflow.changeTracker!.captureCanvasState).toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('calls checkState when input is deselected', async () => {
|
||||
it('calls captureCanvasState when input is deselected', async () => {
|
||||
const workflow = createBuilderWorkflow()
|
||||
workflowStore.activeWorkflow = workflow
|
||||
store.selectedInputs.push([42, 'prompt'])
|
||||
await nextTick()
|
||||
vi.mocked(workflow.changeTracker!.checkState).mockClear()
|
||||
vi.mocked(workflow.changeTracker!.captureCanvasState).mockClear()
|
||||
|
||||
store.selectedInputs.splice(0, 1)
|
||||
await nextTick()
|
||||
|
||||
expect(workflow.changeTracker!.checkState).toHaveBeenCalled()
|
||||
expect(workflow.changeTracker!.captureCanvasState).toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('reflects input changes in linearData', async () => {
|
||||
|
||||
@@ -89,7 +89,7 @@ export const useAppModeStore = defineStore('appMode', () => {
|
||||
inputs: [...data.inputs],
|
||||
outputs: [...data.outputs]
|
||||
}
|
||||
workflowStore.activeWorkflow?.changeTracker?.checkState()
|
||||
workflowStore.activeWorkflow?.changeTracker?.captureCanvasState()
|
||||
},
|
||||
{ deep: true }
|
||||
)
|
||||
|
||||
@@ -256,7 +256,7 @@ export function createMockChangeTracker(
|
||||
undoQueue: [],
|
||||
redoQueue: [],
|
||||
changeCount: 0,
|
||||
checkState: vi.fn(),
|
||||
captureCanvasState: vi.fn(),
|
||||
reset: vi.fn(),
|
||||
restore: vi.fn(),
|
||||
store: vi.fn(),
|
||||
|
||||
Reference in New Issue
Block a user