Skip to content

Commit

Permalink
[Refactor] Improve separation of concerns / lifecycle (#482)
Browse files Browse the repository at this point in the history
* Remove unused code

* Fix lifecycle - keep store simple

* Improve config lifecycle, convert to basic locator

* Remove static accessor

* Add separation of concerns

Separate dialog open code from config file load code.

* nit - Doc
  • Loading branch information
webfiltered authored Dec 14, 2024
1 parent e251941 commit 26b29c4
Show file tree
Hide file tree
Showing 5 changed files with 66 additions and 42 deletions.
4 changes: 2 additions & 2 deletions src/main-process/appWindow.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { AppWindowSettings } from '../store';
import log from 'electron-log/main';
import { IPC_CHANNELS, ProgressStatus, ServerArgs } from '../constants';
import { getAppResourcesPath } from '../install/resourcePaths';
import { DesktopConfig } from '../store/desktopConfig';
import { useDesktopConfig } from '../store/desktopConfig';
import type { ElectronContextMenuOptions } from '../preload';

/**
Expand All @@ -24,7 +24,7 @@ export class AppWindow {
private editMenu?: Menu;

public constructor() {
const installed = DesktopConfig.store.get('installState') === 'installed';
const installed = useDesktopConfig().get('installState') === 'installed';
const primaryDisplay = screen.getPrimaryDisplay();
const { width, height } = installed ? primaryDisplay.workAreaSize : { width: 1024, height: 768 };
const store = this.loadWindowStore();
Expand Down
32 changes: 15 additions & 17 deletions src/main-process/comfyDesktopApp.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import { DownloadManager } from '../models/DownloadManager';
import { VirtualEnvironment } from '../virtualEnvironment';
import { InstallWizard } from '../install/installWizard';
import { Terminal } from '../shell/terminal';
import { DesktopConfig } from '../store/desktopConfig';
import { useDesktopConfig } from '../store/desktopConfig';
import { InstallationValidator } from '../install/installationValidator';
import { restoreCustomNodes } from '../services/backup';

Expand Down Expand Up @@ -136,7 +136,7 @@ export class ComfyDesktopApp {
});
// Config
ipcMain.handle(IPC_CHANNELS.GET_GPU, async (_event): Promise<TorchDeviceType | undefined> => {
return await DesktopConfig.getAsync('detectedGpu');
return await useDesktopConfig().getAsync('detectedGpu');
});
// Restart core
ipcMain.handle(IPC_CHANNELS.RESTART_CORE, async (_event): Promise<boolean> => {
Expand All @@ -152,7 +152,7 @@ export class ComfyDesktopApp {
*/
static async install(appWindow: AppWindow): Promise<string> {
const validation = await validateHardware();
if (typeof validation?.gpu === 'string') DesktopConfig.store.set('detectedGpu', validation.gpu);
if (typeof validation?.gpu === 'string') useDesktopConfig().set('detectedGpu', validation.gpu);

if (!validation.isValid) {
await appWindow.loadRenderer('not-supported');
Expand All @@ -164,16 +164,15 @@ export class ComfyDesktopApp {
return new Promise<string>((resolve) => {
ipcMain.on(IPC_CHANNELS.INSTALL_COMFYUI, async (event, installOptions: InstallOptions) => {
const installWizard = new InstallWizard(installOptions);
const { store } = DesktopConfig;
store.set('basePath', installWizard.basePath);
useDesktopConfig().set('basePath', installWizard.basePath);

const { device } = installOptions;
if (device !== undefined) {
store.set('selectedDevice', device);
useDesktopConfig().set('selectedDevice', device);
}

await installWizard.install();
store.set('installState', 'installed');
useDesktopConfig().set('installState', 'installed');
appWindow.maximize();
resolve(installWizard.basePath);
});
Expand Down Expand Up @@ -201,8 +200,8 @@ export class ComfyDesktopApp {

this.appWindow.sendServerStartProgress(ProgressStatus.PYTHON_SETUP);

const { store } = DesktopConfig;
const selectedDevice = store.get('selectedDevice');
const config = useDesktopConfig();
const selectedDevice = config.get('selectedDevice');
const virtualEnvironment = new VirtualEnvironment(this.basePath, selectedDevice);

await virtualEnvironment.create({
Expand All @@ -216,13 +215,13 @@ export class ComfyDesktopApp {
},
});

if (!store.get('Comfy-Desktop.RestoredCustomNodes', false)) {
if (!config.get('Comfy-Desktop.RestoredCustomNodes', false)) {
try {
await restoreCustomNodes(virtualEnvironment, this.appWindow);
store.set('Comfy-Desktop.RestoredCustomNodes', true);
config.set('Comfy-Desktop.RestoredCustomNodes', true);
} catch (error) {
log.error('Failed to restore custom nodes:', error);
store.set('Comfy-Desktop.RestoredCustomNodes', false);
config.set('Comfy-Desktop.RestoredCustomNodes', false);
}
}

Expand All @@ -233,9 +232,8 @@ export class ComfyDesktopApp {
}

static async create(appWindow: AppWindow): Promise<ComfyDesktopApp> {
const { store } = DesktopConfig;
// Migrate settings from old version if required
const installState = store.get('installState') ?? (await ComfyDesktopApp.migrateInstallState());
const installState = useDesktopConfig().get('installState') ?? (await ComfyDesktopApp.migrateInstallState());

// Fresh install
const loadedPath = installState === undefined ? undefined : await ComfyDesktopApp.loadBasePath();
Expand All @@ -256,10 +254,10 @@ export class ComfyDesktopApp {
const basePath = await ComfyDesktopApp.loadBasePath();

// Migrate config
const { store } = DesktopConfig;
const config = useDesktopConfig();
const upgraded = 'upgraded';
store.set('installState', upgraded);
store.set('basePath', basePath);
config.set('installState', upgraded);
config.set('basePath', basePath);
return upgraded;
}

Expand Down
13 changes: 8 additions & 5 deletions src/main.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { IPC_CHANNELS, DEFAULT_SERVER_ARGS, ProgressStatus } from './constants';
import { app, dialog, ipcMain } from 'electron';
import { app, dialog, ipcMain, shell } from 'electron';
import log from 'electron-log/main';
import { findAvailablePort } from './utils';
import dotenv from 'dotenv';
Expand Down Expand Up @@ -41,12 +41,15 @@ if (!gotTheLock) {
app.on('ready', async () => {
log.debug('App ready');

const store = await DesktopConfig.load();
if (store) {
startApp();
} else {
try {
const store = await DesktopConfig.load(shell);
if (!store) throw new Error('Unknown error loading app config on startup.');
} catch (error) {
dialog.showErrorBox('User Data', `Unknown error whilst writing to user data folder:\n\n${error}`);
app.exit(20);
}

await startApp();
});
}

Expand Down
56 changes: 39 additions & 17 deletions src/store/desktopConfig.ts
Original file line number Diff line number Diff line change
@@ -1,31 +1,53 @@
import log from 'electron-log/main';
import ElectronStore from 'electron-store';
import { app, dialog, shell } from 'electron';
import { app, dialog } from 'electron';
import path from 'node:path';
import fs from 'fs/promises';
import type { DesktopSettings } from '.';
import type { TorchDeviceType } from '../preload';

/** Backing ref for the singleton config instance. */
let current: DesktopConfig;

/** Temporary service locator. DesktopConfig.load() must be called before access. */
export function useDesktopConfig() {
if (!current) throw new Error('Cannot access store before initialization.');
return current;
}

/** Handles loading of electron-store config, pre-window errors, and provides a non-null interface for the store. */
export class DesktopConfig {
static #store: ElectronStore<DesktopSettings> | undefined;
static get store(): ElectronStore<DesktopSettings> {
const store = this.#store;
if (!store) throw new Error('Cannot access store before initialization.');
return store;
#store: ElectronStore<DesktopSettings>;

private constructor(store: ElectronStore<DesktopSettings>) {
this.#store = store;
}

static get gpu(): TorchDeviceType | undefined {
return DesktopConfig.store.get('detectedGpu');
/** @inheritdoc {@link ElectronStore.get} */
get<Key extends keyof DesktopSettings>(key: Key, defaultValue?: Required<DesktopSettings>[Key]) {
return defaultValue === undefined ? this.#store.get(key) : this.#store.get(key, defaultValue);
}

/** @inheritdoc {@link ElectronStore.set} */
set<Key extends keyof DesktopSettings>(key: Key, value: Required<DesktopSettings>[Key]) {
return value === undefined ? this.#store.delete(key) : this.#store.set(key, value);
}

/**
* Static factory method. Loads the config from disk.
* @param shell Shell environment that can open file and folder views for the user
* @param options electron-store options to pass through to the backing store
* @throws On unknown error
*/
static async load(
shell: Electron.Shell,
options?: ConstructorParameters<typeof ElectronStore<DesktopSettings>>[0]
): Promise<ElectronStore<DesktopSettings> | undefined> {
): Promise<DesktopConfig | undefined> {
try {
DesktopConfig.#store = new ElectronStore<DesktopSettings>(options);
const store = new ElectronStore<DesktopSettings>(options);
current = new DesktopConfig(store);

return DesktopConfig.#store;
return current;
} catch (error) {
const configFilePath = path.join(getUserDataOrQuit(), `${options?.name ?? 'config'}.json`);

Expand All @@ -48,7 +70,7 @@ export class DesktopConfig {
await tryDeleteConfigFile(configFilePath);

// Causing a stack overflow from this recursion would take immense patience.
return DesktopConfig.load(options);
return DesktopConfig.load(shell, options);
}
}

Expand All @@ -57,7 +79,7 @@ export class DesktopConfig {
} else {
// Crash: Unknown filesystem error, permission denied on user data folder, etc
log.error(`Unknown error whilst loading configuration file: ${configFilePath}`, error);
dialog.showErrorBox('User Data', `Unknown error whilst writing to user data folder:\n\n${configFilePath}`);
throw new Error(configFilePath);
}
}
}
Expand All @@ -68,11 +90,11 @@ export class DesktopConfig {
* @param value The value to be saved. Must be valid.
* @returns A promise that resolves on successful save, or rejects with the first caught error.
*/
static async setAsync<Key extends keyof DesktopSettings>(key: Key, value: DesktopSettings[Key]): Promise<void> {
async setAsync<Key extends keyof DesktopSettings>(key: Key, value: DesktopSettings[Key]): Promise<void> {
return new Promise((resolve, reject) => {
log.info(`Saving setting: [${key}]`, value);
try {
DesktopConfig.store.set(key, value);
this.#store.set(key, value);
resolve();
} catch (error) {
reject(error);
Expand All @@ -81,10 +103,10 @@ export class DesktopConfig {
}

/** @inheritdoc {@link ElectronStore.get} */
static async getAsync<Key extends keyof DesktopSettings>(key: Key): Promise<DesktopSettings[Key]> {
async getAsync<Key extends keyof DesktopSettings>(key: Key): Promise<DesktopSettings[Key]> {
return new Promise((resolve, reject) => {
try {
resolve(DesktopConfig.store.get(key));
resolve(this.#store.get(key));
} catch (error) {
reject(error);
}
Expand Down
3 changes: 2 additions & 1 deletion src/store/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ export type AppWindowSettings = {
};

export type DesktopSettings = {
basePath?: string;
basePath?: string | null;
/**
* The state of the installation.
* - `started`: The installation has started.
Expand All @@ -25,4 +25,5 @@ export type DesktopSettings = {
detectedGpu?: GpuType;
/** The pytorch device that the user selected during installation. */
selectedDevice?: TorchDeviceType;
'Comfy-Desktop.RestoredCustomNodes': boolean;
};

0 comments on commit 26b29c4

Please sign in to comment.