mirror of
https://github.com/Comfy-Org/ComfyUI_frontend.git
synced 2026-04-28 02:02:08 +00:00
fix: Make feature flags reactive and fix manager UI state determination
- Create reactive featureFlagsStore for centralized feature flag management - Fix race conditions in feature flag updates with version tracking - Make managerStateStore properly reactive to feature flag changes - Add useManagerHelper composable as single entry point for opening manager - Fix Legacy UI keybinding fallback to new manager when appropriate - Update WebSocket handler to sync feature flags with reactive store - Add comprehensive tests for all manager UI states This ensures the Manager UI state (NEW_UI, LEGACY_UI, DISABLED) updates reactively when feature flags change, fixing the issue where manager state wasn't properly responding to server capability changes. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
@@ -1,3 +1,4 @@
|
||||
import { createPinia, setActivePinia } from 'pinia'
|
||||
import { beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import { isReactive, isReadonly } from 'vue'
|
||||
|
||||
@@ -5,18 +6,31 @@ import {
|
||||
ServerFeatureFlag,
|
||||
useFeatureFlags
|
||||
} from '@/composables/useFeatureFlags'
|
||||
import { api } from '@/scripts/api'
|
||||
import { useFeatureFlagsStore } from '@/stores/featureFlagsStore'
|
||||
|
||||
// Mock the API module
|
||||
vi.mock('@/scripts/api', () => ({
|
||||
api: {
|
||||
getServerFeature: vi.fn()
|
||||
}
|
||||
// Mock the store module
|
||||
vi.mock('@/stores/featureFlagsStore', () => ({
|
||||
useFeatureFlagsStore: vi.fn()
|
||||
}))
|
||||
|
||||
describe('useFeatureFlags', () => {
|
||||
let mockStore: any
|
||||
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks()
|
||||
setActivePinia(createPinia())
|
||||
|
||||
// Create mock store
|
||||
mockStore = {
|
||||
getServerFeature: vi.fn(),
|
||||
getClientFeature: vi.fn(),
|
||||
serverSupportsFeature: vi.fn(),
|
||||
supportsManagerV4: false,
|
||||
clientSupportsManagerV4UI: false,
|
||||
isReady: false
|
||||
}
|
||||
|
||||
vi.mocked(useFeatureFlagsStore).mockReturnValue(mockStore)
|
||||
})
|
||||
|
||||
describe('flags object', () => {
|
||||
@@ -28,69 +42,74 @@ describe('useFeatureFlags', () => {
|
||||
})
|
||||
|
||||
it('should access supportsPreviewMetadata', () => {
|
||||
vi.mocked(api.getServerFeature).mockImplementation(
|
||||
(path, defaultValue) => {
|
||||
if (path === ServerFeatureFlag.SUPPORTS_PREVIEW_METADATA)
|
||||
return true as any
|
||||
mockStore.getServerFeature.mockImplementation(
|
||||
(path: string, defaultValue?: any) => {
|
||||
if (path === ServerFeatureFlag.SUPPORTS_PREVIEW_METADATA) return true
|
||||
return defaultValue
|
||||
}
|
||||
)
|
||||
|
||||
const { flags } = useFeatureFlags()
|
||||
expect(flags.supportsPreviewMetadata).toBe(true)
|
||||
expect(api.getServerFeature).toHaveBeenCalledWith(
|
||||
ServerFeatureFlag.SUPPORTS_PREVIEW_METADATA
|
||||
expect(flags.value.supportsPreviewMetadata).toBe(true)
|
||||
expect(mockStore.getServerFeature).toHaveBeenCalledWith(
|
||||
ServerFeatureFlag.SUPPORTS_PREVIEW_METADATA,
|
||||
false
|
||||
)
|
||||
})
|
||||
|
||||
it('should access maxUploadSize', () => {
|
||||
vi.mocked(api.getServerFeature).mockImplementation(
|
||||
(path, defaultValue) => {
|
||||
if (path === ServerFeatureFlag.MAX_UPLOAD_SIZE)
|
||||
return 209715200 as any // 200MB
|
||||
mockStore.getServerFeature.mockImplementation(
|
||||
(path: string, defaultValue?: any) => {
|
||||
if (path === ServerFeatureFlag.MAX_UPLOAD_SIZE) return 209715200 // 200MB
|
||||
return defaultValue
|
||||
}
|
||||
)
|
||||
|
||||
const { flags } = useFeatureFlags()
|
||||
expect(flags.maxUploadSize).toBe(209715200)
|
||||
expect(api.getServerFeature).toHaveBeenCalledWith(
|
||||
expect(flags.value.maxUploadSize).toBe(209715200)
|
||||
expect(mockStore.getServerFeature).toHaveBeenCalledWith(
|
||||
ServerFeatureFlag.MAX_UPLOAD_SIZE
|
||||
)
|
||||
})
|
||||
|
||||
it('should access supportsManagerV4', () => {
|
||||
vi.mocked(api.getServerFeature).mockImplementation(
|
||||
(path, defaultValue) => {
|
||||
if (path === ServerFeatureFlag.MANAGER_SUPPORTS_V4) return true as any
|
||||
return defaultValue
|
||||
}
|
||||
)
|
||||
mockStore.supportsManagerV4 = true
|
||||
|
||||
const { flags } = useFeatureFlags()
|
||||
expect(flags.supportsManagerV4).toBe(true)
|
||||
expect(api.getServerFeature).toHaveBeenCalledWith(
|
||||
ServerFeatureFlag.MANAGER_SUPPORTS_V4
|
||||
)
|
||||
expect(flags.value.supportsManagerV4).toBe(true)
|
||||
})
|
||||
|
||||
it('should return undefined when features are not available and no default provided', () => {
|
||||
vi.mocked(api.getServerFeature).mockImplementation(
|
||||
(_path, defaultValue) => defaultValue as any
|
||||
it('should access clientSupportsManagerV4UI', () => {
|
||||
mockStore.clientSupportsManagerV4UI = true
|
||||
|
||||
const { flags } = useFeatureFlags()
|
||||
expect(flags.value.clientSupportsManagerV4UI).toBe(true)
|
||||
})
|
||||
|
||||
it('should access isReady state', () => {
|
||||
mockStore.isReady = true
|
||||
|
||||
const { flags } = useFeatureFlags()
|
||||
expect(flags.value.isReady).toBe(true)
|
||||
})
|
||||
|
||||
it('should return default values when features are not available', () => {
|
||||
mockStore.getServerFeature.mockImplementation(
|
||||
(_path: string, defaultValue?: any) => defaultValue
|
||||
)
|
||||
|
||||
const { flags } = useFeatureFlags()
|
||||
expect(flags.supportsPreviewMetadata).toBeUndefined()
|
||||
expect(flags.maxUploadSize).toBeUndefined()
|
||||
expect(flags.supportsManagerV4).toBeUndefined()
|
||||
expect(flags.value.supportsPreviewMetadata).toBe(false) // default value is false
|
||||
expect(flags.value.maxUploadSize).toBeUndefined()
|
||||
expect(flags.value.supportsManagerV4).toBe(false) // store mock returns false
|
||||
})
|
||||
})
|
||||
|
||||
describe('featureFlag', () => {
|
||||
it('should create reactive computed for custom feature flags', () => {
|
||||
vi.mocked(api.getServerFeature).mockImplementation(
|
||||
(path, defaultValue) => {
|
||||
if (path === 'custom.feature') return 'custom-value' as any
|
||||
mockStore.getServerFeature.mockImplementation(
|
||||
(path: string, defaultValue?: any) => {
|
||||
if (path === 'custom.feature') return 'custom-value'
|
||||
return defaultValue
|
||||
}
|
||||
)
|
||||
@@ -99,16 +118,16 @@ describe('useFeatureFlags', () => {
|
||||
const customFlag = featureFlag('custom.feature', 'default')
|
||||
|
||||
expect(customFlag.value).toBe('custom-value')
|
||||
expect(api.getServerFeature).toHaveBeenCalledWith(
|
||||
expect(mockStore.getServerFeature).toHaveBeenCalledWith(
|
||||
'custom.feature',
|
||||
'default'
|
||||
)
|
||||
})
|
||||
|
||||
it('should handle nested paths', () => {
|
||||
vi.mocked(api.getServerFeature).mockImplementation(
|
||||
(path, defaultValue) => {
|
||||
if (path === 'extension.custom.nested.feature') return true as any
|
||||
mockStore.getServerFeature.mockImplementation(
|
||||
(path: string, defaultValue?: any) => {
|
||||
if (path === 'extension.custom.nested.feature') return true
|
||||
return defaultValue
|
||||
}
|
||||
)
|
||||
@@ -120,10 +139,9 @@ describe('useFeatureFlags', () => {
|
||||
})
|
||||
|
||||
it('should work with ServerFeatureFlag enum', () => {
|
||||
vi.mocked(api.getServerFeature).mockImplementation(
|
||||
(path, defaultValue) => {
|
||||
if (path === ServerFeatureFlag.MAX_UPLOAD_SIZE)
|
||||
return 104857600 as any
|
||||
mockStore.getServerFeature.mockImplementation(
|
||||
(path: string, defaultValue?: any) => {
|
||||
if (path === ServerFeatureFlag.MAX_UPLOAD_SIZE) return 104857600
|
||||
return defaultValue
|
||||
}
|
||||
)
|
||||
@@ -134,4 +152,32 @@ describe('useFeatureFlags', () => {
|
||||
expect(maxUploadSize.value).toBe(104857600)
|
||||
})
|
||||
})
|
||||
|
||||
describe('serverSupportsFeature', () => {
|
||||
it('should create reactive computed for feature support check', () => {
|
||||
mockStore.serverSupportsFeature.mockImplementation(
|
||||
(path: string) => path === 'supported.feature'
|
||||
)
|
||||
|
||||
const { serverSupportsFeature } = useFeatureFlags()
|
||||
const isSupported = serverSupportsFeature('supported.feature')
|
||||
|
||||
expect(isSupported.value).toBe(true)
|
||||
expect(mockStore.serverSupportsFeature).toHaveBeenCalledWith(
|
||||
'supported.feature'
|
||||
)
|
||||
})
|
||||
})
|
||||
|
||||
describe('direct store methods', () => {
|
||||
it('should expose getServerFeature method', () => {
|
||||
const { getServerFeature } = useFeatureFlags()
|
||||
expect(getServerFeature).toBe(mockStore.getServerFeature)
|
||||
})
|
||||
|
||||
it('should expose getClientFeature method', () => {
|
||||
const { getClientFeature } = useFeatureFlags()
|
||||
expect(getClientFeature).toBe(mockStore.getClientFeature)
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
@@ -1,9 +1,8 @@
|
||||
import { createPinia, setActivePinia } from 'pinia'
|
||||
import { beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
|
||||
import { useFeatureFlags } from '@/composables/useFeatureFlags'
|
||||
import { api } from '@/scripts/api'
|
||||
import { useExtensionStore } from '@/stores/extensionStore'
|
||||
import { useFeatureFlagsStore } from '@/stores/featureFlagsStore'
|
||||
import {
|
||||
ManagerUIState,
|
||||
useManagerStateStore
|
||||
@@ -11,18 +10,8 @@ import {
|
||||
import { useSystemStatsStore } from '@/stores/systemStatsStore'
|
||||
|
||||
// Mock dependencies
|
||||
vi.mock('@/scripts/api', () => ({
|
||||
api: {
|
||||
getClientFeatureFlags: vi.fn(),
|
||||
getServerFeature: vi.fn()
|
||||
}
|
||||
}))
|
||||
|
||||
vi.mock('@/composables/useFeatureFlags', () => ({
|
||||
useFeatureFlags: vi.fn(() => ({
|
||||
flags: { supportsManagerV4: false },
|
||||
featureFlag: vi.fn()
|
||||
}))
|
||||
vi.mock('@/stores/featureFlagsStore', () => ({
|
||||
useFeatureFlagsStore: vi.fn()
|
||||
}))
|
||||
|
||||
vi.mock('@/stores/extensionStore', () => ({
|
||||
@@ -46,7 +35,11 @@ describe('useManagerStateStore', () => {
|
||||
system: { argv: ['python', 'main.py', '--disable-manager'] }
|
||||
}
|
||||
} as any)
|
||||
vi.mocked(api.getClientFeatureFlags).mockReturnValue({})
|
||||
vi.mocked(useFeatureFlagsStore).mockReturnValue({
|
||||
clientSupportsManagerV4UI: false,
|
||||
supportsManagerV4: false,
|
||||
isReady: false
|
||||
} as any)
|
||||
vi.mocked(useExtensionStore).mockReturnValue({
|
||||
extensions: []
|
||||
} as any)
|
||||
@@ -62,7 +55,11 @@ describe('useManagerStateStore', () => {
|
||||
system: { argv: ['python', 'main.py', '--enable-manager-legacy-ui'] }
|
||||
}
|
||||
} as any)
|
||||
vi.mocked(api.getClientFeatureFlags).mockReturnValue({})
|
||||
vi.mocked(useFeatureFlagsStore).mockReturnValue({
|
||||
clientSupportsManagerV4UI: false,
|
||||
supportsManagerV4: false,
|
||||
isReady: false
|
||||
} as any)
|
||||
vi.mocked(useExtensionStore).mockReturnValue({
|
||||
extensions: []
|
||||
} as any)
|
||||
@@ -76,13 +73,10 @@ describe('useManagerStateStore', () => {
|
||||
vi.mocked(useSystemStatsStore).mockReturnValue({
|
||||
systemStats: { system: { argv: ['python', 'main.py'] } }
|
||||
} as any)
|
||||
vi.mocked(api.getClientFeatureFlags).mockReturnValue({
|
||||
supports_manager_v4_ui: true
|
||||
})
|
||||
vi.mocked(api.getServerFeature).mockReturnValue(true)
|
||||
vi.mocked(useFeatureFlags).mockReturnValue({
|
||||
flags: { supportsManagerV4: true },
|
||||
featureFlag: vi.fn()
|
||||
vi.mocked(useFeatureFlagsStore).mockReturnValue({
|
||||
clientSupportsManagerV4UI: true,
|
||||
supportsManagerV4: true,
|
||||
isReady: true
|
||||
} as any)
|
||||
vi.mocked(useExtensionStore).mockReturnValue({
|
||||
extensions: []
|
||||
@@ -97,13 +91,10 @@ describe('useManagerStateStore', () => {
|
||||
vi.mocked(useSystemStatsStore).mockReturnValue({
|
||||
systemStats: { system: { argv: ['python', 'main.py'] } }
|
||||
} as any)
|
||||
vi.mocked(api.getClientFeatureFlags).mockReturnValue({
|
||||
supports_manager_v4_ui: false
|
||||
})
|
||||
vi.mocked(api.getServerFeature).mockReturnValue(true)
|
||||
vi.mocked(useFeatureFlags).mockReturnValue({
|
||||
flags: { supportsManagerV4: true },
|
||||
featureFlag: vi.fn()
|
||||
vi.mocked(useFeatureFlagsStore).mockReturnValue({
|
||||
clientSupportsManagerV4UI: false,
|
||||
supportsManagerV4: true,
|
||||
isReady: true
|
||||
} as any)
|
||||
vi.mocked(useExtensionStore).mockReturnValue({
|
||||
extensions: []
|
||||
@@ -118,10 +109,10 @@ describe('useManagerStateStore', () => {
|
||||
vi.mocked(useSystemStatsStore).mockReturnValue({
|
||||
systemStats: { system: { argv: ['python', 'main.py'] } }
|
||||
} as any)
|
||||
vi.mocked(api.getClientFeatureFlags).mockReturnValue({})
|
||||
vi.mocked(useFeatureFlags).mockReturnValue({
|
||||
flags: { supportsManagerV4: false },
|
||||
featureFlag: vi.fn()
|
||||
vi.mocked(useFeatureFlagsStore).mockReturnValue({
|
||||
clientSupportsManagerV4UI: false,
|
||||
supportsManagerV4: false,
|
||||
isReady: true
|
||||
} as any)
|
||||
vi.mocked(useExtensionStore).mockReturnValue({
|
||||
extensions: [{ name: 'Comfy.CustomNodesManager' }]
|
||||
@@ -136,11 +127,10 @@ describe('useManagerStateStore', () => {
|
||||
vi.mocked(useSystemStatsStore).mockReturnValue({
|
||||
systemStats: { system: { argv: ['python', 'main.py'] } }
|
||||
} as any)
|
||||
vi.mocked(api.getClientFeatureFlags).mockReturnValue({})
|
||||
vi.mocked(api.getServerFeature).mockReturnValue(undefined)
|
||||
vi.mocked(useFeatureFlags).mockReturnValue({
|
||||
flags: { supportsManagerV4: undefined },
|
||||
featureFlag: vi.fn()
|
||||
vi.mocked(useFeatureFlagsStore).mockReturnValue({
|
||||
clientSupportsManagerV4UI: false,
|
||||
supportsManagerV4: undefined,
|
||||
isReady: true
|
||||
} as any)
|
||||
vi.mocked(useExtensionStore).mockReturnValue({
|
||||
extensions: []
|
||||
@@ -155,11 +145,10 @@ describe('useManagerStateStore', () => {
|
||||
vi.mocked(useSystemStatsStore).mockReturnValue({
|
||||
systemStats: { system: { argv: ['python', 'main.py'] } }
|
||||
} as any)
|
||||
vi.mocked(api.getClientFeatureFlags).mockReturnValue({})
|
||||
vi.mocked(api.getServerFeature).mockReturnValue(false)
|
||||
vi.mocked(useFeatureFlags).mockReturnValue({
|
||||
flags: { supportsManagerV4: false },
|
||||
featureFlag: vi.fn()
|
||||
vi.mocked(useFeatureFlagsStore).mockReturnValue({
|
||||
clientSupportsManagerV4UI: false,
|
||||
supportsManagerV4: false,
|
||||
isReady: true
|
||||
} as any)
|
||||
vi.mocked(useExtensionStore).mockReturnValue({
|
||||
extensions: []
|
||||
@@ -174,13 +163,10 @@ describe('useManagerStateStore', () => {
|
||||
vi.mocked(useSystemStatsStore).mockReturnValue({
|
||||
systemStats: null
|
||||
} as any)
|
||||
vi.mocked(api.getClientFeatureFlags).mockReturnValue({
|
||||
supports_manager_v4_ui: true
|
||||
})
|
||||
vi.mocked(api.getServerFeature).mockReturnValue(true)
|
||||
vi.mocked(useFeatureFlags).mockReturnValue({
|
||||
flags: { supportsManagerV4: true },
|
||||
featureFlag: vi.fn()
|
||||
vi.mocked(useFeatureFlagsStore).mockReturnValue({
|
||||
clientSupportsManagerV4UI: true,
|
||||
supportsManagerV4: true,
|
||||
isReady: true
|
||||
} as any)
|
||||
vi.mocked(useExtensionStore).mockReturnValue({
|
||||
extensions: []
|
||||
|
||||
Reference in New Issue
Block a user