diff --git a/src/components/graph/GraphCanvas.vue b/src/components/graph/GraphCanvas.vue index 07c2b1399..1122fe85f 100644 --- a/src/components/graph/GraphCanvas.vue +++ b/src/components/graph/GraphCanvas.vue @@ -185,8 +185,9 @@ const colorPaletteStore = useColorPaletteStore() const colorPaletteService = useColorPaletteService() const canvasInteractions = useCanvasInteractions() const bootstrapStore = useBootstrapStore() -const { isI18nReady, i18nError, isSettingsReady, settingsError } = - storeToRefs(bootstrapStore) +const { isI18nReady, i18nError } = storeToRefs(bootstrapStore) +const { isReady: isSettingsReady, error: settingsError } = + storeToRefs(settingStore) const betaMenuEnabled = computed( () => settingStore.get('Comfy.UseNewMenu') !== 'Disabled' diff --git a/src/composables/useCoreCommands.test.ts b/src/composables/useCoreCommands.test.ts index b4c230bb2..6f1f3cfb6 100644 --- a/src/composables/useCoreCommands.test.ts +++ b/src/composables/useCoreCommands.test.ts @@ -167,25 +167,30 @@ describe('useCoreCommands', () => { return { get: vi.fn().mockReturnValue(getReturnValue), addSetting: vi.fn(), - loadSettingValues: vi.fn(), + load: vi.fn(), set: vi.fn(), exists: vi.fn(), getDefaultValue: vi.fn(), + isReady: true, + isLoading: false, + error: undefined, settingValues: {}, settingsById: {}, $id: 'setting', $state: { settingValues: {}, - settingsById: {} + settingsById: {}, + isReady: true, + isLoading: false, + error: undefined }, $patch: vi.fn(), $reset: vi.fn(), $subscribe: vi.fn(), $onAction: vi.fn(), $dispose: vi.fn(), - _customProperties: new Set(), - _p: {} - } as ReturnType + _customProperties: new Set() + } satisfies ReturnType } beforeEach(() => { diff --git a/src/platform/settings/settingStore.test.ts b/src/platform/settings/settingStore.test.ts index 16b827a78..8e1d26515 100644 --- a/src/platform/settings/settingStore.test.ts +++ b/src/platform/settings/settingStore.test.ts @@ -1,4 +1,5 @@ -import { createPinia, setActivePinia } from 'pinia' +import { createTestingPinia } from '@pinia/testing' +import { setActivePinia } from 'pinia' import { beforeEach, describe, expect, it, vi } from 'vitest' import { @@ -32,7 +33,7 @@ describe('useSettingStore', () => { let store: ReturnType beforeEach(() => { - setActivePinia(createPinia()) + setActivePinia(createTestingPinia({ stubActions: false })) store = useSettingStore() vi.clearAllMocks() }) @@ -42,18 +43,18 @@ describe('useSettingStore', () => { expect(store.settingsById).toEqual({}) }) - describe('loadSettingValues', () => { + describe('load', () => { it('should load settings from API', async () => { const mockSettings = { 'test.setting': 'value' } vi.mocked(api.getSettings).mockResolvedValue(mockSettings as any) - await store.loadSettingValues() + await store.load() expect(store.settingValues).toEqual(mockSettings) expect(api.getSettings).toHaveBeenCalled() }) - it('should throw error if settings are loaded after registration', async () => { + it('should set error if settings are loaded after registration', async () => { const setting: SettingParams = { id: 'test.setting', name: 'test.setting', @@ -62,9 +63,14 @@ describe('useSettingStore', () => { } store.addSetting(setting) - await expect(store.loadSettingValues()).rejects.toThrow( - 'Setting values must be loaded before any setting is registered.' - ) + await store.load() + + expect(store.error).toBeInstanceOf(Error) + if (store.error instanceof Error) { + expect(store.error.message).toBe( + 'Setting values must be loaded before any setting is registered.' + ) + } }) }) diff --git a/src/platform/settings/settingStore.ts b/src/platform/settings/settingStore.ts index c09740574..cbaf7d1fb 100644 --- a/src/platform/settings/settingStore.ts +++ b/src/platform/settings/settingStore.ts @@ -1,4 +1,5 @@ import _ from 'es-toolkit/compat' +import { useAsyncState } from '@vueuse/core' import { defineStore } from 'pinia' import { compare, valid } from 'semver' import { ref } from 'vue' @@ -47,6 +48,31 @@ export const useSettingStore = defineStore('setting', () => { const settingValues = ref>({}) const settingsById = ref>({}) + const { + isReady, + isLoading, + error, + execute: loadSettingValues + } = useAsyncState( + async () => { + if (Object.keys(settingsById.value).length) { + throw new Error( + 'Setting values must be loaded before any setting is registered.' + ) + } + settingValues.value = await api.getSettings() + await migrateZoomThresholdToFontSize() + }, + undefined, + { immediate: false } + ) + + async function load() { + if (!isReady.value && !isLoading.value) { + return loadSettingValues() + } + } + /** * Check if a setting's value exists, i.e. if the user has set it manually. * @param key - The key of the setting to check. @@ -188,22 +214,6 @@ export const useSettingStore = defineStore('setting', () => { onChange(setting, get(setting.id), undefined) } - /* - * Load setting values from server. - * This needs to be called before any setting is registered. - */ - async function loadSettingValues() { - if (Object.keys(settingsById.value).length) { - throw new Error( - 'Setting values must be loaded before any setting is registered.' - ) - } - settingValues.value = await api.getSettings() - - // Migrate old zoom threshold setting to new font size setting - await migrateZoomThresholdToFontSize() - } - /** * Migrate the old zoom threshold setting to the new font size setting. * Preserves the exact zoom threshold behavior by converting it to equivalent font size. @@ -246,8 +256,11 @@ export const useSettingStore = defineStore('setting', () => { return { settingValues, settingsById, + isReady, + isLoading, + error, + load, addSetting, - loadSettingValues, set, get, exists, diff --git a/src/stores/bootstrapStore.test.ts b/src/stores/bootstrapStore.test.ts index 8c59ecba2..3013dfbc8 100644 --- a/src/stores/bootstrapStore.test.ts +++ b/src/stores/bootstrapStore.test.ts @@ -1,5 +1,9 @@ -import { createPinia, setActivePinia } from 'pinia' +import { createTestingPinia } from '@pinia/testing' +import { setActivePinia } from 'pinia' import { beforeEach, describe, expect, it, vi } from 'vitest' +import { ref } from 'vue' + +import { useSettingStore } from '@/platform/settings/settingStore' import { useBootstrapStore } from './bootstrapStore' @@ -16,9 +20,16 @@ vi.mock('@/i18n', () => ({ mergeCustomNodesI18n: vi.fn() })) +const mockIsSettingsReady = ref(false) + vi.mock('@/platform/settings/settingStore', () => ({ useSettingStore: vi.fn(() => ({ - loadSettingValues: vi.fn().mockResolvedValue(undefined) + load: vi.fn(() => { + mockIsSettingsReady.value = true + }), + isReady: mockIsSettingsReady, + isLoading: ref(false), + error: ref(undefined) })) })) @@ -34,14 +45,16 @@ describe('bootstrapStore', () => { let store: ReturnType beforeEach(() => { - setActivePinia(createPinia()) + mockIsSettingsReady.value = false + setActivePinia(createTestingPinia({ stubActions: false })) store = useBootstrapStore() vi.clearAllMocks() }) it('initializes with all flags false', () => { + const settingStore = useSettingStore() expect(store.isNodeDefsReady).toBe(false) - expect(store.isSettingsReady).toBe(false) + expect(settingStore.isReady).toBe(false) expect(store.isI18nReady).toBe(false) }) @@ -58,10 +71,11 @@ describe('bootstrapStore', () => { }) it('starts store bootstrap (settings, i18n)', async () => { + const settingStore = useSettingStore() void store.startStoreBootstrap() await vi.waitFor(() => { - expect(store.isSettingsReady).toBe(true) + expect(settingStore.isReady).toBe(true) expect(store.isI18nReady).toBe(true) }) }) diff --git a/src/stores/bootstrapStore.ts b/src/stores/bootstrapStore.ts index 3448b84f6..43c9531eb 100644 --- a/src/stores/bootstrapStore.ts +++ b/src/stores/bootstrapStore.ts @@ -1,11 +1,14 @@ import { useAsyncState } from '@vueuse/core' import { defineStore } from 'pinia' +import { useSettingStore } from '@/platform/settings/settingStore' import type { ComfyNodeDef } from '@/schemas/nodeDefSchema' import { api } from '@/scripts/api' import { useUserStore } from '@/stores/userStore' export const useBootstrapStore = defineStore('bootstrap', () => { + const settingStore = useSettingStore() + const { state: nodeDefs, isReady: isNodeDefsReady, @@ -20,30 +23,6 @@ export const useBootstrapStore = defineStore('bootstrap', () => { { immediate: false } ) - const { - isReady: isSettingsReady, - isLoading: isSettingsLoading, - error: settingsError, - execute: executeLoadSettings - } = useAsyncState( - async () => { - const { useSettingStore } = - await import('@/platform/settings/settingStore') - await useSettingStore().loadSettingValues() - }, - undefined, - { immediate: false } - ) - - function loadSettings() { - // TODO: This check makes the store "sticky" across logouts. Add a reset - // method to clear isSettingsReady, then replace window.location.reload() - // with router.push() in SidebarLogoutIcon.vue - if (!isSettingsReady.value && !isSettingsLoading.value) { - void executeLoadSettings() - } - } - const { isReady: isI18nReady, error: i18nError, @@ -91,7 +70,7 @@ export const useBootstrapStore = defineStore('bootstrap', () => { void loadI18n() if (!userStore.needsLogin) { - loadSettings() + await settingStore.load() syncWorkflows() } } @@ -100,13 +79,9 @@ export const useBootstrapStore = defineStore('bootstrap', () => { nodeDefs, isNodeDefsReady, nodeDefsError, - isSettingsReady, - settingsError, isI18nReady, i18nError, startEarlyBootstrap, - startStoreBootstrap, - loadSettings, - syncWorkflows + startStoreBootstrap } })