[refactor] Consolidate settings migration logic into dedicated utility

- Extract all migrateDeprecatedValue logic from individual settings into centralized settingsMigration.ts
- Remove migrateDeprecatedValue from SettingParams interface and coreSettings definitions
- Simplify settingStore by removing tryMigrateDeprecatedValue function
- Ensure migrations run after loadSettingValues() for clean initialization flow
- Update tests to reflect new migration approach

This refactor centralizes all setting value migrations in one place, making them easier to maintain and avoiding timing issues with settings that don't exist yet.
This commit is contained in:
Benjamin Lu
2025-07-01 18:47:04 -04:00
parent 4c177121a6
commit 472f90799d
6 changed files with 96 additions and 57 deletions

View File

@@ -86,6 +86,7 @@ import { useSettingStore } from '@/stores/settingStore'
import { useToastStore } from '@/stores/toastStore'
import { useColorPaletteStore } from '@/stores/workspace/colorPaletteStore'
import { useWorkspaceStore } from '@/stores/workspaceStore'
import { runSettingMigrations } from '@/utils/migration/settingsMigration'
const emit = defineEmits<{
ready: []
@@ -297,6 +298,10 @@ onMounted(async () => {
throw error
}
}
// Run cross-setting migrations
await runSettingMigrations()
CORE_SETTINGS.forEach((setting) => {
settingStore.addSetting(setting)
})

View File

@@ -430,14 +430,7 @@ export const CORE_SETTINGS: SettingParams[] = [
defaultValue: 'Top',
name: 'Use new menu',
type: 'combo',
options: ['Disabled', 'Top', 'Bottom'],
migrateDeprecatedValue: (value: string) => {
// Floating is now supported by dragging the docked actionbar off.
if (value === 'Floating') {
return 'Top'
}
return value
}
options: ['Disabled', 'Top', 'Bottom']
},
{
id: 'Comfy.Workflow.WorkflowTabsPosition',
@@ -470,15 +463,7 @@ export const CORE_SETTINGS: SettingParams[] = [
type: 'hidden',
defaultValue: [] as Keybinding[],
versionAdded: '1.3.7',
versionModified: '1.7.3',
migrateDeprecatedValue: (value: any[]) => {
return value.map((keybinding) => {
if (keybinding['targetSelector'] === '#graph-canvas') {
keybinding['targetElementId'] = 'graph-canvas'
}
return keybinding
})
}
versionModified: '1.7.3'
},
{
id: 'Comfy.Keybinding.NewBindings',
@@ -716,11 +701,7 @@ export const CORE_SETTINGS: SettingParams[] = [
name: 'The active color palette id',
type: 'hidden',
defaultValue: 'dark',
versionModified: '1.6.7',
migrateDeprecatedValue(value: string) {
// Legacy custom palettes were prefixed with 'custom_'
return value.startsWith('custom_') ? value.replace('custom_', '') : value
}
versionModified: '1.6.7'
},
{
id: 'Comfy.CustomColorPalettes',

View File

@@ -20,10 +20,6 @@ export interface SettingTreeNode extends TreeNode {
data?: SettingParams
}
function tryMigrateDeprecatedValue(setting: SettingParams, value: any) {
return setting?.migrateDeprecatedValue?.(value) ?? value
}
function onChange(setting: SettingParams, newValue: any, oldValue: any) {
if (setting?.onChange) {
setting.onChange(newValue, oldValue)
@@ -54,16 +50,12 @@ export const useSettingStore = defineStore('setting', () => {
async function set<K extends keyof Settings>(key: K, value: Settings[K]) {
// Clone the incoming value to prevent external mutations
const clonedValue = _.cloneDeep(value)
const newValue = tryMigrateDeprecatedValue(
settingsById.value[key],
clonedValue
)
const oldValue = get(key)
if (newValue === oldValue) return
if (clonedValue === oldValue) return
onChange(settingsById.value[key], newValue, oldValue)
settingValues.value[key] = newValue
await api.storeSetting(key, newValue)
onChange(settingsById.value[key], clonedValue, oldValue)
settingValues.value[key] = clonedValue
await api.storeSetting(key, clonedValue)
}
/**
@@ -102,12 +94,7 @@ export const useSettingStore = defineStore('setting', () => {
settingsById.value[setting.id] = setting
if (settingValues.value[setting.id] !== undefined) {
settingValues.value[setting.id] = tryMigrateDeprecatedValue(
setting,
settingValues.value[setting.id]
)
}
// Trigger onChange callback with current value
onChange(setting, get(setting.id), undefined)
}

View File

@@ -43,8 +43,6 @@ export interface SettingParams extends FormItem {
category?: string[]
experimental?: boolean
deprecated?: boolean
// Deprecated values are mapped to new values.
migrateDeprecatedValue?: (value: any) => any
// Version of the setting when it was added
versionAdded?: string
// Version of the setting when it was last modified

View File

@@ -0,0 +1,83 @@
import type { Keybinding } from '@/schemas/keyBindingSchema'
import { useSettingStore } from '@/stores/settingStore'
export interface SettingMigration {
condition: () => boolean
migrate: () => Promise<void>
}
/**
* Setting value migrations that transform deprecated values to new formats.
* These run after settings are loaded from the server but before they are
* registered, ensuring settings have valid values when the app initializes.
*/
export const SETTING_MIGRATIONS: SettingMigration[] = [
// Migrate Comfy.UseNewMenu "Floating" value to "Top"
{
condition: () => {
const settingStore = useSettingStore()
return (
settingStore.exists('Comfy.UseNewMenu') &&
(settingStore.get('Comfy.UseNewMenu') as string) === 'Floating'
)
},
migrate: async () => {
const settingStore = useSettingStore()
await settingStore.set('Comfy.UseNewMenu', 'Top')
}
},
// Migrate Comfy.Keybinding.UnsetBindings targetSelector to targetElementId
{
condition: () => {
const settingStore = useSettingStore()
if (!settingStore.exists('Comfy.Keybinding.UnsetBindings')) return false
const keybindings = settingStore.get(
'Comfy.Keybinding.UnsetBindings'
) as Keybinding[]
return keybindings.some((kb: any) => 'targetSelector' in kb)
},
migrate: async () => {
const settingStore = useSettingStore()
const keybindings = settingStore.get(
'Comfy.Keybinding.UnsetBindings'
) as any[]
const migrated = keybindings.map((keybinding) => {
if (keybinding['targetSelector'] === '#graph-canvas') {
keybinding['targetElementId'] = 'graph-canvas'
delete keybinding['targetSelector']
}
return keybinding
})
await settingStore.set('Comfy.Keybinding.UnsetBindings', migrated)
}
},
// Migrate Comfy.ColorPalette custom_ prefix
{
condition: () => {
const settingStore = useSettingStore()
return (
settingStore.exists('Comfy.ColorPalette') &&
(settingStore.get('Comfy.ColorPalette') as string).startsWith('custom_')
)
},
migrate: async () => {
const settingStore = useSettingStore()
const value = settingStore.get('Comfy.ColorPalette') as string
await settingStore.set('Comfy.ColorPalette', value.replace('custom_', ''))
}
}
]
/**
* Runs all setting migrations that meet their conditions.
* This is called after loadSettingValues() to ensure all deprecated
* setting values are migrated to their current format before the
* application starts using them.
*/
export async function runSettingMigrations(): Promise<void> {
for (const migration of SETTING_MIGRATIONS) {
if (migration.condition()) {
await migration.migrate()
}
}
}

View File

@@ -92,21 +92,6 @@ describe('useSettingStore', () => {
'Setting test.setting must have a unique ID.'
)
})
it('should migrate deprecated values', () => {
const setting: SettingParams = {
id: 'test.setting',
name: 'test.setting',
type: 'text',
defaultValue: 'default',
migrateDeprecatedValue: (value: string) => value.toUpperCase()
}
store.settingValues['test.setting'] = 'oldvalue'
store.addSetting(setting)
expect(store.settingValues['test.setting']).toBe('OLDVALUE')
})
})
describe('get and set', () => {