From 5059833298bd1239d69c3a352ec9470bb8f1ca4d Mon Sep 17 00:00:00 2001 From: filtered <176114999+webfiltered@users.noreply.github.com> Date: Sun, 15 Dec 2024 06:36:13 +1100 Subject: [PATCH] Standardise & simplify code (#488) nit - Standardise imports Fix test import / mock [Refactor] Assorted ESLint nits [Refactor] Simplify code - prefer spread, for-of nit - Standardise utf8 name nit - Standardise error var names nit - Doc --- scripts/env.mjs | 2 +- src/config/comfyConfigManager.ts | 8 ++++---- src/config/comfyServerConfig.ts | 13 +++++++------ src/config/comfySettings.ts | 6 +++--- src/handlers/pathHandlers.ts | 4 ++-- src/install/installWizard.ts | 4 ++-- src/install/installationValidator.ts | 2 +- src/install/resourcePaths.ts | 2 +- src/main-process/comfyDesktopApp.ts | 21 +++++++++------------ src/main-process/comfyServer.ts | 18 +++++++++--------- src/main.ts | 4 ++-- src/models/DownloadManager.ts | 6 +++--- src/preload.ts | 2 +- src/services/backup.ts | 16 ++++++++-------- src/shell/terminal.ts | 2 +- src/shell/util.ts | 2 +- src/store/desktopConfig.ts | 3 ++- src/utils.ts | 14 +++++++------- src/virtualEnvironment.ts | 22 +++++++++++----------- tests/unit/comfyConfigManager.test.ts | 4 ++-- tests/unit/comfyServerConfig.test.ts | 4 ++-- 21 files changed, 79 insertions(+), 80 deletions(-) diff --git a/scripts/env.mjs b/scripts/env.mjs index c215fb4b..5126767f 100644 --- a/scripts/env.mjs +++ b/scripts/env.mjs @@ -1,4 +1,4 @@ -import * as fs from 'node:fs/promises'; +import fs from 'node:fs/promises'; const envContent = `# env vars picked up by the ComfyUI executable on startup COMFYUI_CPU_ONLY=true diff --git a/src/config/comfyConfigManager.ts b/src/config/comfyConfigManager.ts index 118b058a..e1d4e61d 100644 --- a/src/config/comfyConfigManager.ts +++ b/src/config/comfyConfigManager.ts @@ -1,5 +1,5 @@ -import fs from 'fs'; -import path from 'path'; +import fs from 'node:fs'; +import path from 'node:path'; import log from 'electron-log/main'; export type DirectoryStructure = (string | DirectoryStructure)[]; @@ -70,7 +70,7 @@ export class ComfyConfigManager { } static createNestedDirectories(basePath: string, structure: DirectoryStructure): void { - structure.forEach((item) => { + for (const item of structure) { if (typeof item === 'string') { const dirPath = path.join(basePath, item); this.createDirIfNotExists(dirPath); @@ -88,7 +88,7 @@ export class ComfyConfigManager { } else { log.warn(`Invalid directory structure item: ${JSON.stringify(item)}`); } - }); + } } /** diff --git a/src/config/comfyServerConfig.ts b/src/config/comfyServerConfig.ts index 21036005..7982c936 100644 --- a/src/config/comfyServerConfig.ts +++ b/src/config/comfyServerConfig.ts @@ -1,5 +1,5 @@ -import * as fs from 'node:fs'; -import * as fsPromises from 'node:fs/promises'; +import fs from 'node:fs'; +import fsPromises from 'node:fs/promises'; import log from 'electron-log/main'; import yaml, { type YAMLParseError } from 'yaml'; import path from 'node:path'; @@ -155,10 +155,11 @@ export class ComfyServerConfig { } public static getBaseModelPathsFromRepoPath(repoPath: string): ModelPaths { - return knownModelKeys.reduce((acc, key) => { - acc[key] = path.join(repoPath, 'models', key) + path.sep; - return acc; - }, {} as ModelPaths); + const paths: ModelPaths = {}; + for (const key of knownModelKeys) { + paths[key] = path.join(repoPath, 'models', key) + path.sep; + } + return paths; } /** diff --git a/src/config/comfySettings.ts b/src/config/comfySettings.ts index cf85b9d5..6e492319 100644 --- a/src/config/comfySettings.ts +++ b/src/config/comfySettings.ts @@ -1,5 +1,5 @@ -import * as fs from 'fs/promises'; -import * as path from 'path'; +import fs from 'node:fs/promises'; +import path from 'node:path'; import log from 'electron-log/main'; export const DEFAULT_SETTINGS: ComfySettingsData = { @@ -40,7 +40,7 @@ export class ComfySettings { return; } try { - const fileContent = await fs.readFile(this.filePath, 'utf-8'); + const fileContent = await fs.readFile(this.filePath, 'utf8'); this.settings = JSON.parse(fileContent); } catch (error) { log.error(`Settings file cannot be loaded.`, error); diff --git a/src/handlers/pathHandlers.ts b/src/handlers/pathHandlers.ts index fe16f52f..8e33d39d 100644 --- a/src/handlers/pathHandlers.ts +++ b/src/handlers/pathHandlers.ts @@ -3,10 +3,10 @@ import { IPC_CHANNELS } from '../constants'; import log from 'electron-log/main'; import { ComfyServerConfig } from '../config/comfyServerConfig'; import type { SystemPaths } from '../preload'; -import fs from 'fs'; +import fs from 'node:fs'; import si from 'systeminformation'; import { ComfyConfigManager } from '../config/comfyConfigManager'; -import path from 'path'; +import path from 'node:path'; export class PathHandlers { static readonly REQUIRED_SPACE = 10 * 1024 * 1024 * 1024; // 10GB in bytes diff --git a/src/install/installWizard.ts b/src/install/installWizard.ts index 352238a0..b82cd41a 100644 --- a/src/install/installWizard.ts +++ b/src/install/installWizard.ts @@ -1,6 +1,6 @@ -import path from 'path'; +import path from 'node:path'; import log from 'electron-log/main'; -import fs from 'fs'; +import fs from 'node:fs'; import { InstallOptions } from '../preload'; import { DEFAULT_SETTINGS } from '../config/comfySettings'; import { ComfyServerConfig, ModelPaths } from '../config/comfyServerConfig'; diff --git a/src/install/installationValidator.ts b/src/install/installationValidator.ts index 2bf7ed5d..2c7ec75e 100644 --- a/src/install/installationValidator.ts +++ b/src/install/installationValidator.ts @@ -1,5 +1,5 @@ import { app, dialog, shell } from 'electron'; -import fs from 'fs/promises'; +import fs from 'node:fs/promises'; import log from 'electron-log/main'; import path from 'node:path'; diff --git a/src/install/resourcePaths.ts b/src/install/resourcePaths.ts index f5302561..fc3d4ad2 100644 --- a/src/install/resourcePaths.ts +++ b/src/install/resourcePaths.ts @@ -1,5 +1,5 @@ import { app } from 'electron'; -import path from 'path'; +import path from 'node:path'; export function getAppResourcesPath(): string { return app.isPackaged ? process.resourcesPath : path.join(app.getAppPath(), 'assets'); diff --git a/src/main-process/comfyDesktopApp.ts b/src/main-process/comfyDesktopApp.ts index d20c0d90..353d8fcc 100644 --- a/src/main-process/comfyDesktopApp.ts +++ b/src/main-process/comfyDesktopApp.ts @@ -8,9 +8,9 @@ import { ComfySettings } from '../config/comfySettings'; import { AppWindow } from './appWindow'; import { ComfyServer } from './comfyServer'; import { ComfyServerConfig } from '../config/comfyServerConfig'; -import fs from 'fs'; +import fs from 'node:fs'; import { InstallOptions, type ElectronContextMenuOptions, type TorchDeviceType } from '../preload'; -import path from 'path'; +import path from 'node:path'; import { ansiCodes, getModelsDirectory, validateHardware } from '../utils'; import { DownloadManager } from '../models/DownloadManager'; import { VirtualEnvironment } from '../virtualEnvironment'; @@ -86,8 +86,8 @@ export class ComfyDesktopApp { const allGpuInfo = { ...gpuInfo }; // Set Sentry context with all GPU information Sentry.setContext('gpus', allGpuInfo); - } catch (e) { - log.error('Error getting GPU info: ', e); + } catch (error) { + log.error('Error getting GPU info: ', error); } } @@ -136,8 +136,8 @@ export class ComfyDesktopApp { comfyorigin: 'core', }, }); - } catch (err) { - log.error('Failed to send error to Sentry:', err); + } catch (error_) { + log.error('Failed to send error to Sentry:', error_); return null; } } @@ -186,9 +186,7 @@ export class ComfyDesktopApp { appWindow.maximize(); resolve(installWizard.basePath); }) - .catch((reason) => { - reject(reason); - }); + .catch(reject); }); }); } @@ -290,9 +288,8 @@ export class ComfyDesktopApp { return null; case 'notFound': return null; - case 'error': default: - // Explain and quit + // 'error': Explain and quit // TODO: Support link? Something? await InstallationValidator.showInvalidFileAndQuit(ComfyServerConfig.configPath, { message: `Unable to read the YAML configuration file. Please ensure this file is available and can be read: @@ -304,7 +301,7 @@ If this problem persists, back up and delete the config file, then restart the a defaultId: 0, cancelId: 1, }); - throw new Error(/* Unreachable. */); + throw new Error('Unreachable'); } } diff --git a/src/main-process/comfyServer.ts b/src/main-process/comfyServer.ts index c7614e9c..40a1550d 100644 --- a/src/main-process/comfyServer.ts +++ b/src/main-process/comfyServer.ts @@ -4,11 +4,11 @@ import { VirtualEnvironment } from '../virtualEnvironment'; import { ansiCodes, rotateLogFiles } from '../utils'; import { getAppResourcesPath } from '../install/resourcePaths'; import log from 'electron-log/main'; -import path from 'path'; +import path from 'node:path'; import { ComfyServerConfig } from '../config/comfyServerConfig'; import { AppWindow } from './appWindow'; import waitOn from 'wait-on'; -import { ChildProcess } from 'child_process'; +import { ChildProcess } from 'node:child_process'; export class ComfyServer { /** @@ -80,13 +80,13 @@ export class ComfyServer { } static buildLaunchArgs(mainScriptPath: string, args: Record) { - return [mainScriptPath].concat( - Object.entries(args) - .map(([key, value]) => [`--${key}`, value]) - .flat() + return [ + mainScriptPath, + ...Object.entries(args) + .flatMap(([key, value]) => [`--${key}`, value]) // Empty string values are ignored. e.g. { cpu: '' } => '--cpu' - .filter((value: string) => value !== '') - ); + .filter((value: string) => value !== ''), + ]; } get launchArgs() { @@ -164,7 +164,7 @@ export class ComfyServer { // Set up a timeout in case the process doesn't exit const timeout = setTimeout(() => { reject(new Error('Timeout: Python server did not exit within 10 seconds')); - }, 10000); + }, 10_000); // Listen for the 'exit' event this.comfyServerProcess.once('exit', (code, signal) => { diff --git a/src/main.ts b/src/main.ts index f671f814..bdd2cc44 100644 --- a/src/main.ts +++ b/src/main.ts @@ -41,8 +41,8 @@ if (!gotTheLock) { app.on('ready', () => { log.debug('App ready'); - startApp().catch((reason) => { - log.error('Unhandled exception in app startup', reason); + startApp().catch((error) => { + log.error('Unhandled exception in app startup', error); app.exit(2020); }); }); diff --git a/src/models/DownloadManager.ts b/src/models/DownloadManager.ts index bb236dc7..2550bb3a 100644 --- a/src/models/DownloadManager.ts +++ b/src/models/DownloadManager.ts @@ -1,6 +1,6 @@ import { session, DownloadItem, ipcMain } from 'electron'; -import * as path from 'path'; -import * as fs from 'fs'; +import path from 'node:path'; +import fs from 'node:fs'; import { IPC_CHANNELS } from '../constants'; import log from 'electron-log/main'; import type { AppWindow } from '../main-process/appWindow'; @@ -223,7 +223,7 @@ export class DownloadManager { } getAllDownloads(): DownloadState[] { - return Array.from(this.downloads.values()) + return [...this.downloads.values()] .filter((download) => download.item !== null) .map((download) => ({ url: download.url, diff --git a/src/preload.ts b/src/preload.ts index 5d1b5591..ead4d9f8 100644 --- a/src/preload.ts +++ b/src/preload.ts @@ -71,7 +71,7 @@ const electronAPI = { return ipcRenderer.invoke(IPC_CHANNELS.IS_PACKAGED); }, //Emulates app.ispackaged in renderer restartApp: (customMessage?: string, delay?: number): void => { - console.log('Sending restarting app message to main process with custom message: ', customMessage); + console.log('Sending restarting app message to main process with custom message:', customMessage); ipcRenderer.send(IPC_CHANNELS.RESTART_APP, { customMessage, delay }); }, reinstall: () => { diff --git a/src/services/backup.ts b/src/services/backup.ts index c0abfc61..1389f0a8 100644 --- a/src/services/backup.ts +++ b/src/services/backup.ts @@ -1,6 +1,6 @@ -import * as path from 'path'; -import * as fs from 'fs'; -import * as glob from 'glob'; +import path from 'node:path'; +import fs from 'node:fs'; +import glob from 'glob'; import { app } from 'electron'; import { VirtualEnvironment } from '../virtualEnvironment'; import { getAppResourcesPath } from '../install/resourcePaths'; @@ -12,11 +12,11 @@ import { ansiCodes } from '../utils'; function parseLogFile(logPath: string): Set { console.log('Parsing log file:', logPath); const customNodes = new Set(); - const content = fs.readFileSync(logPath, 'utf-8'); + const content = fs.readFileSync(logPath, 'utf8'); const lines = content.split('\n'); - for (let i = 0; i < lines.length; i++) { - const line = lines[i].trim(); + for (const rawLine of lines) { + const line = rawLine.trim(); // Match the exact format from Python's "{:6.1f} seconds" const timeMatch = line.match(/\s*\d+\.\d+\s+seconds/); if (timeMatch) { @@ -92,9 +92,9 @@ export async function restoreCustomNodes(virtualEnvironment: VirtualEnvironment, const customNodes = new Set(); for (const logFile of logFiles) { const nodes = parseLogFile(logFile); - nodes.forEach((node) => customNodes.add(node)); + for (const node of nodes) customNodes.add(node); } log.info('Found custom nodes:', customNodes); - await installCustomNodes(Array.from(customNodes), virtualEnvironment, appWindow); + await installCustomNodes([...customNodes], virtualEnvironment, appWindow); } diff --git a/src/shell/terminal.ts b/src/shell/terminal.ts index 5f3e4a92..9ce3029d 100644 --- a/src/shell/terminal.ts +++ b/src/shell/terminal.ts @@ -1,4 +1,4 @@ -import * as pty from 'node-pty'; +import pty from 'node-pty'; import { AppWindow } from '../main-process/appWindow'; import { IPC_CHANNELS } from '../constants'; import { getDefaultShell } from './util'; diff --git a/src/shell/util.ts b/src/shell/util.ts index 567376a6..ed638d57 100644 --- a/src/shell/util.ts +++ b/src/shell/util.ts @@ -1,4 +1,4 @@ -import os from 'os'; +import os from 'node:os'; export function getDefaultShell(): string { switch (os.platform()) { diff --git a/src/store/desktopConfig.ts b/src/store/desktopConfig.ts index 93e4e93d..35dcb435 100644 --- a/src/store/desktopConfig.ts +++ b/src/store/desktopConfig.ts @@ -2,7 +2,7 @@ import log from 'electron-log/main'; import ElectronStore from 'electron-store'; import { app, dialog } from 'electron'; import path from 'node:path'; -import fs from 'fs/promises'; +import fs from 'node:fs/promises'; import type { DesktopSettings } from '.'; /** Backing ref for the singleton config instance. */ @@ -36,6 +36,7 @@ export class DesktopConfig { * 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 + * @returns The newly created instance, or `undefined` on error. * @throws On unknown error */ static async load( diff --git a/src/utils.ts b/src/utils.ts index b637cde0..ba68e84c 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -1,14 +1,14 @@ -import * as net from 'net'; -import * as fsPromises from 'node:fs/promises'; +import net from 'node:net'; +import fsPromises from 'node:fs/promises'; import path from 'node:path'; -import fs from 'fs'; +import fs from 'node:fs'; import si from 'systeminformation'; -import { exec } from 'child_process'; -import { promisify } from 'util'; +import { exec } from 'node:child_process'; +import { promisify } from 'node:util'; import log from 'electron-log/main'; import type { GpuType } from './preload'; -export const ansiCodes = /[\u001b\u009b][[()#;?]*(?:[0-9]{1,4}(?:;[0-9]{0,4})*)?[0-9A-ORZcf-nqry=><]/g; +export const ansiCodes = /[\u001B\u009B][#();?[]*(?:\d{1,4}(?:;\d{0,4})*)?[\d<=>A-ORZcf-nqry]/g; export async function pathAccessible(path: string): Promise { try { @@ -82,7 +82,7 @@ export async function rotateLogFiles(logDir: string, baseName: string, maxFiles } } - const timestamp = new Date().toISOString().replace(/[:.]/g, '-'); + const timestamp = new Date().toISOString().replaceAll(/[.:]/g, '-'); const newLogPath = path.join(logDir, `${baseName}_${timestamp}.log`); await fsPromises.rename(currentLogPath, newLogPath); } diff --git a/src/virtualEnvironment.ts b/src/virtualEnvironment.ts index a8b2a2af..1511d9e7 100644 --- a/src/virtualEnvironment.ts +++ b/src/virtualEnvironment.ts @@ -1,10 +1,10 @@ -import * as path from 'node:path'; +import path from 'node:path'; import { spawn, ChildProcess } from 'node:child_process'; import log from 'electron-log/main'; import { pathAccessible } from './utils'; import { app } from 'electron'; -import * as pty from 'node-pty'; -import * as os from 'os'; +import pty from 'node-pty'; +import os from 'node:os'; import { getDefaultShell } from './shell/util'; import type { TorchDeviceType } from './preload'; @@ -197,7 +197,7 @@ export class VirtualEnvironment { pythonInterpreterPath, args, { - PYTHONIOENCODING: 'utf-8', + PYTHONIOENCODING: 'utf8', }, callbacks ); @@ -216,7 +216,7 @@ export class VirtualEnvironment { this.pythonInterpreterPath, args, { - PYTHONIOENCODING: 'utf-8', + PYTHONIOENCODING: 'utf8', }, callbacks ); @@ -234,7 +234,7 @@ export class VirtualEnvironment { } private async runPtyCommandAsync(command: string, onData?: (data: string) => void): Promise<{ exitCode: number }> { - const id = new Date().valueOf(); + const id = Date.now(); return new Promise((res) => { const endMarker = `--end-${id}:`; const input = `clear; ${command}; echo "${endMarker}$?"`; @@ -242,7 +242,7 @@ export class VirtualEnvironment { const lines = data.split('\n'); for (const line of lines) { // Remove ansi sequences to see if this the exit marker - const clean = line.replace(/\u001b\[[0-9;?]*[a-zA-Z]/g, ''); + const clean = line.replaceAll(/\u001B\[[\d;?]*[A-Za-z]/g, ''); if (clean.startsWith(endMarker)) { const exit = clean.substring(endMarker.length).trim(); let exitCode: number; @@ -253,8 +253,8 @@ export class VirtualEnvironment { exitCode = -999; } else { // Bash should output a number - exitCode = parseInt(exit); - if (isNaN(exitCode)) { + exitCode = Number.parseInt(exit); + if (Number.isNaN(exitCode)) { console.warn('Unable to parse exit code:', exit); exitCode = -998; } @@ -315,8 +315,8 @@ export class VirtualEnvironment { resolve({ exitCode: code }); }); - childProcess.on('error', (err) => { - reject(err); + childProcess.on('error', (error) => { + reject(error); }); }); } diff --git a/tests/unit/comfyConfigManager.test.ts b/tests/unit/comfyConfigManager.test.ts index e547b946..070e8e6f 100644 --- a/tests/unit/comfyConfigManager.test.ts +++ b/tests/unit/comfyConfigManager.test.ts @@ -1,8 +1,8 @@ -import fs from 'fs'; +import fs from 'node:fs'; import { ComfyConfigManager, DirectoryStructure } from '../../src/config/comfyConfigManager'; // Mock the fs module -jest.mock('fs'); +jest.mock('node:fs'); jest.mock('electron-log/main', () => ({ info: jest.fn(), error: jest.fn(), diff --git a/tests/unit/comfyServerConfig.test.ts b/tests/unit/comfyServerConfig.test.ts index 6a53b493..35e7b2dc 100644 --- a/tests/unit/comfyServerConfig.test.ts +++ b/tests/unit/comfyServerConfig.test.ts @@ -6,9 +6,9 @@ jest.mock('electron', () => ({ })); import { app } from 'electron'; -import path from 'path'; +import path from 'node:path'; import { ComfyServerConfig } from '../../src/config/comfyServerConfig'; -import * as fsPromises from 'node:fs/promises'; +import fsPromises from 'node:fs/promises'; describe('ComfyServerConfig', () => { describe('configPath', () => {