-
Notifications
You must be signed in to change notification settings - Fork 3
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: config reset on app restart and improve migrations handling #394
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,30 +1,36 @@ | ||
import path from 'node:path'; | ||
import { FileSystemStorage } from '/shared/types/storage'; | ||
import { FileSystemStorage, type IFileSystemStorage } from '/shared/types/storage'; | ||
import { SETTINGS_DIRECTORY, CONFIG_FILE_NAME, getFullScenesPath } from '/shared/paths'; | ||
import { DEFAULT_CONFIG, mergeConfig } from '/shared/types/config'; | ||
import { DEFAULT_CONFIG, mergeConfig, type Config } from '/shared/types/config'; | ||
import { getUserDataPath } from './electron'; | ||
import { waitForMigrations } from './migrations'; | ||
import log from 'electron-log/main'; | ||
|
||
export const CONFIG_PATH = path.join(getUserDataPath(), SETTINGS_DIRECTORY, CONFIG_FILE_NAME); | ||
const storage = await FileSystemStorage.getOrCreate(CONFIG_PATH); | ||
|
||
// Initialize with default values if empty | ||
const existingConfig = await storage.get<Record<string, any>>(''); | ||
const defaultConfig = { ...DEFAULT_CONFIG }; | ||
defaultConfig.settings.scenesPath = getFullScenesPath(getUserDataPath()); | ||
let configStorage: IFileSystemStorage | undefined; | ||
|
||
if (!existingConfig || Object.keys(existingConfig).length === 0) { | ||
// Write the default config | ||
for (const [key, value] of Object.entries(defaultConfig)) { | ||
await storage.set(key, value); | ||
} | ||
} else { | ||
// Deep merge with defaults if config exists but might be missing properties | ||
const mergedConfig = mergeConfig(existingConfig, defaultConfig); | ||
if (JSON.stringify(existingConfig) !== JSON.stringify(mergedConfig)) { | ||
for (const [key, value] of Object.entries(mergedConfig)) { | ||
await storage.set(key, value); | ||
export async function getConfig(): Promise<IFileSystemStorage> { | ||
await waitForMigrations(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is just to avoid possible race conditions between the migration of the config and the initialization of the default values. |
||
|
||
if (!configStorage) { | ||
configStorage = await FileSystemStorage.getOrCreate(CONFIG_PATH); | ||
|
||
// Initialize with default values if empty or merge with defaults if partial | ||
const defaultConfig = { ...DEFAULT_CONFIG }; | ||
defaultConfig.settings.scenesPath = getFullScenesPath(getUserDataPath()); | ||
|
||
const existingConfig = await configStorage.getAll<Partial<Config>>(); | ||
|
||
// Deep merge with defaults if config exists but might be missing properties | ||
const mergedConfig = mergeConfig(existingConfig, defaultConfig); | ||
if (JSON.stringify(existingConfig) !== JSON.stringify(mergedConfig)) { | ||
log.info('[Config] Writing merged config to storage'); | ||
await configStorage.setAll(mergedConfig); | ||
} else { | ||
log.info('[Config] Config already exists and is up to date'); | ||
} | ||
} | ||
} | ||
|
||
export const config = storage; | ||
return configStorage; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -304,17 +304,18 @@ export async function duplicateProject(_path: string): Promise<Project> { | |
* @throws An error if the selected directory is not a valid project. | ||
*/ | ||
export async function importProject(): Promise<Project | undefined> { | ||
const config = await getConfig(); | ||
const [projectPath] = await invoke('electron.showOpenDialog', { | ||
title: 'Import project', | ||
properties: ['openDirectory'], | ||
defaultPath: config.settings.scenesPath, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a quality-of-life improvement for users |
||
}); | ||
|
||
const cancelled = !projectPath; | ||
|
||
if (cancelled) return undefined; | ||
|
||
const pathBaseName = path.basename(projectPath); | ||
const config = await getConfig(); | ||
const [projects] = await getProjects(config.workspace.paths); | ||
const projectAlreadyExists = projects.find($ => $.path === projectPath); | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This line was the bug, this always returns
undefined
, and causes the default config to be applied over the existing one.