mirror of
https://github.com/Comfy-Org/ComfyUI_frontend.git
synced 2026-01-26 19:09:52 +00:00
fix(workflow): avoid unloading active workflow during sync (#7875)
## Summary Fix issue where active workflow cannot be closed and new workflow tabs cannot be added after saving and syncing. - Never `unload()` the active workflow; still update its lastModified/size during sync - Skip `unload()` when metadata is unchanged: keep loaded content cached to avoid unnecessary reloads. - Added unit tests covering: - Active workflow is not unloaded during sync - Loaded workflows are not unloaded when metadata is unchanged ## Review Focus <!-- Critical design decisions or edge cases that need attention --> <!-- If this PR fixes an issue, uncomment and update the line below --> Fixes #7845 ## Screenshots (if applicable) **Before fix** active workflow cannot be closed and new tab button is unresponsive. https://github.com/user-attachments/assets/d14f2667-9c87-4b52-84cf-c5cd4cc0ad68 **After fix** https://github.com/user-attachments/assets/3d17cce8-6f8b-4fff-b8ab-d047bfbe92cb ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-7875-fix-workflow-avoid-unloading-active-workflow-during-sync-2e16d73d365081d48a83c0b306d59718) by [Unito](https://www.unito.io)
This commit is contained in:
@@ -54,6 +54,13 @@ describe('useWorkflowStore', () => {
|
||||
return await store.syncWorkflows()
|
||||
}
|
||||
|
||||
const syncRemoteWorkflowsWithMeta = async (
|
||||
files: Array<{ path: string; modified: number; size: number }>
|
||||
) => {
|
||||
vi.mocked(api.listUserDataFullInfo).mockResolvedValue(files)
|
||||
return await store.syncWorkflows()
|
||||
}
|
||||
|
||||
beforeEach(() => {
|
||||
setActivePinia(createPinia())
|
||||
store = useWorkflowStore()
|
||||
@@ -82,6 +89,78 @@ describe('useWorkflowStore', () => {
|
||||
expect(store.workflows.length).toBe(3)
|
||||
expect(store.workflows.filter((w) => w.isTemporary)).toEqual([workflow])
|
||||
})
|
||||
|
||||
it('should not unload the active workflow during sync', async () => {
|
||||
await syncRemoteWorkflowsWithMeta([
|
||||
{ path: 'a.json', modified: 100, size: 1 }
|
||||
])
|
||||
const workflow = store.getWorkflowByPath('workflows/a.json')!
|
||||
|
||||
vi.mocked(api.getUserData).mockResolvedValue({
|
||||
status: 200,
|
||||
text: () => Promise.resolve(defaultGraphJSON)
|
||||
} as Response)
|
||||
|
||||
await store.openWorkflow(workflow)
|
||||
expect(store.activeWorkflow?.path).toBe('workflows/a.json')
|
||||
expect(store.activeWorkflow?.isLoaded).toBe(true)
|
||||
|
||||
await syncRemoteWorkflowsWithMeta([
|
||||
{ path: 'a.json', modified: 200, size: 2 }
|
||||
])
|
||||
|
||||
expect(store.activeWorkflow?.isLoaded).toBe(true)
|
||||
expect(workflow.lastModified).toBe(200)
|
||||
expect(workflow.size).toBe(2)
|
||||
})
|
||||
|
||||
it('should not unload a loaded workflow if metadata has not changed', async () => {
|
||||
await syncRemoteWorkflowsWithMeta([
|
||||
{ path: 'a.json', modified: 100, size: 1 }
|
||||
])
|
||||
const workflow = store.getWorkflowByPath('workflows/a.json')!
|
||||
|
||||
vi.mocked(api.getUserData).mockResolvedValue({
|
||||
status: 200,
|
||||
text: () => Promise.resolve(defaultGraphJSON)
|
||||
} as Response)
|
||||
|
||||
await workflow.load()
|
||||
expect(workflow.isLoaded).toBe(true)
|
||||
|
||||
await syncRemoteWorkflowsWithMeta([
|
||||
{ path: 'a.json', modified: 100, size: 1 }
|
||||
])
|
||||
expect(workflow.isLoaded).toBe(true)
|
||||
})
|
||||
|
||||
it('should unload a loaded workflow if metadata has changed and it is not active', async () => {
|
||||
await syncRemoteWorkflowsWithMeta([
|
||||
{ path: 'a.json', modified: 100, size: 1 }
|
||||
])
|
||||
const workflow = store.getWorkflowByPath('workflows/a.json')!
|
||||
|
||||
vi.mocked(api.getUserData).mockResolvedValue({
|
||||
status: 200,
|
||||
text: () => Promise.resolve(defaultGraphJSON)
|
||||
} as Response)
|
||||
|
||||
await workflow.load()
|
||||
expect(workflow.isLoaded).toBe(true)
|
||||
expect(workflow.content).toBe(defaultGraphJSON)
|
||||
expect(workflow.originalContent).toBe(defaultGraphJSON)
|
||||
|
||||
// Metadata change should invalidate cached content for non-active workflows.
|
||||
await syncRemoteWorkflowsWithMeta([
|
||||
{ path: 'a.json', modified: 200, size: 2 }
|
||||
])
|
||||
|
||||
expect(workflow.lastModified).toBe(200)
|
||||
expect(workflow.size).toBe(2)
|
||||
expect(workflow.isLoaded).toBe(false)
|
||||
expect(workflow.content).toBeNull()
|
||||
expect(workflow.originalContent).toBeNull()
|
||||
})
|
||||
})
|
||||
|
||||
describe('createTemporary', () => {
|
||||
|
||||
@@ -513,8 +513,33 @@ export const useWorkflowStore = defineStore('workflow', () => {
|
||||
size: file.size
|
||||
}),
|
||||
(existingWorkflow, file) => {
|
||||
existingWorkflow.lastModified = file.modified
|
||||
existingWorkflow.size = file.size
|
||||
const isActiveWorkflow =
|
||||
activeWorkflow.value?.path === existingWorkflow.path
|
||||
|
||||
const nextLastModified = Math.max(
|
||||
existingWorkflow.lastModified,
|
||||
file.modified
|
||||
)
|
||||
|
||||
const isMetadataUnchanged =
|
||||
nextLastModified === existingWorkflow.lastModified &&
|
||||
file.size === existingWorkflow.size
|
||||
|
||||
if (!isMetadataUnchanged) {
|
||||
existingWorkflow.lastModified = nextLastModified
|
||||
existingWorkflow.size = file.size
|
||||
}
|
||||
|
||||
// Never unload the active workflow - it may contain unsaved in-memory edits.
|
||||
if (isActiveWorkflow) {
|
||||
return
|
||||
}
|
||||
|
||||
// If nothing changed, keep any loaded content cached.
|
||||
if (isMetadataUnchanged) {
|
||||
return
|
||||
}
|
||||
|
||||
existingWorkflow.unload()
|
||||
},
|
||||
/* exclude */ (workflow) => workflow.isTemporary
|
||||
|
||||
Reference in New Issue
Block a user