mirror of
https://github.com/Comfy-Org/ComfyUI_frontend.git
synced 2026-04-19 22:09:37 +00:00
Don't use reactives for app mode selections (#10342)
- The DraggableList component takes a v-model. - When a drag is completed, it reassigns to v-model so an update event fires - App Builder sets v-model="appModeStore.selectedInputs" - Thus a completed drag operation effectively performs appModeStore.selectedInputs = newList - In appModeStore, selectedInputs is a reactive. Thus, after a drag/drop operation occurs, selectedInputs in the store is not the same value as appModeStore.selectedInputs When a reliable repro for the issue had not yet been found, an attempted earlier fix for this was to swap from watchers to directly updating `selectedInputs`. Since this change makes the code cleaner and still make the code safer, the change is left in place. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-10342-Don-t-use-watcher-for-loading-app-mode-selections-3296d73d365081f7b216e79c4f4f4e8d) by [Unito](https://www.unito.io)
This commit is contained in:
@@ -21,6 +21,7 @@ import { useDialogService } from '@/services/dialogService'
|
||||
import { useAppMode } from '@/composables/useAppMode'
|
||||
import type { AppMode } from '@/composables/useAppMode'
|
||||
import { useDomWidgetStore } from '@/stores/domWidgetStore'
|
||||
import { useAppModeStore } from '@/stores/appModeStore'
|
||||
import { useExecutionErrorStore } from '@/stores/executionErrorStore'
|
||||
import { useWorkspaceStore } from '@/stores/workspaceStore'
|
||||
import {
|
||||
@@ -403,6 +404,7 @@ export const useWorkflowService = () => {
|
||||
// Determine the initial app mode for fresh loads from serialized state.
|
||||
// null means linearMode was never explicitly set (not builder-saved).
|
||||
const freshLoadMode = linearModeToAppMode(workflowData.extra?.linearMode)
|
||||
useAppModeStore().loadSelections(workflowData.extra?.linearData)
|
||||
|
||||
function trackIfEnteringApp(workflow: ComfyWorkflow) {
|
||||
if (!wasAppMode && workflow.initialMode === 'app') {
|
||||
|
||||
@@ -197,14 +197,12 @@ describe('appModeStore', () => {
|
||||
id == 1 ? (node1 as unknown as LGraphNode) : undefined
|
||||
)
|
||||
|
||||
workflowStore.activeWorkflow = workflowWithLinearData(
|
||||
[
|
||||
store.loadSelections({
|
||||
inputs: [
|
||||
[1, 'prompt'],
|
||||
[99, 'width']
|
||||
],
|
||||
[]
|
||||
)
|
||||
await nextTick()
|
||||
]
|
||||
})
|
||||
|
||||
expect(store.selectedInputs).toEqual([[1, 'prompt']])
|
||||
})
|
||||
@@ -215,14 +213,12 @@ describe('appModeStore', () => {
|
||||
id == 1 ? (node1 as unknown as LGraphNode) : undefined
|
||||
)
|
||||
|
||||
workflowStore.activeWorkflow = workflowWithLinearData(
|
||||
[
|
||||
store.loadSelections({
|
||||
inputs: [
|
||||
[1, 'prompt'],
|
||||
[1, 'deleted_widget']
|
||||
],
|
||||
[]
|
||||
)
|
||||
await nextTick()
|
||||
]
|
||||
})
|
||||
|
||||
expect(store.selectedInputs).toEqual([
|
||||
[1, 'prompt'],
|
||||
@@ -236,8 +232,7 @@ describe('appModeStore', () => {
|
||||
id == 1 ? (node1 as unknown as LGraphNode) : undefined
|
||||
)
|
||||
|
||||
workflowStore.activeWorkflow = workflowWithLinearData([], [1, 99])
|
||||
await nextTick()
|
||||
store.loadSelections({ outputs: [1, 99] })
|
||||
|
||||
expect(store.selectedOutputs).toEqual([1])
|
||||
})
|
||||
@@ -247,8 +242,11 @@ describe('appModeStore', () => {
|
||||
|
||||
// Initially nodes are not resolvable — pruning removes them
|
||||
mockResolveNode.mockReturnValue(undefined)
|
||||
workflowStore.activeWorkflow = workflowWithLinearData([[1, 'seed']], [1])
|
||||
const inputs: [number, string][] = [[1, 'seed']]
|
||||
workflowStore.activeWorkflow = workflowWithLinearData(inputs, [1])
|
||||
store.loadSelections({ inputs })
|
||||
await nextTick()
|
||||
|
||||
expect(store.selectedInputs).toEqual([])
|
||||
expect(store.selectedOutputs).toEqual([])
|
||||
|
||||
@@ -268,8 +266,7 @@ describe('appModeStore', () => {
|
||||
it('hasOutputs is false when all output nodes are deleted', async () => {
|
||||
mockResolveNode.mockReturnValue(undefined)
|
||||
|
||||
workflowStore.activeWorkflow = workflowWithLinearData([], [10, 20])
|
||||
await nextTick()
|
||||
store.loadSelections({ outputs: [10, 20] })
|
||||
|
||||
expect(store.selectedOutputs).toEqual([])
|
||||
expect(store.hasOutputs).toBe(false)
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
import { defineStore } from 'pinia'
|
||||
import { reactive, computed, watch } from 'vue'
|
||||
import { ref, computed, watch } from 'vue'
|
||||
import { useEventListener } from '@vueuse/core'
|
||||
|
||||
import { useEmptyWorkflowDialog } from '@/components/builder/useEmptyWorkflowDialog'
|
||||
@@ -25,9 +25,9 @@ export const useAppModeStore = defineStore('appMode', () => {
|
||||
const { mode, setMode, isBuilderMode, isSelectMode } = useAppMode()
|
||||
const emptyWorkflowDialog = useEmptyWorkflowDialog()
|
||||
|
||||
const selectedInputs = reactive<[NodeId, string][]>([])
|
||||
const selectedOutputs = reactive<NodeId[]>([])
|
||||
const hasOutputs = computed(() => !!selectedOutputs.length)
|
||||
const selectedInputs = ref<[NodeId, string][]>([])
|
||||
const selectedOutputs = ref<NodeId[]>([])
|
||||
const hasOutputs = computed(() => !!selectedOutputs.value.length)
|
||||
const hasNodes = computed(() => {
|
||||
// Nodes are not reactive, so trigger recomputation when workflow changes
|
||||
void workflowStore.activeWorkflow
|
||||
@@ -54,8 +54,8 @@ export const useAppModeStore = defineStore('appMode', () => {
|
||||
|
||||
function loadSelections(data: Partial<LinearData> | undefined) {
|
||||
const { inputs, outputs } = pruneLinearData(data)
|
||||
selectedInputs.splice(0, selectedInputs.length, ...inputs)
|
||||
selectedOutputs.splice(0, selectedOutputs.length, ...outputs)
|
||||
selectedInputs.value = inputs
|
||||
selectedOutputs.value = outputs
|
||||
}
|
||||
|
||||
function resetSelectedToWorkflow() {
|
||||
@@ -65,20 +65,6 @@ export const useAppModeStore = defineStore('appMode', () => {
|
||||
loadSelections(activeWorkflow.changeTracker?.activeState?.extra?.linearData)
|
||||
}
|
||||
|
||||
watch(
|
||||
() => workflowStore.activeWorkflow,
|
||||
(newWorkflow) => {
|
||||
if (newWorkflow) {
|
||||
loadSelections(
|
||||
newWorkflow.changeTracker?.activeState?.extra?.linearData
|
||||
)
|
||||
} else {
|
||||
loadSelections(undefined)
|
||||
}
|
||||
},
|
||||
{ immediate: true }
|
||||
)
|
||||
|
||||
useEventListener(
|
||||
() => app.rootGraph?.events,
|
||||
'configured',
|
||||
@@ -88,7 +74,7 @@ export const useAppModeStore = defineStore('appMode', () => {
|
||||
watch(
|
||||
() =>
|
||||
isBuilderMode.value
|
||||
? { inputs: selectedInputs, outputs: selectedOutputs }
|
||||
? { inputs: selectedInputs.value, outputs: selectedOutputs.value }
|
||||
: null,
|
||||
(data) => {
|
||||
if (!data || ChangeTracker.isLoadingGraph) return
|
||||
@@ -144,10 +130,10 @@ export const useAppModeStore = defineStore('appMode', () => {
|
||||
const storeName = isPromotedWidgetView(widget)
|
||||
? widget.sourceWidgetName
|
||||
: widget.name
|
||||
const index = selectedInputs.findIndex(
|
||||
const index = selectedInputs.value.findIndex(
|
||||
([id, name]) => storeId == id && storeName === name
|
||||
)
|
||||
if (index !== -1) selectedInputs.splice(index, 1)
|
||||
if (index !== -1) selectedInputs.value.splice(index, 1)
|
||||
}
|
||||
|
||||
return {
|
||||
@@ -155,6 +141,7 @@ export const useAppModeStore = defineStore('appMode', () => {
|
||||
exitBuilder,
|
||||
hasNodes,
|
||||
hasOutputs,
|
||||
loadSelections,
|
||||
pruneLinearData,
|
||||
removeSelectedInput,
|
||||
resetSelectedToWorkflow,
|
||||
|
||||
Reference in New Issue
Block a user