From 219036c963aa657d16b496a5eef8c58c4fa60337 Mon Sep 17 00:00:00 2001 From: David Sanders Date: Thu, 7 May 2026 11:42:23 -0400 Subject: [PATCH 01/22] chore: add new IPC to get fiddle start params from renderer --- src/ambient.d.ts | 7 ++++-- src/interfaces.ts | 13 +++++----- src/ipc-events.ts | 1 + src/main/utils/get-start-fiddle-options.ts | 28 ++++++++++++++++++++++ src/preload/preload.ts | 13 +++++++--- src/renderer/runner.ts | 28 ++++++++++++++++++++++ tests/mocks/electron-fiddle.ts | 1 + 7 files changed, 80 insertions(+), 11 deletions(-) create mode 100644 src/main/utils/get-start-fiddle-options.ts diff --git a/src/ambient.d.ts b/src/ambient.d.ts index c057d91701..e3290510ad 100644 --- a/src/ambient.d.ts +++ b/src/ambient.d.ts @@ -22,7 +22,7 @@ import { RunnableVersion, SelectedLocalVersion, SemVer, - StartFiddleParams, + StartFiddleOptions, TestRequest, Version, } from './interfaces'; @@ -151,6 +151,9 @@ declare global { transforms: Array, ) => Promise<{ localPath?: string; files: Files }>, ); + onGetStartFiddleOptions( + callback: () => Promise, + ): void; openThemeFolder(): Promise; packageRun( { dir, packageManager }: PMOperationOptions, @@ -169,7 +172,7 @@ declare global { setShowMeTemplate(template?: string): void; showWarningDialog(messageOptions: MessageOptions): void; showWindow(): void; - startFiddle(params: StartFiddleParams): Promise; + startFiddle(): Promise; stopFiddle(): void; taskDone(result: RunResult): void; themePath: string; diff --git a/src/interfaces.ts b/src/interfaces.ts index f850f1c1d6..01115256b4 100644 --- a/src/interfaces.ts +++ b/src/interfaces.ts @@ -289,14 +289,15 @@ export interface PackageJsonOptions { includeDependencies?: boolean; } -export interface StartFiddleParams { - localPath: string | undefined; +export interface StartFiddleOptions { + version: string; enableElectronLogging: boolean; - isValidBuild: boolean; // If the localPath is a valid Electron build - version: string; // The user selected version - dir: string; - options: string[]; + executionFlags: string[]; env: { [x: string]: string | undefined }; + modules: Array<[string, string]>; + packageManager: IPackageManager; + useSocketFirewall: boolean; + isKeepingUserDataDirs: boolean; } export interface DownloadVersionParams { diff --git a/src/ipc-events.ts b/src/ipc-events.ts index c8626dcb46..0a6d26a2fc 100644 --- a/src/ipc-events.ts +++ b/src/ipc-events.ts @@ -69,6 +69,7 @@ export enum IpcEvents { SAVE_FILES_TO_TEMP = 'SAVE_FILES_TO_TEMP', SAVED_LOCAL_FIDDLE = 'SAVED_LOCAL_FIDDLE', GET_FILES = 'GET_FILES', + GET_START_FIDDLE_OPTIONS = 'GET_START_FIDDLE_OPTIONS', START_FIDDLE = 'START_FIDDLE', STOP_FIDDLE = 'STOP_FIDDLE', GET_VERSION_STATE = 'GET_VERSION_STATE', diff --git a/src/main/utils/get-start-fiddle-options.ts b/src/main/utils/get-start-fiddle-options.ts new file mode 100644 index 0000000000..d68c2c0eba --- /dev/null +++ b/src/main/utils/get-start-fiddle-options.ts @@ -0,0 +1,28 @@ +import { MessageChannelMain } from 'electron'; + +import { StartFiddleOptions } from '../../interfaces'; +import { IpcEvents } from '../../ipc-events'; +import { ipcMainManager } from '../ipc'; + +/** + * Asks the renderer for everything the main process needs to run the + * current fiddle (version, env, modules, packageManager, etc.). + */ +export function getStartFiddleOptions( + webContents: Electron.WebContents, +): Promise { + return new Promise((resolve) => { + const { port1, port2 } = new MessageChannelMain(); + ipcMainManager.postMessage( + IpcEvents.GET_START_FIDDLE_OPTIONS, + undefined, + [port1], + webContents, + ); + port2.once('message', (event) => { + resolve(event.data as StartFiddleOptions); + port2.close(); + }); + port2.start(); + }); +} diff --git a/src/preload/preload.ts b/src/preload/preload.ts index 3ee7a942ea..7f083b079b 100644 --- a/src/preload/preload.ts +++ b/src/preload/preload.ts @@ -14,7 +14,7 @@ import { PackageJsonOptions, RunResult, RunnableVersion, - StartFiddleParams, + StartFiddleOptions, Version, } from '../interfaces'; import { IpcEvents, WEBCONTENTS_READY_FOR_IPC_SIGNAL } from '../ipc-events'; @@ -194,6 +194,13 @@ export async function setupFiddleGlobal() { }, ); }, + onGetStartFiddleOptions(callback: () => Promise) { + ipcRenderer.removeAllListeners(IpcEvents.GET_START_FIDDLE_OPTIONS); + ipcRenderer.on(IpcEvents.GET_START_FIDDLE_OPTIONS, async (e) => { + const options = await callback(); + e.ports[0].postMessage(options); + }); + }, async openThemeFolder() { await ipcRenderer.invoke(IpcEvents.OPEN_THEME_FOLDER); }, @@ -246,8 +253,8 @@ export async function setupFiddleGlobal() { showWindow() { ipcRenderer.send(IpcEvents.SHOW_WINDOW); }, - async startFiddle(params: StartFiddleParams) { - await ipcRenderer.invoke(IpcEvents.START_FIDDLE, params); + async startFiddle() { + await ipcRenderer.invoke(IpcEvents.START_FIDDLE); }, stopFiddle() { ipcRenderer.send(IpcEvents.STOP_FIDDLE); diff --git a/src/renderer/runner.ts b/src/renderer/runner.ts index b15290484f..346706efa1 100644 --- a/src/renderer/runner.ts +++ b/src/renderer/runner.ts @@ -12,6 +12,7 @@ import { PackageJsonOptions, RunResult, RunnableVersion, + StartFiddleOptions, VersionSource, } from '../interfaces'; @@ -37,6 +38,7 @@ export class Runner { constructor(private readonly appState: AppState) { this.run = this.run.bind(this); this.stop = this.stop.bind(this); + this.getStartFiddleOptions = this.getStartFiddleOptions.bind(this); window.ElectronFiddle.removeAllListeners('run-fiddle'); window.ElectronFiddle.removeAllListeners('package-fiddle'); @@ -49,6 +51,8 @@ export class Runner { window.ElectronFiddle.addEventListener('make-fiddle', () => { this.performForgeOperation(ForgeCommands.MAKE); }); + + window.ElectronFiddle.onGetStartFiddleOptions(this.getStartFiddleOptions); } /** @@ -431,6 +435,30 @@ export class Runner { }); } + /** + * Build the options object the main process needs to run the current + * fiddle. Sent in response to a request from main. + */ + public async getStartFiddleOptions(): Promise { + const { appState } = this; + + const modules = Array.from(appState.modules.entries()); + if (modules.length > 0) { + appState.isInstallingModules = true; + } + + return { + version: appState.currentElectronVersion.version, + enableElectronLogging: appState.isEnablingElectronLogging, + executionFlags: [...appState.executionFlags], + env: this.buildChildEnvVars(), + modules, + packageManager: appState.packageManager, + useSocketFirewall: appState.isUsingSocketFirewall, + isKeepingUserDataDirs: appState.isKeepingUserDataDirs, + }; + } + /** * Save files to temp, logging to the Fiddle terminal while doing so */ diff --git a/tests/mocks/electron-fiddle.ts b/tests/mocks/electron-fiddle.ts index 2b93779bd7..5dce2849cf 100644 --- a/tests/mocks/electron-fiddle.ts +++ b/tests/mocks/electron-fiddle.ts @@ -34,6 +34,7 @@ export class ElectronFiddleMock { public getVersionState = vi.fn(); public macTitlebarClicked = vi.fn(); public onGetFiles = vi.fn(); + public onGetStartFiddleOptions = vi.fn(); public openThemeFolder = vi.fn(); public packageRun = vi.fn(); public platform = process.platform; From b44c80aab413adc36f346801a8caaf731ac2446d Mon Sep 17 00:00:00 2001 From: David Sanders Date: Thu, 7 May 2026 11:44:12 -0400 Subject: [PATCH 02/22] chore: remove unneeded // eslint-disable-next-line --- src/main/fiddle-core.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/main/fiddle-core.ts b/src/main/fiddle-core.ts index b02b7a3793..990c6b3d81 100644 --- a/src/main/fiddle-core.ts +++ b/src/main/fiddle-core.ts @@ -56,7 +56,6 @@ function pushOutput(webContents: WebContents, message: string): void { ipcMainManager.send(IpcEvents.FIDDLE_RUNNER_OUTPUT, [message], webContents); } -// eslint-disable-next-line @typescript-eslint/no-unused-vars function pushOutputLine(webContents: WebContents, message: string): void { pushOutput(webContents, `${message}\n`); } From 3101b281abe198ec72ded8a2d94d4b40ed4cda53 Mon Sep 17 00:00:00 2001 From: David Sanders Date: Thu, 7 May 2026 12:39:09 -0400 Subject: [PATCH 03/22] chore: refactor to start fiddle in main process --- src/main/fiddle-core.ts | 214 ++++++++++++++++++----- src/renderer/runner.ts | 161 +++-------------- tests/main/fiddle-core.spec.ts | 310 ++++++++++++++++++++++----------- tests/renderer/runner.spec.tsx | 99 ++--------- 4 files changed, 421 insertions(+), 363 deletions(-) diff --git a/src/main/fiddle-core.ts b/src/main/fiddle-core.ts index 990c6b3d81..f49a506890 100644 --- a/src/main/fiddle-core.ts +++ b/src/main/fiddle-core.ts @@ -1,7 +1,5 @@ import { ChildProcess } from 'node:child_process'; import * as fs from 'node:fs'; -import * as os from 'node:os'; -import * as path from 'node:path'; import { ElectronVersions, Installer, Runner } from '@electron/fiddle-core'; import { @@ -12,13 +10,26 @@ import { } from 'electron'; import { ELECTRON_DOWNLOAD_PATH, ELECTRON_INSTALL_PATH } from './constants'; +import { cleanupDirectory, deleteUserData, saveFilesToTemp } from './files'; import { ipcMainManager } from './ipc'; +import { addModules, getIsPackageManagerInstalled } from './npm'; +import { getFiles } from './utils/get-files'; +import { getStartFiddleOptions } from './utils/get-start-fiddle-options'; +import { getLocalVersions } from './versions'; import { DownloadVersionParams, + IPackageManager, + PACKAGE_NAME, ProgressObject, - StartFiddleParams, } from '../interfaces'; import { IpcEvents } from '../ipc-events'; +import { maybePlural } from '../renderer/utils/plural-maybe'; + +export interface PMOperationOptions { + dir: string; + packageManager: IPackageManager; + useSocketFirewall?: boolean; +} let installer: Installer; let runner: Runner; @@ -33,16 +44,6 @@ const BLOCKED_ENV_KEYS = new Set([ 'DYLD_LIBRARY_PATH', ]); -/** - * Returns true if `dir` resolves to a path inside the OS temp directory. - */ -function isInsideTempDir(dir: unknown): dir is string { - if (typeof dir !== 'string') return false; - const tmpDir = fs.realpathSync(os.tmpdir()); - const resolved = fs.realpathSync(path.resolve(dir)); - return resolved.startsWith(tmpDir + path.sep) || resolved === tmpDir; -} - // Keep track of which fiddle process belongs to which WebContents const fiddleProcesses = new WeakMap(); @@ -52,37 +53,164 @@ const removingVersions = new Map>(); /** * Push to the renderer's run output. */ -function pushOutput(webContents: WebContents, message: string): void { - ipcMainManager.send(IpcEvents.FIDDLE_RUNNER_OUTPUT, [message], webContents); +function pushOutput( + webContents: WebContents, + message: string, + options?: { isNotPre?: boolean }, +): void { + ipcMainManager.send( + IpcEvents.FIDDLE_RUNNER_OUTPUT, + [message, options], + webContents, + ); } -function pushOutputLine(webContents: WebContents, message: string): void { - pushOutput(webContents, `${message}\n`); +function pushOutputLine( + webContents: WebContents, + message: string, + options?: { isNotPre?: boolean }, +): void { + pushOutput(webContents, `${message}\n`, options); } /** - * Start running an Electron fiddle. + * Little convenience method that pushes message and error. */ -export async function startFiddle( +function pushError(webContents: WebContents, message: string, error: Error) { + pushOutput(webContents, `⚠️ ${message}. Error encountered:`); + pushOutput(webContents, error.toString()); + console.warn(error); +} + +/** + * Installs the specified modules + */ +export async function installModules( webContents: WebContents, - params: StartFiddleParams, + modulesPairs: [string, string][], + options: PMOperationOptions, ): Promise { - const { dir, enableElectronLogging, localPath, options, version } = params; + const modules = modulesPairs.map(([pkg, version]) => `${pkg}@${version}`); + + // Install any modules the user added to the fiddle. + if (modules.length > 0) { + const pmInstalled = await getIsPackageManagerInstalled( + options.packageManager, + ); + if (!pmInstalled) { + let message = `The ${maybePlural(`module`, modules)} ${modules.join( + ', ', + )} need to be installed, `; + message += `but we could not find ${options.packageManager}. Fiddle requires Node.js and npm `; + message += `to support the installation of modules not included in `; + message += `Electron. Please visit https://nodejs.org to install Node.js `; + message += `and npm, or https://classic.yarnpkg.com/lang/en/ to install Yarn`; + + pushOutput(webContents, message, { isNotPre: true }); + throw new Error('Package manager not installed'); + } - // Guard 1: working directory must be inside the OS temp directory. - if (!isInsideTempDir(dir)) { - throw new Error(`startFiddle: dir must be inside the temp directory`); + pushOutput( + webContents, + `Installing node modules using ${ + options.packageManager + }: ${modules.join(', ')}...`, + { isNotPre: true }, + ); + + const result = await addModules( + { + dir: options.dir, + packageManager: options.packageManager, + useSocketFirewall: options.useSocketFirewall, + }, + ...modules, + ); + if (result) pushOutputLine(webContents, result); } +} - // Guard 2: determine whether to use the local path by verifying the - // executable exists on disk — do not trust the renderer-supplied - // isValidBuild flag. +/** + * Drive the entire run lifecycle from main: ask the renderer for the + * settings and files, write them to a temp directory, install modules, + * spawn Electron, and clean everything up when the process exits. + */ +export async function startFiddle(webContents: WebContents): Promise { + const options = await getStartFiddleOptions(webContents); + const { + enableElectronLogging, + env: rendererEnv, + executionFlags, + isKeepingUserDataDirs, + modules, + packageManager, + useSocketFirewall, + version, + } = options; + + // Look up local Electron builds by version string. Local builds use a + // version of the form `0.0.0-local.`, so only consult the + // stored local versions when the version string contains `-local`. + const localPath = version.includes('-local') + ? getLocalVersions().find((v) => v.version === version)?.localPath + : undefined; + + // Verify the executable exists on disk before using it. const resolvedExec = localPath ? Installer.getExecPath(localPath) : undefined; const useLocalPath = !!resolvedExec && fs.existsSync(resolvedExec); - // Guard 3: strip any CLI option containing a null byte, which can - // truncate strings at the OS level. - const safeOptions = options.filter( + // Get the fiddle's files from the renderer. + const files = new Map( + (await getFiles(BrowserWindow.fromWebContents(webContents)!, [])).files, + ); + + // Pull the project name out of the fiddle's package.json — that's the + // name Electron will use for its user-data dir, so that's what we + // need to delete on cleanup. + let appName: string | undefined; + try { + const pkg = JSON.parse(files.get(PACKAGE_NAME) ?? '{}'); + if (typeof pkg.name === 'string') appName = pkg.name; + } catch { + // package.json is malformed; skip user-data cleanup. + } + + // Create the temp directory and write files into it. This is the only + // place that creates the run directory — the renderer never sees it. + pushOutputLine(webContents, 'Saving files to temp directory...'); + const dir = await saveFilesToTemp(files); + pushOutputLine(webContents, `Saved files to ${dir}`); + + const cleanup = async () => { + await cleanupDirectory(dir); + if (!appName) return; + if (isKeepingUserDataDirs) { + console.log( + `Cleanup: Not deleting data dir due to isKeepingUserDataDirs setting`, + ); + return; + } + console.log(`Cleanup: Deleting data dir for ${appName}`); + await deleteUserData(appName); + }; + + try { + await installModules(webContents, modules, { + dir, + packageManager, + useSocketFirewall, + }); + } catch (error: any) { + console.error('Runner: Could not install modules', error); + + pushError(webContents, 'Could not install modules', error); + await cleanup(); + throw error; + } + + // Strip any CLI option containing a null byte, which can truncate + // strings at the OS level. + const safeOptions = [dir, '--inspect', ...executionFlags].filter( (opt) => typeof opt === 'string' && !opt.includes('\0'), ); @@ -99,23 +227,31 @@ export async function startFiddle( } const safeEnv = Object.fromEntries( - Object.entries(params.env).filter(([key]) => !BLOCKED_ENV_KEYS.has(key)), + Object.entries(rendererEnv).filter(([key]) => !BLOCKED_ENV_KEYS.has(key)), ); Object.assign(env, safeEnv); - const child = await runner.spawn( - useLocalPath ? resolvedExec! : version, - dir, - { args: safeOptions, cwd: dir, env }, - ); + let child: ChildProcess; + try { + child = await runner.spawn(useLocalPath ? resolvedExec! : version, dir, { + args: safeOptions, + cwd: dir, + env, + }); + } catch (error) { + await cleanup(); + throw error; + } fiddleProcesses.set(webContents, child); + pushOutputLine(webContents, `Electron v${version} started as "${appName}"`); + child.stdout?.on('data', (data) => pushOutput(webContents, data.toString())); child.stderr?.on('data', (data) => pushOutput(webContents, data.toString())); child.on('close', async (code, signal) => { fiddleProcesses.delete(webContents); - + await cleanup(); ipcMainManager.send(IpcEvents.FIDDLE_STOPPED, [code, signal], webContents); }); } @@ -220,8 +356,8 @@ export async function setupFiddleCore(versions: ElectronVersions) { ); ipcMainManager.handle( IpcEvents.START_FIDDLE, - async (event: IpcMainInvokeEvent, params: StartFiddleParams) => { - await startFiddle(event.sender, params); + async (event: IpcMainInvokeEvent) => { + await startFiddle(event.sender); }, ); ipcMainManager.on(IpcEvents.STOP_FIDDLE, (event: IpcMainEvent) => { diff --git a/src/renderer/runner.ts b/src/renderer/runner.ts index 346706efa1..5dc605e739 100644 --- a/src/renderer/runner.ts +++ b/src/renderer/runner.ts @@ -3,7 +3,6 @@ import semver from 'semver'; import { Bisector } from './bisect'; import { AppState } from './state'; -import { maybePlural } from './utils/plural-maybe'; import { FileTransformOperation, InstallState, @@ -21,13 +20,6 @@ export enum ForgeCommands { MAKE = 'make', } -interface RunFiddleParams { - localPath: string | undefined; - isValidBuild: boolean; // If the localPath is a valid Electron build - version: string; // The user selected version - dir: string; -} - const resultString: Record = Object.freeze({ [RunResult.FAILURE]: '❌ failed', [RunResult.INVALID]: '❓ invalid', @@ -144,11 +136,9 @@ export class Runner { * Actually run the fiddle. */ public async run(): Promise { - const options = { includeDependencies: false, includeElectron: false }; - const { appState } = this; const currentRunnable = appState.currentElectronVersion; - const { version, state, localPath } = currentRunnable; + const { version, state } = currentRunnable; const isValidBuild = // Destructure currentRunnable so it's not a Proxy object, which can't be used window.ElectronFiddle.getLocalVersionState({ ...currentRunnable }) === @@ -185,24 +175,6 @@ export class Runner { } appState.isConsoleShowing = true; - const dir = await this.saveToTemp(options); - const packageManager = appState.packageManager; - const useSocketFirewall = appState.isUsingSocketFirewall; - - if (!dir) return RunResult.INVALID; - - try { - await this.installModules({ dir, packageManager, useSocketFirewall }); - } catch (error: any) { - console.error('Runner: Could not install modules', error); - - appState.pushError('Could not install modules', error.message); - appState.isInstallingModules = false; - - await window.ElectronFiddle.cleanupDirectory(dir); - return RunResult.INVALID; - } - const isReady = state === InstallState.installed || state === InstallState.downloaded || @@ -217,16 +189,10 @@ export class Runner { message += `before running the fiddle.`; appState.pushOutput(message, { isNotPre: true }); - await window.ElectronFiddle.cleanupDirectory(dir); return RunResult.INVALID; } - return this.runFiddle({ - localPath, - isValidBuild, - dir, - version, - }); + return this.runFiddle(); } /** @@ -295,53 +261,6 @@ export class Runner { return true; } - /** - * Installs the specified modules - */ - public async installModules(pmOptions: PMOperationOptions): Promise { - const modules = Array.from(this.appState.modules.entries()).map( - ([pkg, version]) => `${pkg}@${version}`, - ); - const { pushOutput } = this.appState; - - if (modules && modules.length > 0) { - this.appState.isInstallingModules = true; - const packageManager = pmOptions.packageManager; - const pmInstalled = - await window.ElectronFiddle.getIsPackageManagerInstalled( - packageManager, - ); - if (!pmInstalled) { - let message = `The ${maybePlural(`module`, modules)} ${modules.join( - ', ', - )} need to be installed, `; - message += `but we could not find ${packageManager}. Fiddle requires Node.js and npm `; - message += `to support the installation of modules not included in `; - message += `Electron. Please visit https://nodejs.org to install Node.js `; - message += `and npm, or https://classic.yarnpkg.com/lang/en/ to install Yarn`; - - pushOutput(message, { isNotPre: true }); - this.appState.isInstallingModules = false; - return; - } - - pushOutput( - `Installing node modules using ${ - pmOptions.packageManager - }: ${modules.join(', ')}...`, - { isNotPre: true }, - ); - - const result = await window.ElectronFiddle.addModules( - pmOptions, - ...modules, - ); - pushOutput(result); - - this.appState.isInstallingModules = false; - } - } - public buildChildEnvVars(): { [x: string]: string | undefined } { const { environmentVariables } = this.appState; @@ -369,59 +288,34 @@ export class Runner { /** * Executes the fiddle with either local electron build - * or the user selected electron version + * or the user selected electron version. The main process owns the + * temp directory, module installation, and cleanup — the renderer + * simply triggers the run and waits for the `fiddle-stopped` event. */ - private async runFiddle(params: RunFiddleParams): Promise { - const { version, dir } = params; - const { pushOutput, flushOutput, executionFlags } = this.appState; - const env = this.buildChildEnvVars(); + private async runFiddle(): Promise { + const { pushOutput, flushOutput } = this.appState; - // Add user-specified cli flags if any have been set. - const options = [dir, '--inspect'].concat(executionFlags); - - const cleanup = async () => { + const cleanup = () => { flushOutput(); - this.appState.isRunning = false; - - // Clean older folders - await window.ElectronFiddle.cleanupDirectory(dir); - await this.deleteUserData(); + this.appState.isInstallingModules = false; }; - return new Promise(async (resolve, _reject) => { - try { - await window.ElectronFiddle.startFiddle({ - ...params, - enableElectronLogging: this.appState.isEnablingElectronLogging, - options, - env, - }); - } catch (e: any) { - pushOutput(`Failed to spawn Fiddle: ${e.message}`); - await cleanup(); - return resolve(RunResult.FAILURE); - } - - this.appState.isRunning = true; - - const name = await this.appState.getName(); - pushOutput(`Electron v${version} started as "${name}"`); - + return new Promise(async (resolve) => { window.ElectronFiddle.removeAllListeners('fiddle-runner-output'); window.ElectronFiddle.removeAllListeners('fiddle-stopped'); window.ElectronFiddle.addEventListener( 'fiddle-runner-output', - (output: string) => { - pushOutput(output, { bypassBuffer: false }); + (output: string, options?: { isNotPre?: boolean }) => { + pushOutput(output, { ...options, bypassBuffer: false }); }, ); window.ElectronFiddle.addEventListener( 'fiddle-stopped', - async (code, signal) => { - await cleanup(); + (code, signal) => { + cleanup(); if (typeof code !== 'number') { pushOutput(`Electron exited with signal ${signal}.`); @@ -432,6 +326,16 @@ export class Runner { } }, ); + + this.appState.isRunning = true; + + try { + await window.ElectronFiddle.startFiddle(); + } catch (e: any) { + pushOutput(`Failed to spawn Fiddle: ${e.message}`); + cleanup(); + resolve(RunResult.FAILURE); + } }); } @@ -497,21 +401,4 @@ export class Runner { return false; } - - /** - * Deletes the user data dir after a run. - */ - private async deleteUserData() { - if (this.appState.isKeepingUserDataDirs) { - console.log( - `Cleanup: Not deleting data dir due to isKeepingUserDataDirs setting`, - ); - return; - } - - const name = await this.appState.getName(); - - console.log(`Cleanup: Deleting data dir for ${name}`); - await window.ElectronFiddle.deleteUserData(name); - } } diff --git a/tests/main/fiddle-core.spec.ts b/tests/main/fiddle-core.spec.ts index e317f657dd..d4f976409d 100644 --- a/tests/main/fiddle-core.spec.ts +++ b/tests/main/fiddle-core.spec.ts @@ -4,13 +4,20 @@ import * as fs from 'node:fs'; import * as os from 'node:os'; -import * as path from 'node:path'; import { ElectronVersions, Installer, Runner } from '@electron/fiddle-core'; import { WebContents } from 'electron'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; -import { setupFiddleCore, startFiddle } from '../../src/main/fiddle-core'; +import type { StartFiddleOptions } from '../../src/interfaces'; +import { + installModules, + setupFiddleCore, + startFiddle, +} from '../../src/main/fiddle-core'; +import * as filesMod from '../../src/main/files'; +import { addModules, getIsPackageManagerInstalled } from '../../src/main/npm'; +import { getFiles } from '../../src/main/utils/get-files'; import { ChildProcessMock } from '../mocks/child-process'; import { ElectronVersionsMock, FiddleRunnerMock } from '../mocks/fiddle-core'; import { WebContentsMock } from '../mocks/web-contents'; @@ -28,6 +35,59 @@ vi.mock('@electron/fiddle-core', async () => { vi.mock('node:fs'); +// The refactored startFiddle delegates the heavy lifting to small helper +// modules. Mock them so these tests can focus on the spawn behaviour. +const { TEMP_DIR, getLocalVersionsMock, getStartFiddleOptionsMock } = + vi.hoisted(() => { + return { + TEMP_DIR: require('node:path').join( + require('node:os').tmpdir(), + 'test-fiddle', + ), + getLocalVersionsMock: vi.fn( + () => [] as Array<{ version: string; localPath: string }>, + ), + getStartFiddleOptionsMock: vi.fn(), + }; + }); +vi.mock('../../src/main/files', () => ({ + saveFilesToTemp: vi.fn().mockResolvedValue(TEMP_DIR), + cleanupDirectory: vi.fn().mockResolvedValue(true), + deleteUserData: vi.fn().mockResolvedValue(undefined), +})); +vi.mock('../../src/main/npm', () => ({ + addModules: vi.fn().mockResolvedValue(''), + getIsPackageManagerInstalled: vi.fn().mockResolvedValue(true), +})); +vi.mock('../../src/main/utils/get-files', () => ({ + getFiles: vi.fn().mockResolvedValue({ + files: new Map([['package.json', JSON.stringify({ name: 'test-app' })]]), + }), +})); +vi.mock('../../src/main/versions', () => ({ + getLocalVersions: () => getLocalVersionsMock(), +})); +vi.mock('../../src/main/utils/get-start-fiddle-options', () => ({ + getStartFiddleOptions: (...args: unknown[]) => + getStartFiddleOptionsMock(...args), +})); + +function makeOptions( + overrides: Partial = {}, +): StartFiddleOptions { + return { + enableElectronLogging: false, + env: {}, + executionFlags: [], + isKeepingUserDataDirs: false, + modules: [], + packageManager: 'npm', + useSocketFirewall: false, + version: '18.0.0', + ...overrides, + }; +} + describe('fiddle-core', () => { let runner: FiddleRunnerMock; let originalEnv: NodeJS.ProcessEnv; @@ -40,7 +100,6 @@ describe('fiddle-core', () => { TEMP: REAL_TMPDIR, TMP: REAL_TMPDIR, }); - const dir = path.join(REAL_TMPDIR, 'test-fiddle'); beforeEach(() => { runner = new FiddleRunnerMock(); @@ -49,9 +108,15 @@ describe('fiddle-core', () => { vi.mocked(Runner.create).mockResolvedValue(runner as unknown as Runner); setupFiddleCore(new ElectronVersionsMock() as unknown as ElectronVersions); vi.mocked(fs.existsSync).mockReturnValue(false); - vi.mocked(fs.realpathSync as (p: fs.PathLike) => string).mockImplementation( - (p) => p.toString(), - ); + vi.mocked(filesMod.saveFilesToTemp).mockResolvedValue(TEMP_DIR); + vi.mocked(filesMod.cleanupDirectory).mockResolvedValue(true); + vi.mocked(filesMod.deleteUserData).mockResolvedValue(); + vi.mocked(addModules).mockResolvedValue(''); + vi.mocked(getIsPackageManagerInstalled).mockResolvedValue(true); + vi.mocked(getFiles).mockResolvedValue({ + files: new Map([['package.json', JSON.stringify({ name: 'test-app' })]]), + }); + getLocalVersionsMock.mockReturnValue([]); }); afterEach(() => { @@ -59,42 +124,53 @@ describe('fiddle-core', () => { vi.clearAllMocks(); }); - describe('startFiddle', () => { - const version = '18.0.0'; - - it('rejects dir outside the temp directory', async () => { + describe('installModules()', () => { + it('does not attempt installation if npm is not installed', async () => { + vi.mocked(getIsPackageManagerInstalled).mockResolvedValue(false); await expect( - startFiddle(new WebContentsMock() as unknown as WebContents, { - dir: '/not/a/tmp/path', - enableElectronLogging: false, - env: {}, - isValidBuild: false, - localPath: undefined, - options: [], - version, - }), - ).rejects.toThrow('startFiddle: dir must be inside the temp directory'); + installModules( + new WebContentsMock() as unknown as WebContents, + [['lodash', '4.0.0']], + { + dir: '/fake/path', + packageManager: 'npm', + }, + ), + ).rejects.toThrow('Package manager not installed'); + + expect(addModules).toHaveBeenCalledTimes(0); + }); + + it('does attempt installation if npm is installed', async () => { + vi.mocked(getIsPackageManagerInstalled).mockResolvedValue(true); + await installModules( + new WebContentsMock() as unknown as WebContents, + [['lodash', '4.0.0']], + { + dir: '/fake/path', + packageManager: 'npm', + }, + ); + + expect(addModules).toHaveBeenCalledTimes(1); }); + }); + + describe('startFiddle', () => { + const version = '18.0.0'; it('uses provided env', async () => { const child = new ChildProcessMock(); vi.mocked(runner.spawn).mockResolvedValue(child); + getStartFiddleOptionsMock.mockResolvedValueOnce( + makeOptions({ env: { NODE_OPTIONS: '--inspect-brk' } }), + ); - await startFiddle(new WebContentsMock() as unknown as WebContents, { - dir, - enableElectronLogging: false, - env: { - NODE_OPTIONS: '--inspect-brk', - }, - isValidBuild: true, - localPath: undefined, - options: [], - version, - }); + await startFiddle(new WebContentsMock() as unknown as WebContents); expect(runner.spawn).toHaveBeenCalledWith( version, - dir, + TEMP_DIR, expect.objectContaining({ env: { ...mockEnv, @@ -107,20 +183,15 @@ describe('fiddle-core', () => { it('runs with logging when enabled', async () => { const child = new ChildProcessMock(); vi.mocked(runner.spawn).mockResolvedValue(child); + getStartFiddleOptionsMock.mockResolvedValueOnce( + makeOptions({ enableElectronLogging: true }), + ); - await startFiddle(new WebContentsMock() as unknown as WebContents, { - dir, - enableElectronLogging: true, - env: {}, - isValidBuild: true, - localPath: undefined, - options: [], - version, - }); + await startFiddle(new WebContentsMock() as unknown as WebContents); expect(runner.spawn).toHaveBeenCalledWith( version, - dir, + TEMP_DIR, expect.objectContaining({ env: { ...mockEnv, @@ -135,22 +206,15 @@ describe('fiddle-core', () => { it('can set ELECTRON_ENABLE_LOGGING in env', async () => { const child = new ChildProcessMock(); vi.mocked(runner.spawn).mockResolvedValue(child); + getStartFiddleOptionsMock.mockResolvedValueOnce( + makeOptions({ env: { ELECTRON_ENABLE_LOGGING: 'true' } }), + ); - await startFiddle(new WebContentsMock() as unknown as WebContents, { - dir, - enableElectronLogging: false, - env: { - ELECTRON_ENABLE_LOGGING: 'true', - }, - isValidBuild: true, - localPath: undefined, - options: [], - version, - }); + await startFiddle(new WebContentsMock() as unknown as WebContents); expect(runner.spawn).toHaveBeenCalledWith( version, - dir, + TEMP_DIR, expect.objectContaining({ env: { ...mockEnv, @@ -160,25 +224,22 @@ describe('fiddle-core', () => { ); }); - it('strips blocked env keys from params.env', async () => { + it('strips blocked env keys from the renderer-supplied env', async () => { const child = new ChildProcessMock(); vi.mocked(runner.spawn).mockResolvedValue(child); + getStartFiddleOptionsMock.mockResolvedValueOnce( + makeOptions({ + env: { + LD_PRELOAD: '/evil/lib.so', + DYLD_INSERT_LIBRARIES: '/evil/lib.dylib', + DYLD_FRAMEWORK_PATH: '/evil/frameworks', + DYLD_LIBRARY_PATH: '/evil/lib', + SAFE_VAR: 'allowed', + }, + }), + ); - await startFiddle(new WebContentsMock() as unknown as WebContents, { - dir, - enableElectronLogging: false, - env: { - LD_PRELOAD: '/evil/lib.so', - DYLD_INSERT_LIBRARIES: '/evil/lib.dylib', - DYLD_FRAMEWORK_PATH: '/evil/frameworks', - DYLD_LIBRARY_PATH: '/evil/lib', - SAFE_VAR: 'allowed', - }, - isValidBuild: false, - localPath: undefined, - options: [], - version, - }); + await startFiddle(new WebContentsMock() as unknown as WebContents); const spawnEnv = ( vi.mocked(runner.spawn).mock.calls[0]! as unknown as [ @@ -194,74 +255,113 @@ describe('fiddle-core', () => { expect(spawnEnv['SAFE_VAR']).toBe('allowed'); }); - it('uses localPath when the resolved executable exists on disk', async () => { + it('uses localPath when the version resolves to a local build with an existing executable', async () => { const child = new ChildProcessMock(); vi.mocked(runner.spawn).mockResolvedValue(child); const localPath = '/some/local/electron'; + const localVersion = '0.0.0-local.123'; + getLocalVersionsMock.mockReturnValueOnce([ + { version: localVersion, localPath }, + ]); // InstallerMock.getExecPath returns `${localPath}/electron` vi.mocked(fs.existsSync).mockReturnValue(true); + getStartFiddleOptionsMock.mockResolvedValueOnce( + makeOptions({ version: localVersion }), + ); - await startFiddle(new WebContentsMock() as unknown as WebContents, { - dir, - enableElectronLogging: false, - env: {}, - isValidBuild: true, - localPath, - options: [], - version, - }); + await startFiddle(new WebContentsMock() as unknown as WebContents); expect(Installer.getExecPath).toHaveBeenCalledWith(localPath); expect(runner.spawn).toHaveBeenCalledWith( `${localPath}/electron`, - dir, + TEMP_DIR, expect.anything(), ); }); - it('falls back to version when localPath executable does not exist', async () => { + it('falls back to version when the local build executable does not exist', async () => { const child = new ChildProcessMock(); vi.mocked(runner.spawn).mockResolvedValue(child); + const localPath = '/nonexistent/electron'; + const localVersion = '0.0.0-local.456'; + getLocalVersionsMock.mockReturnValueOnce([ + { version: localVersion, localPath }, + ]); vi.mocked(fs.existsSync).mockReturnValue(false); + getStartFiddleOptionsMock.mockResolvedValueOnce( + makeOptions({ version: localVersion }), + ); - await startFiddle(new WebContentsMock() as unknown as WebContents, { - dir, - enableElectronLogging: false, - env: {}, - isValidBuild: true, - localPath: '/nonexistent/electron', - options: [], - version, - }); + await startFiddle(new WebContentsMock() as unknown as WebContents); expect(runner.spawn).toHaveBeenCalledWith( - version, - dir, + localVersion, + TEMP_DIR, expect.anything(), ); }); - it('strips null bytes from options', async () => { + it('strips null bytes from execution flags', async () => { const child = new ChildProcessMock(); vi.mocked(runner.spawn).mockResolvedValue(child); + getStartFiddleOptionsMock.mockResolvedValueOnce( + makeOptions({ + executionFlags: ['safe-flag', 'evil\0flag', '--js-flags=ok'], + }), + ); - await startFiddle(new WebContentsMock() as unknown as WebContents, { - dir, - enableElectronLogging: false, - env: {}, - isValidBuild: false, - localPath: undefined, - options: ['--inspect', 'safe-flag', 'evil\0flag', '--js-flags=ok'], - version, - }); + await startFiddle(new WebContentsMock() as unknown as WebContents); expect(runner.spawn).toHaveBeenCalledWith( version, - dir, + TEMP_DIR, expect.objectContaining({ - args: ['--inspect', 'safe-flag', '--js-flags=ok'], + args: [TEMP_DIR, '--inspect', 'safe-flag', '--js-flags=ok'], }), ); }); + + it('cleans up the temp dir and user data after the child process closes', async () => { + const child = new ChildProcessMock(); + vi.mocked(runner.spawn).mockResolvedValue(child); + getStartFiddleOptionsMock.mockResolvedValueOnce(makeOptions()); + + await startFiddle(new WebContentsMock() as unknown as WebContents); + + child.emit('close', 0, null); + await new Promise(process.nextTick); + + expect(filesMod.cleanupDirectory).toHaveBeenCalledWith(TEMP_DIR); + expect(filesMod.deleteUserData).toHaveBeenCalledWith('test-app'); + }); + + it('does not delete user data when isKeepingUserDataDirs is true', async () => { + const child = new ChildProcessMock(); + vi.mocked(runner.spawn).mockResolvedValue(child); + getStartFiddleOptionsMock.mockResolvedValueOnce( + makeOptions({ isKeepingUserDataDirs: true }), + ); + + await startFiddle(new WebContentsMock() as unknown as WebContents); + + child.emit('close', 0, null); + await new Promise(process.nextTick); + + expect(filesMod.cleanupDirectory).toHaveBeenCalledWith(TEMP_DIR); + expect(filesMod.deleteUserData).not.toHaveBeenCalled(); + }); + + it('cleans up when saveFilesToTemp fails', async () => { + getStartFiddleOptionsMock.mockResolvedValueOnce(makeOptions()); + vi.mocked(filesMod.saveFilesToTemp).mockRejectedValueOnce( + new Error('disk full'), + ); + + await expect( + startFiddle(new WebContentsMock() as unknown as WebContents), + ).rejects.toThrow('disk full'); + + expect(runner.spawn).not.toHaveBeenCalled(); + }); }); }); diff --git a/tests/renderer/runner.spec.tsx b/tests/renderer/runner.spec.tsx index 32eb87c0b5..1e47a8f3d5 100644 --- a/tests/renderer/runner.spec.tsx +++ b/tests/renderer/runner.spec.tsx @@ -9,12 +9,7 @@ import { } from '../../src/interfaces'; import { ForgeCommands, Runner } from '../../src/renderer/runner'; import { AppState } from '../../src/renderer/state'; -import { - AppMock, - FileManagerMock, - StateMock, - VersionsMock, -} from '../mocks/mocks'; +import { AppMock, StateMock, VersionsMock } from '../mocks/mocks'; import { emitEvent } from '../utils'; vi.mock('../../src/renderer/file-manager'); @@ -22,13 +17,12 @@ vi.mock('../../src/renderer/file-manager'); describe('Runner component', () => { let store: StateMock; let instance: Runner; - let fileManager: FileManagerMock; let mockVersions: Record; let mockVersionsArray: RunnableVersion[]; beforeEach(() => { ({ mockVersions, mockVersionsArray } = new VersionsMock()); - ({ fileManager, state: store } = window.app as unknown as AppMock); + ({ state: store } = window.app as unknown as AppMock); store.initVersions('2.0.2', { ...mockVersions }); store.getName.mockResolvedValue('test-app-name'); store.modules = new Map([['cow', '*']]); @@ -96,8 +90,15 @@ describe('Runner component', () => { expect(result).toBe(RunResult.SUCCESS); expect(store.isRunning).toBe(false); - expect(fileManager.saveToTemp).toHaveBeenCalled(); - expect(window.ElectronFiddle.addModules).toHaveBeenCalled(); + expect(window.ElectronFiddle.startFiddle).toHaveBeenCalled(); + }); + + it('exposes run-time options to the main process', async () => { + store.isEnablingElectronLogging = true; + const options = await instance.getStartFiddleOptions(); + expect(options).toEqual( + expect.objectContaining({ enableElectronLogging: true }), + ); }); it('runs with logging when enabled', async () => { @@ -114,13 +115,7 @@ describe('Runner component', () => { expect(result).toBe(RunResult.SUCCESS); expect(store.isRunning).toBe(false); - expect(fileManager.saveToTemp).toHaveBeenCalled(); - expect(window.ElectronFiddle.addModules).toHaveBeenCalled(); - expect(window.ElectronFiddle.startFiddle).toBeCalledWith( - expect.objectContaining({ - enableElectronLogging: true, - }), - ); + expect(window.ElectronFiddle.startFiddle).toHaveBeenCalled(); }); it('emits output with exitCode', async () => { @@ -139,7 +134,7 @@ describe('Runner component', () => { expect(result).toBe(RunResult.SUCCESS); expect(store.isRunning).toBe(false); - expect(store.pushOutput).toHaveBeenCalledTimes(8); + expect(store.pushOutput).toHaveBeenCalledTimes(3); expect(store.flushOutput).toHaveBeenCalledTimes(1); expect(store.pushOutput).toHaveBeenLastCalledWith( 'Electron exited with code 0.', @@ -204,34 +199,15 @@ describe('Runner component', () => { expect(result).toBe(RunResult.FAILURE); expect(store.isRunning).toBe(false); expect(store.flushOutput).toHaveBeenCalledTimes(1); - expect(store.pushOutput).toHaveBeenCalledTimes(8); + expect(store.pushOutput).toHaveBeenCalledTimes(3); expect(store.pushOutput).toHaveBeenLastCalledWith( `Electron exited with signal ${signal}.`, ); }); - it('cleans the app data dir after a run', async () => { - setTimeout(() => emitEvent('fiddle-stopped', 0)); - const result = await instance.run(); - - expect(result).toBe(RunResult.SUCCESS); - await new Promise(process.nextTick); - expect(window.ElectronFiddle.cleanupDirectory).toHaveBeenCalledTimes(1); - expect(window.ElectronFiddle.deleteUserData).toHaveBeenCalledTimes(1); - expect(window.ElectronFiddle.deleteUserData).toHaveBeenCalledWith( - 'test-app-name', - ); - }); - - it('does not clean the app data dir after a run if configured', async () => { - (instance as any).appState.isKeepingUserDataDirs = true; - - setTimeout(() => emitEvent('fiddle-stopped', 0)); - const result = await instance.run(); - - expect(result).toBe(RunResult.SUCCESS); - await new Promise(process.nextTick); - expect(window.ElectronFiddle.cleanupDirectory).toHaveBeenCalledTimes(1); + it('does not run version not yet downloaded', async () => { + store.currentElectronVersion.state = InstallState.missing; + expect(await instance.run()).toBe(RunResult.INVALID); }); it('automatically cleans the console when enabled', async () => { @@ -243,30 +219,6 @@ describe('Runner component', () => { expect(result).toBe(RunResult.SUCCESS); expect(store.clearConsole).toHaveBeenCalled(); }); - - it('does not run version not yet downloaded', async () => { - store.currentElectronVersion.state = InstallState.missing; - expect(await instance.run()).toBe(RunResult.INVALID); - }); - - it('does not run if writing files fails', async () => { - vi.mocked(fileManager.saveToTemp).mockRejectedValueOnce('bwap bwap'); - - expect(await instance.run()).toBe(RunResult.INVALID); - }); - - it('does not run if installing modules fails', async () => { - const oldError = console.error; - console.error = vi.fn(); - - instance.installModules = vi.fn().mockImplementationOnce(async () => { - throw new Error('Bwap-bwap'); - }); - - expect(await instance.run()).toBe(RunResult.INVALID); - - console.error = oldError; - }); }); describe('stop()', () => { @@ -465,21 +417,4 @@ describe('Runner component', () => { ); }); }); - - describe('installModules()', () => { - it.each([ - ['does not attempt installation if npm is not installed', false, 0], - ['does attempt installation if npm is installed', true, 1], - ])('%s', async (_: unknown, haveNpm: boolean, numCalls: number) => { - vi.mocked( - window.ElectronFiddle.getIsPackageManagerInstalled, - ).mockResolvedValue(haveNpm); - await instance.installModules({ - dir: '/fake/path', - packageManager: 'npm', - }); - - expect(window.ElectronFiddle.addModules).toHaveBeenCalledTimes(numCalls); - }); - }); }); From 1a7446cf481fdc7f4b7e9c158a1888ff6a57e4bd Mon Sep 17 00:00:00 2001 From: David Sanders Date: Thu, 7 May 2026 13:08:10 -0400 Subject: [PATCH 04/22] fix: change run button state to installing modules correctly --- src/interfaces.ts | 1 + src/ipc-events.ts | 1 + src/main/fiddle-core.ts | 5 +++++ src/preload/preload.ts | 1 + src/renderer/components/commands-runner.tsx | 10 +++++----- src/renderer/runner.ts | 5 +++++ 6 files changed, 18 insertions(+), 5 deletions(-) diff --git a/src/interfaces.ts b/src/interfaces.ts index 01115256b4..b7c6b29c09 100644 --- a/src/interfaces.ts +++ b/src/interfaces.ts @@ -180,6 +180,7 @@ export type FiddleEvent = | 'electron-types-changed' | 'execute-monaco-command' | 'fiddle-runner-output' + | 'fiddle-modules-installed' | 'fiddle-stopped' | 'load-example' | 'load-gist' diff --git a/src/ipc-events.ts b/src/ipc-events.ts index 0a6d26a2fc..c76e28ddc9 100644 --- a/src/ipc-events.ts +++ b/src/ipc-events.ts @@ -76,6 +76,7 @@ export enum IpcEvents { DOWNLOAD_VERSION = 'DOWNLOAD_VERSION', REMOVE_VERSION = 'REMOVE_VERSION', FIDDLE_RUNNER_OUTPUT = 'FIDDLE_RUNNER_OUTPUT', + FIDDLE_MODULES_INSTALLED = 'FIDDLE_MODULES_INSTALLED', FIDDLE_STOPPED = 'FIDDLE_STOPPED', VERSION_DOWNLOAD_PROGRESS = 'VERSION_DOWNLOAD_PROGRESS', VERSION_STATE_CHANGED = 'VERSION_STATE_CHANGED', diff --git a/src/main/fiddle-core.ts b/src/main/fiddle-core.ts index f49a506890..71e5066e2a 100644 --- a/src/main/fiddle-core.ts +++ b/src/main/fiddle-core.ts @@ -244,6 +244,11 @@ export async function startFiddle(webContents: WebContents): Promise { } fiddleProcesses.set(webContents, child); + // Signal the renderer that module installation is done and the process is + // running. Sent after fiddleProcesses.set() so that stopFiddle() will work + // as soon as the button transitions to "Stop". + ipcMainManager.send(IpcEvents.FIDDLE_MODULES_INSTALLED, [], webContents); + pushOutputLine(webContents, `Electron v${version} started as "${appName}"`); child.stdout?.on('data', (data) => pushOutput(webContents, data.toString())); diff --git a/src/preload/preload.ts b/src/preload/preload.ts index 7f083b079b..76e8d8c19d 100644 --- a/src/preload/preload.ts +++ b/src/preload/preload.ts @@ -27,6 +27,7 @@ const channelMapping: Record = { 'electron-types-changed': IpcEvents.ELECTRON_TYPES_CHANGED, 'execute-monaco-command': IpcEvents.MONACO_EXECUTE_COMMAND, 'fiddle-runner-output': IpcEvents.FIDDLE_RUNNER_OUTPUT, + 'fiddle-modules-installed': IpcEvents.FIDDLE_MODULES_INSTALLED, 'fiddle-stopped': IpcEvents.FIDDLE_STOPPED, 'load-example': IpcEvents.LOAD_ELECTRON_EXAMPLE_REQUEST, 'load-gist': IpcEvents.LOAD_GIST_REQUEST, diff --git a/src/renderer/components/commands-runner.tsx b/src/renderer/components/commands-runner.tsx index c023241495..c7bc4bdef7 100644 --- a/src/renderer/components/commands-runner.tsx +++ b/src/renderer/components/commands-runner.tsx @@ -49,15 +49,15 @@ export const Runner = observer( case downloaded: case installed: { props.disabled = false; - if (isRunning) { + if (isInstallingModules) { + props.disabled = true; + props.text = 'Installing modules'; + props.icon = ; + } else if (isRunning) { props.active = true; props.text = 'Stop'; props.onClick = window.app.runner.stop; props.icon = 'stop'; - } else if (isInstallingModules) { - props.disabled = true; - props.text = 'Installing modules'; - props.icon = ; } else { props.text = 'Run'; props.onClick = window.app.runner.run; diff --git a/src/renderer/runner.ts b/src/renderer/runner.ts index 5dc605e739..686669ae81 100644 --- a/src/renderer/runner.ts +++ b/src/renderer/runner.ts @@ -303,6 +303,7 @@ export class Runner { return new Promise(async (resolve) => { window.ElectronFiddle.removeAllListeners('fiddle-runner-output'); + window.ElectronFiddle.removeAllListeners('fiddle-modules-installed'); window.ElectronFiddle.removeAllListeners('fiddle-stopped'); window.ElectronFiddle.addEventListener( @@ -312,6 +313,10 @@ export class Runner { }, ); + window.ElectronFiddle.addEventListener('fiddle-modules-installed', () => { + this.appState.isInstallingModules = false; + }); + window.ElectronFiddle.addEventListener( 'fiddle-stopped', (code, signal) => { From c62780388a8208cbd355e65e05055688f8e91540 Mon Sep 17 00:00:00 2001 From: David Sanders Date: Thu, 7 May 2026 14:29:45 -0400 Subject: [PATCH 05/22] refactor: move plural-maybe.ts to common utils --- src/main/fiddle-core.ts | 2 +- src/{renderer => }/utils/plural-maybe.ts | 0 tests/{renderer => }/utils/plural-maybe.spec.ts | 2 +- 3 files changed, 2 insertions(+), 2 deletions(-) rename src/{renderer => }/utils/plural-maybe.ts (100%) rename tests/{renderer => }/utils/plural-maybe.spec.ts (84%) diff --git a/src/main/fiddle-core.ts b/src/main/fiddle-core.ts index 71e5066e2a..49d702f0d0 100644 --- a/src/main/fiddle-core.ts +++ b/src/main/fiddle-core.ts @@ -23,7 +23,7 @@ import { ProgressObject, } from '../interfaces'; import { IpcEvents } from '../ipc-events'; -import { maybePlural } from '../renderer/utils/plural-maybe'; +import { maybePlural } from '../utils/plural-maybe'; export interface PMOperationOptions { dir: string; diff --git a/src/renderer/utils/plural-maybe.ts b/src/utils/plural-maybe.ts similarity index 100% rename from src/renderer/utils/plural-maybe.ts rename to src/utils/plural-maybe.ts diff --git a/tests/renderer/utils/plural-maybe.spec.ts b/tests/utils/plural-maybe.spec.ts similarity index 84% rename from tests/renderer/utils/plural-maybe.spec.ts rename to tests/utils/plural-maybe.spec.ts index 961d509e0e..e900490bcd 100644 --- a/tests/renderer/utils/plural-maybe.spec.ts +++ b/tests/utils/plural-maybe.spec.ts @@ -1,6 +1,6 @@ import { describe, expect, it } from 'vitest'; -import { maybePlural } from '../../../src/renderer/utils/plural-maybe'; +import { maybePlural } from '../../src/utils/plural-maybe'; describe('maybePlural', () => { describe('maybePlural()', () => { From 5e4fdb9ca77c01c2959865c3e3fdecb0911bad51 Mon Sep 17 00:00:00 2001 From: David Sanders Date: Thu, 7 May 2026 14:44:10 -0400 Subject: [PATCH 06/22] chore: drop dead code --- src/ambient.d.ts | 2 -- src/ipc-events.ts | 2 -- src/main/files.ts | 34 ---------------------------------- src/preload/preload.ts | 6 ------ tests/main/files.spec.ts | 8 -------- tests/mocks/electron-fiddle.ts | 2 -- tests/renderer/runner.spec.tsx | 2 -- 7 files changed, 56 deletions(-) diff --git a/src/ambient.d.ts b/src/ambient.d.ts index e3290510ad..de7b2972c7 100644 --- a/src/ambient.d.ts +++ b/src/ambient.d.ts @@ -105,13 +105,11 @@ declare global { ): Promise; arch: string; blockAccelerators(acceleratorsToBlock: BlockableAccelerator[]): void; - cleanupDirectory(dir: string): Promise; confirmQuit(): void; createThemeFile( newTheme: FiddleTheme, name?: string, ): Promise; - deleteUserData(name: string): Promise; downloadVersion( version: string, opts?: Partial, diff --git a/src/ipc-events.ts b/src/ipc-events.ts index c76e28ddc9..03b7a2d6cb 100644 --- a/src/ipc-events.ts +++ b/src/ipc-events.ts @@ -64,8 +64,6 @@ export enum IpcEvents { GET_ELECTRON_TYPES = 'GET_ELECTRON_TYPES', UNWATCH_ELECTRON_TYPES = 'UNWATCH_ELECTRON_TYPES', GET_NODE_TYPES = 'GET_NODE_TYPES', - CLEANUP_DIRECTORY = 'CLEANUP_DIRECTORY', - DELETE_USER_DATA = 'DELETE_USER_DATA', SAVE_FILES_TO_TEMP = 'SAVE_FILES_TO_TEMP', SAVED_LOCAL_FIDDLE = 'SAVED_LOCAL_FIDDLE', GET_FILES = 'GET_FILES', diff --git a/src/main/files.ts b/src/main/files.ts index 32500798c9..b3c21bf56f 100644 --- a/src/main/files.ts +++ b/src/main/files.ts @@ -1,4 +1,3 @@ -import * as os from 'node:os'; import * as path from 'node:path'; import { BrowserWindow, IpcMainInvokeEvent, app, dialog } from 'electron'; @@ -27,43 +26,10 @@ function isSafeDataName(str: unknown): str is string { ); } -/** - * Returns true if `dir` resolves to a path inside the OS temp directory. - * Used to ensure CLEANUP_DIRECTORY cannot reach outside tmp. - */ -function isInsideTempDir(dir: unknown): dir is string { - if (typeof dir !== 'string') return false; - const tmpDir = fs.realpathSync(os.tmpdir()); - const resolved = path.resolve(dir); - return resolved.startsWith(tmpDir + path.sep) || resolved === tmpDir; -} - /** * Ensures that we're listening to file events */ export function setupFileListeners() { - ipcMainManager.handle( - IpcEvents.CLEANUP_DIRECTORY, - (_: IpcMainInvokeEvent, dir: string) => { - if (!isInsideTempDir(dir)) { - console.warn( - `cleanupDirectory: rejected path outside temp dir: ${dir}`, - ); - return false; - } - return cleanupDirectory(dir); - }, - ); - ipcMainManager.handle( - IpcEvents.DELETE_USER_DATA, - (_: IpcMainInvokeEvent, name: string) => { - if (!isSafeDataName(name)) { - console.warn(`deleteUserData: rejected unsafe name: ${name}`); - return; - } - return deleteUserData(name); - }, - ); ipcMainManager.handle( IpcEvents.SAVE_FILES_TO_TEMP, (_: IpcMainInvokeEvent, files: [string, string][]) => diff --git a/src/preload/preload.ts b/src/preload/preload.ts index 76e8d8c19d..47f4f42e9d 100644 --- a/src/preload/preload.ts +++ b/src/preload/preload.ts @@ -91,18 +91,12 @@ export async function setupFiddleGlobal() { blockAccelerators(acceleratorsToBlock: BlockableAccelerator[]) { ipcRenderer.send(IpcEvents.BLOCK_ACCELERATORS, acceleratorsToBlock); }, - cleanupDirectory(dir: string) { - return ipcRenderer.invoke(IpcEvents.CLEANUP_DIRECTORY, dir); - }, confirmQuit() { ipcRenderer.send(IpcEvents.CONFIRM_QUIT); }, createThemeFile(newTheme: FiddleTheme, name?: string) { return ipcRenderer.invoke(IpcEvents.CREATE_THEME_FILE, newTheme, name); }, - async deleteUserData(name: string) { - await ipcRenderer.invoke(IpcEvents.DELETE_USER_DATA, name); - }, async downloadVersion( version: string, opts?: Partial, diff --git a/tests/main/files.spec.ts b/tests/main/files.spec.ts index d0d1cbd5c3..c57c497d57 100644 --- a/tests/main/files.spec.ts +++ b/tests/main/files.spec.ts @@ -56,14 +56,6 @@ describe('files', () => { const spy = vi.spyOn(ipcMainManager, 'handle'); setupFileListeners(); - expect(spy).toHaveBeenCalledWith( - IpcEvents.CLEANUP_DIRECTORY, - expect.anything(), - ); - expect(spy).toHaveBeenCalledWith( - IpcEvents.DELETE_USER_DATA, - expect.anything(), - ); expect(spy).toHaveBeenCalledWith( IpcEvents.SAVE_FILES_TO_TEMP, expect.anything(), diff --git a/tests/mocks/electron-fiddle.ts b/tests/mocks/electron-fiddle.ts index 5dce2849cf..cdf16db9ac 100644 --- a/tests/mocks/electron-fiddle.ts +++ b/tests/mocks/electron-fiddle.ts @@ -5,10 +5,8 @@ export class ElectronFiddleMock { public addModules = vi.fn(); public arch = process.arch; public blockAccelerators = vi.fn(); - public cleanupDirectory = vi.fn(); public confirmQuit = vi.fn(); public createThemeFile = vi.fn(); - public deleteUserData = vi.fn(); public downloadVersion = vi.fn(); public fetchVersions = vi.fn(); public getAvailableThemes = vi.fn(); diff --git a/tests/renderer/runner.spec.tsx b/tests/renderer/runner.spec.tsx index 1e47a8f3d5..d29124a08c 100644 --- a/tests/renderer/runner.spec.tsx +++ b/tests/renderer/runner.spec.tsx @@ -30,8 +30,6 @@ describe('Runner component', () => { vi.mocked( window.ElectronFiddle.getIsPackageManagerInstalled, ).mockResolvedValue(true); - vi.mocked(window.ElectronFiddle.deleteUserData).mockResolvedValue(); - instance = new Runner(store as unknown as AppState); }); From 203f2e1a5e5af6d395d15cca3ab46bdcadab1ba0 Mon Sep 17 00:00:00 2001 From: David Sanders Date: Fri, 8 May 2026 11:40:26 -0400 Subject: [PATCH 07/22] refactor: remove runner.stop wrapper method --- src/renderer/components/commands-bisect.tsx | 4 +-- src/renderer/components/commands-runner.tsx | 2 +- src/renderer/runner.ts | 8 ----- tests/renderer/runner.spec.tsx | 34 --------------------- 4 files changed, 3 insertions(+), 45 deletions(-) diff --git a/src/renderer/components/commands-bisect.tsx b/src/renderer/components/commands-bisect.tsx index 311ccf78a4..3e8ff8cf66 100644 --- a/src/renderer/components/commands-bisect.tsx +++ b/src/renderer/components/commands-bisect.tsx @@ -20,7 +20,7 @@ export const BisectHandler = observer( } private continueBisect(isGood: boolean) { - window.app.runner.stop(); + window.ElectronFiddle.stopFiddle(); const { appState } = this.props; const response = appState.Bisector!.continue(isGood); @@ -52,7 +52,7 @@ export const BisectHandler = observer( } private skipBisect() { - window.app.runner.stop(); + window.ElectronFiddle.stopFiddle(); const { appState } = this.props; const response = appState.Bisector!.skip(); diff --git a/src/renderer/components/commands-runner.tsx b/src/renderer/components/commands-runner.tsx index c7bc4bdef7..c218c43c36 100644 --- a/src/renderer/components/commands-runner.tsx +++ b/src/renderer/components/commands-runner.tsx @@ -56,7 +56,7 @@ export const Runner = observer( } else if (isRunning) { props.active = true; props.text = 'Stop'; - props.onClick = window.app.runner.stop; + props.onClick = () => window.ElectronFiddle.stopFiddle(); props.icon = 'stop'; } else { props.text = 'Run'; diff --git a/src/renderer/runner.ts b/src/renderer/runner.ts index 686669ae81..0877be18dd 100644 --- a/src/renderer/runner.ts +++ b/src/renderer/runner.ts @@ -29,7 +29,6 @@ const resultString: Record = Object.freeze({ export class Runner { constructor(private readonly appState: AppState) { this.run = this.run.bind(this); - this.stop = this.stop.bind(this); this.getStartFiddleOptions = this.getStartFiddleOptions.bind(this); window.ElectronFiddle.removeAllListeners('run-fiddle'); @@ -195,13 +194,6 @@ export class Runner { return this.runFiddle(); } - /** - * Stop a currently running Electron fiddle. - */ - public stop(): void { - window.ElectronFiddle.stopFiddle(); - } - /** * Uses electron-forge to either package or make the current fiddle */ diff --git a/tests/renderer/runner.spec.tsx b/tests/renderer/runner.spec.tsx index d29124a08c..7e6a8e189b 100644 --- a/tests/renderer/runner.spec.tsx +++ b/tests/renderer/runner.spec.tsx @@ -219,40 +219,6 @@ describe('Runner component', () => { }); }); - describe('stop()', () => { - it('stops a running session', async () => { - vi.mocked(window.ElectronFiddle.stopFiddle).mockImplementationOnce(() => { - emitEvent('fiddle-stopped', RunResult.FAILURE); - }); - - // wait for run() to get running - const runPromise = instance.run(); - await vi.waitUntil(() => store.isRunning); - expect(store.isRunning).toBe(true); - - // call stop and wait for run() to resolve - instance.stop(); - const runResult = await runPromise; - - expect(runResult).toBe(RunResult.FAILURE); - expect(store.isRunning).toBe(false); - }); - - it('fails if stopping fiddle fails', async () => { - vi.mocked(window.ElectronFiddle.stopFiddle).mockImplementationOnce( - () => {}, - ); - - // wait for run() to get running - instance.run(); - await vi.waitUntil(() => store.isRunning); - expect(store.isRunning).toBe(true); - - instance.stop(); - expect(store.isRunning).toBe(true); - }); - }); - describe('autobisect()', () => { it('returns success if bisection succeeds', async () => { // make sure good, bad exist in our mock From 201c8091b042cf0f2f91c662eb8b3234ad698704 Mon Sep 17 00:00:00 2001 From: David Sanders Date: Fri, 8 May 2026 14:23:28 -0400 Subject: [PATCH 08/22] chore: move push output helpers to utils --- src/main/fiddle-core.ts | 33 +-------------------------- src/main/utils/push-output.ts | 43 +++++++++++++++++++++++++++++++++++ 2 files changed, 44 insertions(+), 32 deletions(-) create mode 100644 src/main/utils/push-output.ts diff --git a/src/main/fiddle-core.ts b/src/main/fiddle-core.ts index 49d702f0d0..7b2d4784fc 100644 --- a/src/main/fiddle-core.ts +++ b/src/main/fiddle-core.ts @@ -15,6 +15,7 @@ import { ipcMainManager } from './ipc'; import { addModules, getIsPackageManagerInstalled } from './npm'; import { getFiles } from './utils/get-files'; import { getStartFiddleOptions } from './utils/get-start-fiddle-options'; +import { pushError, pushOutput, pushOutputLine } from './utils/push-output'; import { getLocalVersions } from './versions'; import { DownloadVersionParams, @@ -50,38 +51,6 @@ const fiddleProcesses = new WeakMap(); const downloadingVersions = new Map>(); const removingVersions = new Map>(); -/** - * Push to the renderer's run output. - */ -function pushOutput( - webContents: WebContents, - message: string, - options?: { isNotPre?: boolean }, -): void { - ipcMainManager.send( - IpcEvents.FIDDLE_RUNNER_OUTPUT, - [message, options], - webContents, - ); -} - -function pushOutputLine( - webContents: WebContents, - message: string, - options?: { isNotPre?: boolean }, -): void { - pushOutput(webContents, `${message}\n`, options); -} - -/** - * Little convenience method that pushes message and error. - */ -function pushError(webContents: WebContents, message: string, error: Error) { - pushOutput(webContents, `⚠️ ${message}. Error encountered:`); - pushOutput(webContents, error.toString()); - console.warn(error); -} - /** * Installs the specified modules */ diff --git a/src/main/utils/push-output.ts b/src/main/utils/push-output.ts new file mode 100644 index 0000000000..66a18b0f65 --- /dev/null +++ b/src/main/utils/push-output.ts @@ -0,0 +1,43 @@ +import type { WebContents } from 'electron'; + +import { IpcEvents } from '../../ipc-events'; +import { ipcMainManager } from '../ipc'; + +/** + * Push to the renderer's run output. + */ +export function pushOutput( + webContents: WebContents, + message: string, + options?: { isNotPre?: boolean }, +): void { + ipcMainManager.send( + IpcEvents.FIDDLE_RUNNER_OUTPUT, + [message, options], + webContents, + ); +} + +/** + * Push a line to the renderer's run output. + */ +export function pushOutputLine( + webContents: WebContents, + message: string, + options?: { isNotPre?: boolean }, +): void { + pushOutput(webContents, `${message}\n`, options); +} + +/** + * Little convenience method that pushes message and error. + */ +export function pushError( + webContents: WebContents, + message: string, + error: Error, +) { + pushOutput(webContents, `⚠️ ${message}. Error encountered:`); + pushOutput(webContents, error.toString()); + console.warn(error); +} From 426b792d745386d006c59cfa8209f36d25fd4327 Mon Sep 17 00:00:00 2001 From: David Sanders Date: Fri, 8 May 2026 12:26:51 -0400 Subject: [PATCH 09/22] refactor: move bisect.ts to common utils --- src/renderer/components/dialog-bisect.tsx | 2 +- src/renderer/state.ts | 2 +- src/{renderer => utils}/bisect.ts | 0 tests/renderer/components/dialog-bisect.spec.tsx | 4 ++-- tests/renderer/state.spec.ts | 2 +- tests/{renderer => utils}/bisect.spec.ts | 2 +- 6 files changed, 6 insertions(+), 6 deletions(-) rename src/{renderer => utils}/bisect.ts (100%) rename tests/{renderer => utils}/bisect.spec.ts (97%) diff --git a/src/renderer/components/dialog-bisect.tsx b/src/renderer/components/dialog-bisect.tsx index 69281ea599..4b22508ba0 100644 --- a/src/renderer/components/dialog-bisect.tsx +++ b/src/renderer/components/dialog-bisect.tsx @@ -5,7 +5,7 @@ import { observer } from 'mobx-react'; import { VersionSelect } from './version-select'; import { RunnableVersion } from '../../interfaces'; -import { Bisector } from '../bisect'; +import { Bisector } from '../../utils/bisect'; import { AppState } from '../state'; interface BisectDialogProps { diff --git a/src/renderer/state.ts b/src/renderer/state.ts index 7481bbbbac..925ad22a83 100644 --- a/src/renderer/state.ts +++ b/src/renderer/state.ts @@ -7,7 +7,6 @@ import { when, } from 'mobx'; -import { Bisector } from './bisect'; import { EditorMosaic } from './editor-mosaic'; import { ELECTRON_MIRROR } from './mirror-constants'; import { normalizeVersion } from './utils/normalize-version'; @@ -41,6 +40,7 @@ import { VersionSource, WindowSpecificSetting, } from '../interfaces'; +import { Bisector } from '../utils/bisect'; /** * The application's state. Exported as a singleton below. diff --git a/src/renderer/bisect.ts b/src/utils/bisect.ts similarity index 100% rename from src/renderer/bisect.ts rename to src/utils/bisect.ts diff --git a/tests/renderer/components/dialog-bisect.spec.tsx b/tests/renderer/components/dialog-bisect.spec.tsx index 11d734f01c..6254614cae 100644 --- a/tests/renderer/components/dialog-bisect.spec.tsx +++ b/tests/renderer/components/dialog-bisect.spec.tsx @@ -7,14 +7,14 @@ import { RunResult, VersionSource, } from '../../../src/interfaces'; -import { Bisector } from '../../../src/renderer/bisect'; import { BisectDialog } from '../../../src/renderer/components/dialog-bisect'; import { Runner } from '../../../src/renderer/runner'; import { AppState } from '../../../src/renderer/state'; +import { Bisector } from '../../../src/utils/bisect'; import { StateMock } from '../../mocks/mocks'; import { renderClassComponentWithInstanceRef } from '../utils/renderClassComponentWithInstanceRef'; -vi.mock('../../../src/renderer/bisect'); +vi.mock('../../../src/utils/bisect'); describe.each([8, 15])('BisectDialog component', (numVersions) => { let runner: Runner; diff --git a/tests/renderer/state.spec.ts b/tests/renderer/state.spec.ts index fedd88b77f..c129b991af 100644 --- a/tests/renderer/state.spec.ts +++ b/tests/renderer/state.spec.ts @@ -21,13 +21,13 @@ import { Version, VersionSource, } from '../../src/interfaces'; -import { Bisector } from '../../src/renderer/bisect'; import { AppState } from '../../src/renderer/state'; import { fetchVersions, getElectronVersions, makeRunnable, } from '../../src/renderer/versions'; +import { Bisector } from '../../src/utils/bisect'; import { VersionsMock, createEditorValues } from '../mocks/mocks'; import { overrideRendererPlatform, resetRendererPlatform } from '../utils'; diff --git a/tests/renderer/bisect.spec.ts b/tests/utils/bisect.spec.ts similarity index 97% rename from tests/renderer/bisect.spec.ts rename to tests/utils/bisect.spec.ts index ba67e866b5..5671ee6657 100644 --- a/tests/renderer/bisect.spec.ts +++ b/tests/utils/bisect.spec.ts @@ -1,7 +1,7 @@ import { beforeEach, describe, expect, it } from 'vitest'; import { InstallState, VersionSource } from '../../src/interfaces'; -import { Bisector } from '../../src/renderer/bisect'; +import { Bisector } from '../../src/utils/bisect'; const generateVersionRange = (rangeLength: number) => new Array(rangeLength).fill(0).map((_, i) => ({ From 9787428ce7924b457a77d0bcaf6a8c626b4dece2 Mon Sep 17 00:00:00 2001 From: David Sanders Date: Fri, 8 May 2026 12:33:02 -0400 Subject: [PATCH 10/22] chore: move autobisect to main process --- src/ambient.d.ts | 6 + src/interfaces.ts | 1 + src/ipc-events.ts | 4 + src/main/autobisect.ts | 117 ++++++++++++++++++ src/main/main.ts | 2 + src/main/utils/set-version.ts | 28 +++++ src/preload/preload.ts | 11 ++ src/renderer/components/dialog-bisect.tsx | 6 +- src/renderer/runner.ts | 103 ++------------- tests/mocks/electron-fiddle.ts | 2 + .../components/dialog-bisect.spec.tsx | 9 +- 11 files changed, 188 insertions(+), 101 deletions(-) create mode 100644 src/main/autobisect.ts create mode 100644 src/main/utils/set-version.ts diff --git a/src/ambient.d.ts b/src/ambient.d.ts index 3a5536ba62..31f1f7770f 100644 --- a/src/ambient.d.ts +++ b/src/ambient.d.ts @@ -48,6 +48,10 @@ declare global { type: 'fiddle-stopped', listener: (code: number | null, signal: string | null) => void, ): void; + addEventListener( + type: 'is-auto-bisecting', + listener: (isAutoBisecting: boolean) => void, + ): void; addEventListener( type: 'load-example', listener: (exampleInfo: { path: string; tag: string }) => void, @@ -90,6 +94,7 @@ declare global { ...names: Array ): Promise; arch: string; + autobisectFiddle(versions: Array): void; blockAccelerators(acceleratorsToBlock: BlockableAccelerator[]): void; confirmQuit(): void; createThemeFile( @@ -138,6 +143,7 @@ declare global { onGetStartFiddleOptions( callback: () => Promise, ): void; + onSetVersion(callback: (version: string) => Promise): void; openThemeFolder(): Promise; packageRun( { dir, packageManager }: PMOperationOptions, diff --git a/src/interfaces.ts b/src/interfaces.ts index cd069f4c95..5330430564 100644 --- a/src/interfaces.ts +++ b/src/interfaces.ts @@ -171,6 +171,7 @@ export type FiddleEvent = | 'fiddle-runner-output' | 'fiddle-modules-installed' | 'fiddle-stopped' + | 'is-auto-bisecting' | 'load-example' | 'load-gist' | 'make-fiddle' diff --git a/src/ipc-events.ts b/src/ipc-events.ts index 7ece7a9011..5db61d82e6 100644 --- a/src/ipc-events.ts +++ b/src/ipc-events.ts @@ -64,8 +64,11 @@ export enum IpcEvents { SAVED_LOCAL_FIDDLE = 'SAVED_LOCAL_FIDDLE', GET_FILES = 'GET_FILES', GET_START_FIDDLE_OPTIONS = 'GET_START_FIDDLE_OPTIONS', + SET_VERSION = 'SET_VERSION', START_FIDDLE = 'START_FIDDLE', STOP_FIDDLE = 'STOP_FIDDLE', + AUTOBISECT_FIDDLE = 'AUTOBISECT_FIDDLE', + IS_AUTO_BISECTING = 'IS_AUTO_BISECTING', GET_VERSION_STATE = 'GET_VERSION_STATE', DOWNLOAD_VERSION = 'DOWNLOAD_VERSION', REMOVE_VERSION = 'REMOVE_VERSION', @@ -106,6 +109,7 @@ export const ipcMainEvents = [ IpcEvents.GET_RELEASED_VERSIONS, IpcEvents.GET_USERNAME, IpcEvents.STOP_FIDDLE, + IpcEvents.AUTOBISECT_FIDDLE, IpcEvents.GET_VERSION_STATE, ] as const; diff --git a/src/main/autobisect.ts b/src/main/autobisect.ts new file mode 100644 index 0000000000..2a23ec431f --- /dev/null +++ b/src/main/autobisect.ts @@ -0,0 +1,117 @@ +import type { IpcMainEvent, WebContents } from 'electron'; + +import { ipcMainManager } from './ipc'; +import { pushOutputLine } from './utils/push-output'; +import { setVersion } from './utils/set-version'; +import { RunResult, RunnableVersion } from '../interfaces'; +import { IpcEvents } from '../ipc-events'; +import { Bisector } from '../utils/bisect'; + +const resultString: Record = Object.freeze({ + [RunResult.FAILURE]: '❌ failed', + [RunResult.INVALID]: '❓ invalid', + [RunResult.SUCCESS]: '✅ passed', +}); + +/** + * Bisect the current fiddle across the specified versions. + * + * @param versions - versions to bisect + */ +async function autobisect( + webContents: WebContents, + versions: Array, +) { + try { + ipcMainManager.send(IpcEvents.IS_AUTO_BISECTING, [true], webContents); + await autobisectImpl(webContents, versions); + } finally { + ipcMainManager.send(IpcEvents.IS_AUTO_BISECTING, [false], webContents); + } +} + +async function autobisectImpl( + webContents: WebContents, + versions: Array, +) { + const prefix = `Runner: autobisect`; + + // precondition: can't bisect unless we have >= 2 versions + if (versions.length < 2) { + pushOutputLine( + webContents, + `${prefix} needs at least two Electron versions`, + ); + return; + } + + const results: Map = new Map(); + + const runVersion = async (version: string) => { + const result = results.get(version); + if (result === undefined) { + const pre = `${prefix} Electron ${version} -`; + pushOutputLine(webContents, `${pre} setting version`); + await setVersion(webContents, version); + pushOutputLine(webContents, `${pre} starting test`); + // TODO - result = await this.run(); + results.set(version, result); + pushOutputLine( + webContents, + `${pre} finished test ${resultString[result]}`, + ); + } + return result; + }; + + const bisector = new Bisector(versions); + let targetVersion = bisector.getCurrentVersion(); + let next; + while (true) { + const { version } = targetVersion; + + const result = await runVersion(version); + if (result === RunResult.INVALID) { + return result; + } + + next = bisector.continue(result === RunResult.SUCCESS); + if (Array.isArray(next)) { + break; + } + + targetVersion = next; + } + + const [good, bad] = next.map((v) => v.version); + const resultGood = await runVersion(good); + const resultBad = await runVersion(bad); + if (resultGood === resultBad) { + pushOutputLine( + webContents, + `${prefix} 'good' ${good} and 'bad' ${bad} both returned ${resultString[resultGood]}`, + ); + return; + } + + const msgs = [ + `${prefix} complete`, + `${prefix} ${resultString[RunResult.SUCCESS]} ${good}`, + `${prefix} ${resultString[RunResult.FAILURE]} ${bad}`, + `${prefix} Commits between versions:`, + `https://github.com/electron/electron/compare/v${good}...v${bad}`, + ]; + msgs.forEach((msg) => pushOutputLine(webContents, msg)); +} + +/** + * Wire up the IPC handler so the renderer can trigger an autobisect. + */ +export function setupAutobisect(): void { + ipcMainManager.on( + IpcEvents.AUTOBISECT_FIDDLE, + (event: IpcMainEvent, versions: Array) => { + autobisect(event.sender, versions).catch(console.error); + }, + ); +} diff --git a/src/main/main.ts b/src/main/main.ts index 3776940409..2093eabe59 100644 --- a/src/main/main.ts +++ b/src/main/main.ts @@ -13,6 +13,7 @@ import { } from 'electron'; import { setupAboutPanel } from './about-panel'; +import { setupAutobisect } from './autobisect'; import { setupContent } from './content'; import { setupDevTools } from './devtools'; import { setupDialogs } from './dialogs'; @@ -67,6 +68,7 @@ export async function onReady() { setupGetUsername(); setupTypes(knownVersions); await setupFiddleCore(knownVersions); + setupAutobisect(); // Do this after setting everything up to ensure that // any IPC listeners are set up before they're used diff --git a/src/main/utils/set-version.ts b/src/main/utils/set-version.ts new file mode 100644 index 0000000000..c1baf0a00a --- /dev/null +++ b/src/main/utils/set-version.ts @@ -0,0 +1,28 @@ +import { MessageChannelMain } from 'electron'; + +import { IpcEvents } from '../../ipc-events'; +import { ipcMainManager } from '../ipc'; + +/** + * Asks the renderer to update `appState`'s active Electron version. + * Resolves once the renderer has finished setting the version. + */ +export function setVersion( + webContents: Electron.WebContents, + version: string, +): Promise { + return new Promise((resolve) => { + const { port1, port2 } = new MessageChannelMain(); + ipcMainManager.postMessage( + IpcEvents.SET_VERSION, + version, + [port1], + webContents, + ); + port2.once('message', () => { + resolve(); + port2.close(); + }); + port2.start(); + }); +} diff --git a/src/preload/preload.ts b/src/preload/preload.ts index e2ee9b0ff0..bd3ac43112 100644 --- a/src/preload/preload.ts +++ b/src/preload/preload.ts @@ -26,6 +26,7 @@ const channelMapping: Record = { 'fiddle-runner-output': IpcEvents.FIDDLE_RUNNER_OUTPUT, 'fiddle-modules-installed': IpcEvents.FIDDLE_MODULES_INSTALLED, 'fiddle-stopped': IpcEvents.FIDDLE_STOPPED, + 'is-auto-bisecting': IpcEvents.IS_AUTO_BISECTING, 'load-example': IpcEvents.LOAD_ELECTRON_EXAMPLE_REQUEST, 'load-gist': IpcEvents.LOAD_GIST_REQUEST, 'make-fiddle': IpcEvents.FIDDLE_MAKE, @@ -84,6 +85,9 @@ export async function setupFiddleGlobal() { ); }, arch: process.arch, + autobisectFiddle(versions: Array): void { + ipcRenderer.send(IpcEvents.AUTOBISECT_FIDDLE, versions); + }, blockAccelerators(acceleratorsToBlock: BlockableAccelerator[]) { ipcRenderer.send(IpcEvents.BLOCK_ACCELERATORS, acceleratorsToBlock); }, @@ -192,6 +196,13 @@ export async function setupFiddleGlobal() { e.ports[0].postMessage(options); }); }, + onSetVersion(callback: (version: string) => Promise) { + ipcRenderer.removeAllListeners(IpcEvents.SET_VERSION); + ipcRenderer.on(IpcEvents.SET_VERSION, async (e, version: string) => { + await callback(version); + e.ports[0].postMessage(undefined); + }); + }, async openThemeFolder() { await ipcRenderer.invoke(IpcEvents.OPEN_THEME_FOLDER); }, diff --git a/src/renderer/components/dialog-bisect.tsx b/src/renderer/components/dialog-bisect.tsx index 4b22508ba0..753a6ea1aa 100644 --- a/src/renderer/components/dialog-bisect.tsx +++ b/src/renderer/components/dialog-bisect.tsx @@ -81,7 +81,11 @@ export const BisectDialog = observer( public async onAuto(): Promise { const range = this.getBisectRange(); if (range.length > 1) { - window.app.runner.autobisect(range); + // the RunnableVersion proxies can't be cloned by structuredClone, + // so we have to create plain objects out of them + window.ElectronFiddle.autobisectFiddle( + range.map((version) => ({ ...version })), + ); this.onClose(); } } diff --git a/src/renderer/runner.ts b/src/renderer/runner.ts index 0877be18dd..8197eb9f66 100644 --- a/src/renderer/runner.ts +++ b/src/renderer/runner.ts @@ -1,7 +1,6 @@ import parseEnvString from 'parse-env-string'; import semver from 'semver'; -import { Bisector } from './bisect'; import { AppState } from './state'; import { FileTransformOperation, @@ -10,7 +9,6 @@ import { PMOperationOptions, PackageJsonOptions, RunResult, - RunnableVersion, StartFiddleOptions, VersionSource, } from '../interfaces'; @@ -20,12 +18,6 @@ export enum ForgeCommands { MAKE = 'make', } -const resultString: Record = Object.freeze({ - [RunResult.FAILURE]: '❌ failed', - [RunResult.INVALID]: '❓ invalid', - [RunResult.SUCCESS]: '✅ passed', -}); - export class Runner { constructor(private readonly appState: AppState) { this.run = this.run.bind(this); @@ -34,6 +26,7 @@ export class Runner { window.ElectronFiddle.removeAllListeners('run-fiddle'); window.ElectronFiddle.removeAllListeners('package-fiddle'); window.ElectronFiddle.removeAllListeners('make-fiddle'); + window.ElectronFiddle.removeAllListeners('is-auto-bisecting'); window.ElectronFiddle.addEventListener('run-fiddle', this.run); window.ElectronFiddle.addEventListener('package-fiddle', () => { @@ -42,93 +35,17 @@ export class Runner { window.ElectronFiddle.addEventListener('make-fiddle', () => { this.performForgeOperation(ForgeCommands.MAKE); }); + window.ElectronFiddle.addEventListener( + 'is-auto-bisecting', + (isAutoBisecting: boolean) => { + this.appState.isAutoBisecting = isAutoBisecting; + }, + ); window.ElectronFiddle.onGetStartFiddleOptions(this.getStartFiddleOptions); - } - - /** - * Bisect the current fiddle across the specified versions. - * - * @param versions - versions to bisect - */ - public autobisect(versions: Array): Promise { - const { appState } = this; - appState.isAutoBisecting = true; - const done = () => (appState.isAutoBisecting = false); - return this.autobisectImpl(versions).finally(done); - } - - /** - * Bisect the current fiddle across the specified versions. - * - * @param versions - versions to bisect - */ - public async autobisectImpl( - versions: Array, - ): Promise { - const prefix = `Runner: autobisect`; - const { appState } = this; - - // precondition: can't bisect unless we have >= 2 versions - if (versions.length < 2) { - appState.pushOutput(`${prefix} needs at least two Electron versions`); - return RunResult.INVALID; - } - - const results: Map = new Map(); - - const runVersion = async (version: string) => { - let result = results.get(version); - if (result === undefined) { - const pre = `${prefix} Electron ${version} -`; - appState.pushOutput(`${pre} setting version`); - await appState.setVersion(version); - appState.pushOutput(`${pre} starting test`); - result = await this.run(); - results.set(version, result); - appState.pushOutput(`${pre} finished test ${resultString[result]}`); - } - return result; - }; - - const bisector = new Bisector(versions); - let targetVersion = bisector.getCurrentVersion(); - let next; - while (true) { - const { version } = targetVersion; - - const result = await runVersion(version); - if (result === RunResult.INVALID) { - return result; - } - - next = bisector.continue(result === RunResult.SUCCESS); - if (Array.isArray(next)) { - break; - } - - targetVersion = next; - } - - const [good, bad] = next.map((v) => v.version); - const resultGood = await runVersion(good); - const resultBad = await runVersion(bad); - if (resultGood === resultBad) { - appState.pushOutput( - `${prefix} 'good' ${good} and 'bad' ${bad} both returned ${resultString[resultGood]}`, - ); - return RunResult.INVALID; - } - - const msgs = [ - `${prefix} complete`, - `${prefix} ${resultString[RunResult.SUCCESS]} ${good}`, - `${prefix} ${resultString[RunResult.FAILURE]} ${bad}`, - `${prefix} Commits between versions:`, - `https://github.com/electron/electron/compare/v${good}...v${bad}`, - ]; - msgs.forEach((msg) => appState.pushOutput(msg)); - return RunResult.SUCCESS; + window.ElectronFiddle.onSetVersion((version: string) => + this.appState.setVersion(version), + ); } /** diff --git a/tests/mocks/electron-fiddle.ts b/tests/mocks/electron-fiddle.ts index d7eff0e0e5..d6bb95af8c 100644 --- a/tests/mocks/electron-fiddle.ts +++ b/tests/mocks/electron-fiddle.ts @@ -4,6 +4,7 @@ export class ElectronFiddleMock { public addEventListener = vi.fn(); public addModules = vi.fn(); public arch = process.arch; + public autobisectFiddle = vi.fn(); public blockAccelerators = vi.fn(); public confirmQuit = vi.fn(); public createThemeFile = vi.fn(); @@ -33,6 +34,7 @@ export class ElectronFiddleMock { public macTitlebarClicked = vi.fn(); public onGetFiles = vi.fn(); public onGetStartFiddleOptions = vi.fn(); + public onSetVersion = vi.fn(); public openThemeFolder = vi.fn(); public packageRun = vi.fn(); public platform = process.platform; diff --git a/tests/renderer/components/dialog-bisect.spec.tsx b/tests/renderer/components/dialog-bisect.spec.tsx index 6254614cae..0a56fe4600 100644 --- a/tests/renderer/components/dialog-bisect.spec.tsx +++ b/tests/renderer/components/dialog-bisect.spec.tsx @@ -4,11 +4,9 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'; import { ElectronReleaseChannel, InstallState, - RunResult, VersionSource, } from '../../../src/interfaces'; import { BisectDialog } from '../../../src/renderer/components/dialog-bisect'; -import { Runner } from '../../../src/renderer/runner'; import { AppState } from '../../../src/renderer/state'; import { Bisector } from '../../../src/utils/bisect'; import { StateMock } from '../../mocks/mocks'; @@ -17,7 +15,6 @@ import { renderClassComponentWithInstanceRef } from '../utils/renderClassCompone vi.mock('../../../src/utils/bisect'); describe.each([8, 15])('BisectDialog component', (numVersions) => { - let runner: Runner; let store: AppState; const generateVersionRange = (rangeLength: number) => @@ -28,7 +25,7 @@ describe.each([8, 15])('BisectDialog component', (numVersions) => { })); beforeEach(() => { - ({ runner, state: store } = window.app); + ({ state: store } = window.app); (store as unknown as StateMock).versionsToShow = generateVersionRange(numVersions); @@ -201,13 +198,11 @@ describe.each([8, 15])('BisectDialog component', (numVersions) => { }); }); - vi.mocked(runner.autobisect).mockResolvedValue(RunResult.SUCCESS); - // click the 'auto' button await instance.onAuto(); // check the results - expect(runner.autobisect).toHaveBeenCalled(); + expect(window.ElectronFiddle.autobisectFiddle).toHaveBeenCalled(); }); it('does nothing if endIndex or startIndex are falsy', async () => { From e4bec97837c5f7a212e4fa749593eeb48aaef511 Mon Sep 17 00:00:00 2001 From: David Sanders Date: Fri, 8 May 2026 15:49:23 -0400 Subject: [PATCH 11/22] refactor(renderer): consolidate run and runFiddle --- src/renderer/components/commands-runner.tsx | 2 +- src/renderer/runner.ts | 128 ++++++++++---------- 2 files changed, 63 insertions(+), 67 deletions(-) diff --git a/src/renderer/components/commands-runner.tsx b/src/renderer/components/commands-runner.tsx index c218c43c36..192deb82e7 100644 --- a/src/renderer/components/commands-runner.tsx +++ b/src/renderer/components/commands-runner.tsx @@ -60,7 +60,7 @@ export const Runner = observer( props.icon = 'stop'; } else { props.text = 'Run'; - props.onClick = window.app.runner.run; + props.onClick = () => (window.app.runner as any).runFiddle(); props.icon = 'play'; } break; diff --git a/src/renderer/runner.ts b/src/renderer/runner.ts index 8197eb9f66..805df368c5 100644 --- a/src/renderer/runner.ts +++ b/src/renderer/runner.ts @@ -20,7 +20,7 @@ export enum ForgeCommands { export class Runner { constructor(private readonly appState: AppState) { - this.run = this.run.bind(this); + this.runFiddle = this.runFiddle.bind(this); this.getStartFiddleOptions = this.getStartFiddleOptions.bind(this); window.ElectronFiddle.removeAllListeners('run-fiddle'); @@ -28,7 +28,7 @@ export class Runner { window.ElectronFiddle.removeAllListeners('make-fiddle'); window.ElectronFiddle.removeAllListeners('is-auto-bisecting'); - window.ElectronFiddle.addEventListener('run-fiddle', this.run); + window.ElectronFiddle.addEventListener('run-fiddle', this.runFiddle); window.ElectronFiddle.addEventListener('package-fiddle', () => { this.performForgeOperation(ForgeCommands.PACKAGE); }); @@ -48,69 +48,6 @@ export class Runner { ); } - /** - * Actually run the fiddle. - */ - public async run(): Promise { - const { appState } = this; - const currentRunnable = appState.currentElectronVersion; - const { version, state } = currentRunnable; - const isValidBuild = - // Destructure currentRunnable so it's not a Proxy object, which can't be used - window.ElectronFiddle.getLocalVersionState({ ...currentRunnable }) === - InstallState.installed; - - // If the current active version is unavailable when we try to run - // the fiddle, show an error and fall back. - const { err, ver } = appState.isVersionUsable(version); - if (!ver) { - console.warn(`Running fiddle with version ('${version}') failed: ${err}`); - appState.showErrorDialog(err!); - const fallback = appState.findUsableVersion(); - if (fallback) await appState.setVersion(fallback.version); - return RunResult.INVALID; - } - - if ( - ver.source !== VersionSource.local && - semver.lt(ver.version, '28.0.0') && - !ver.version.startsWith('28.0.0-nightly') - ) { - const entryPoint = appState.editorMosaic.mainEntryPointFile(); - - if (entryPoint === MAIN_MJS) { - appState.showErrorDialog( - 'ESM main entry points are only supported starting in Electron 28', - ); - return RunResult.INVALID; - } - } - - if (appState.isClearingConsoleOnRun) { - appState.clearConsole(); - } - appState.isConsoleShowing = true; - - const isReady = - state === InstallState.installed || - state === InstallState.downloaded || - isValidBuild; - - if (!isReady) { - console.warn(`Runner: Binary ${version} not ready`); - - let message = `Could not start fiddle: `; - message += `Electron ${version} not downloaded yet. `; - message += `Please wait for it to finish downloading `; - message += `before running the fiddle.`; - - appState.pushOutput(message, { isNotPre: true }); - return RunResult.INVALID; - } - - return this.runFiddle(); - } - /** * Uses electron-forge to either package or make the current fiddle */ @@ -202,7 +139,38 @@ export class Runner { * simply triggers the run and waits for the `fiddle-stopped` event. */ private async runFiddle(): Promise { - const { pushOutput, flushOutput } = this.appState; + const { clearConsole, isClearingConsoleOnRun, pushOutput, flushOutput } = + this.appState; + const currentRunnable = this.appState.currentElectronVersion; + const { version, state } = currentRunnable; + + if (isClearingConsoleOnRun) { + clearConsole(); + } + this.appState.isConsoleShowing = true; + + const isValidBuild = + // Destructure currentRunnable so it's not a Proxy object, which can't be used + window.ElectronFiddle.getLocalVersionState({ ...currentRunnable }) === + InstallState.installed; + + const isReady = + state === InstallState.installed || + state === InstallState.downloaded || + isValidBuild; + + // TODO(dsanders11) - Should this be moved to main process? + if (!isReady) { + console.warn(`Runner: Binary ${version} not ready`); + + let message = `Could not start fiddle: `; + message += `Electron ${version} not downloaded yet. `; + message += `Please wait for it to finish downloading `; + message += `before running the fiddle.`; + + pushOutput(message, { isNotPre: true }); + return RunResult.INVALID; + } const cleanup = () => { flushOutput(); @@ -259,6 +227,34 @@ export class Runner { */ public async getStartFiddleOptions(): Promise { const { appState } = this; + const currentRunnable = appState.currentElectronVersion; + const { version } = currentRunnable; + + // If the current active version is unavailable when we try to run + // the fiddle, show an error and fall back. + const { err, ver } = appState.isVersionUsable(version); + if (!ver) { + console.warn(`Running fiddle with version ('${version}') failed: ${err}`); + appState.showErrorDialog(err!); + const fallback = appState.findUsableVersion(); + if (fallback) await appState.setVersion(fallback.version); + throw new Error(RunResult.INVALID); + } + + if ( + ver.source !== VersionSource.local && + semver.lt(ver.version, '28.0.0') && + !ver.version.startsWith('28.0.0-nightly') + ) { + const entryPoint = appState.editorMosaic.mainEntryPointFile(); + + if (entryPoint === MAIN_MJS) { + appState.showErrorDialog( + 'ESM main entry points are only supported starting in Electron 28', + ); + throw new Error(RunResult.INVALID); + } + } const modules = Array.from(appState.modules.entries()); if (modules.length > 0) { From 7c5e08f868a76e550c3c076b0301fd10d8353818 Mon Sep 17 00:00:00 2001 From: David Sanders Date: Fri, 8 May 2026 16:20:37 -0400 Subject: [PATCH 12/22] chore: fixup starting fiddle during autobisect --- src/main/autobisect.ts | 5 ++- src/main/fiddle-core.ts | 41 ++++++++++++++----- src/renderer/runner.ts | 74 ++++++++++++++++------------------ tests/main/fiddle-core.spec.ts | 35 +++++++++------- 4 files changed, 89 insertions(+), 66 deletions(-) diff --git a/src/main/autobisect.ts b/src/main/autobisect.ts index 2a23ec431f..0e4bcdd00f 100644 --- a/src/main/autobisect.ts +++ b/src/main/autobisect.ts @@ -1,5 +1,6 @@ import type { IpcMainEvent, WebContents } from 'electron'; +import { startFiddle } from './fiddle-core'; import { ipcMainManager } from './ipc'; import { pushOutputLine } from './utils/push-output'; import { setVersion } from './utils/set-version'; @@ -48,13 +49,13 @@ async function autobisectImpl( const results: Map = new Map(); const runVersion = async (version: string) => { - const result = results.get(version); + let result = results.get(version); if (result === undefined) { const pre = `${prefix} Electron ${version} -`; pushOutputLine(webContents, `${pre} setting version`); await setVersion(webContents, version); pushOutputLine(webContents, `${pre} starting test`); - // TODO - result = await this.run(); + result = await startFiddle(webContents); results.set(version, result); pushOutputLine( webContents, diff --git a/src/main/fiddle-core.ts b/src/main/fiddle-core.ts index 7b2d4784fc..6cb7c22fb6 100644 --- a/src/main/fiddle-core.ts +++ b/src/main/fiddle-core.ts @@ -22,6 +22,7 @@ import { IPackageManager, PACKAGE_NAME, ProgressObject, + RunResult, } from '../interfaces'; import { IpcEvents } from '../ipc-events'; import { maybePlural } from '../utils/plural-maybe'; @@ -103,8 +104,13 @@ export async function installModules( * Drive the entire run lifecycle from main: ask the renderer for the * settings and files, write them to a temp directory, install modules, * spawn Electron, and clean everything up when the process exits. + * + * Resolves with a {@link RunResult} describing whether the run was + * successful, failed, or was invalid. */ -export async function startFiddle(webContents: WebContents): Promise { +export async function startFiddle( + webContents: WebContents, +): Promise { const options = await getStartFiddleOptions(webContents); const { enableElectronLogging, @@ -174,7 +180,7 @@ export async function startFiddle(webContents: WebContents): Promise { pushError(webContents, 'Could not install modules', error); await cleanup(); - throw error; + return RunResult.FAILURE; } // Strip any CLI option containing a null byte, which can truncate @@ -207,9 +213,10 @@ export async function startFiddle(webContents: WebContents): Promise { cwd: dir, env, }); - } catch (error) { + } catch (error: any) { + pushError(webContents, 'Failed to spawn Fiddle', error); await cleanup(); - throw error; + return RunResult.FAILURE; } fiddleProcesses.set(webContents, child); @@ -223,10 +230,26 @@ export async function startFiddle(webContents: WebContents): Promise { child.stdout?.on('data', (data) => pushOutput(webContents, data.toString())); child.stderr?.on('data', (data) => pushOutput(webContents, data.toString())); - child.on('close', async (code, signal) => { - fiddleProcesses.delete(webContents); - await cleanup(); - ipcMainManager.send(IpcEvents.FIDDLE_STOPPED, [code, signal], webContents); + return new Promise((resolve) => { + child.on('close', async (code, signal) => { + fiddleProcesses.delete(webContents); + await cleanup(); + + let result: RunResult; + if (typeof code !== 'number') { + result = RunResult.FAILURE; + } else { + result = code === 0 ? RunResult.SUCCESS : RunResult.FAILURE; + } + + ipcMainManager.send( + IpcEvents.FIDDLE_STOPPED, + [code, signal], + webContents, + ); + + resolve(result); + }); }); } @@ -331,7 +354,7 @@ export async function setupFiddleCore(versions: ElectronVersions) { ipcMainManager.handle( IpcEvents.START_FIDDLE, async (event: IpcMainInvokeEvent) => { - await startFiddle(event.sender); + return await startFiddle(event.sender); }, ); ipcMainManager.on(IpcEvents.STOP_FIDDLE, (event: IpcMainEvent) => { diff --git a/src/renderer/runner.ts b/src/renderer/runner.ts index 805df368c5..29d34c2872 100644 --- a/src/renderer/runner.ts +++ b/src/renderer/runner.ts @@ -135,10 +135,11 @@ export class Runner { /** * Executes the fiddle with either local electron build * or the user selected electron version. The main process owns the - * temp directory, module installation, and cleanup — the renderer - * simply triggers the run and waits for the `fiddle-stopped` event. + * temp directory, module installation, spawning Electron, and cleanup — + * the renderer simply triggers the run, streams output back, and waits + * for `startFiddle()` to resolve. */ - private async runFiddle(): Promise { + private async runFiddle(): Promise { const { clearConsole, isClearingConsoleOnRun, pushOutput, flushOutput } = this.appState; const currentRunnable = this.appState.currentElectronVersion; @@ -169,7 +170,7 @@ export class Runner { message += `before running the fiddle.`; pushOutput(message, { isNotPre: true }); - return RunResult.INVALID; + return; } const cleanup = () => { @@ -178,47 +179,40 @@ export class Runner { this.appState.isInstallingModules = false; }; - return new Promise(async (resolve) => { - window.ElectronFiddle.removeAllListeners('fiddle-runner-output'); - window.ElectronFiddle.removeAllListeners('fiddle-modules-installed'); - window.ElectronFiddle.removeAllListeners('fiddle-stopped'); + window.ElectronFiddle.removeAllListeners('fiddle-runner-output'); + window.ElectronFiddle.removeAllListeners('fiddle-modules-installed'); + window.ElectronFiddle.removeAllListeners('fiddle-stopped'); - window.ElectronFiddle.addEventListener( - 'fiddle-runner-output', - (output: string, options?: { isNotPre?: boolean }) => { - pushOutput(output, { ...options, bypassBuffer: false }); - }, - ); - - window.ElectronFiddle.addEventListener('fiddle-modules-installed', () => { - this.appState.isInstallingModules = false; - }); - - window.ElectronFiddle.addEventListener( - 'fiddle-stopped', - (code, signal) => { - cleanup(); - - if (typeof code !== 'number') { - pushOutput(`Electron exited with signal ${signal}.`); - resolve(RunResult.FAILURE); - } else { - pushOutput(`Electron exited with code ${code}.`); - resolve(code === 0 ? RunResult.SUCCESS : RunResult.FAILURE); - } - }, - ); + window.ElectronFiddle.addEventListener( + 'fiddle-runner-output', + (output: string, options?: { isNotPre?: boolean }) => { + pushOutput(output, { ...options, bypassBuffer: false }); + }, + ); - this.appState.isRunning = true; + window.ElectronFiddle.addEventListener('fiddle-stopped', (code, signal) => { + cleanup(); - try { - await window.ElectronFiddle.startFiddle(); - } catch (e: any) { - pushOutput(`Failed to spawn Fiddle: ${e.message}`); - cleanup(); - resolve(RunResult.FAILURE); + if (typeof code !== 'number') { + pushOutput(`Electron exited with signal ${signal}.`); + } else { + pushOutput(`Electron exited with code ${code}.`); } }); + + window.ElectronFiddle.addEventListener('fiddle-modules-installed', () => { + this.appState.isInstallingModules = false; + }); + + this.appState.isRunning = true; + + try { + await window.ElectronFiddle.startFiddle(); + } catch (e: any) { + pushOutput(`Failed to spawn Fiddle: ${e.message}`); + } finally { + cleanup(); + } } /** diff --git a/tests/main/fiddle-core.spec.ts b/tests/main/fiddle-core.spec.ts index d4f976409d..8e2d80bef7 100644 --- a/tests/main/fiddle-core.spec.ts +++ b/tests/main/fiddle-core.spec.ts @@ -159,9 +159,20 @@ describe('fiddle-core', () => { describe('startFiddle', () => { const version = '18.0.0'; + function mockSpawnWithAutoClose( + child: ChildProcessMock, + code: number | null = 0, + signal: NodeJS.Signals | null = null, + ) { + vi.mocked(runner.spawn).mockImplementation(async () => { + setImmediate(() => child.emit('close', code, signal)); + return child; + }); + } + it('uses provided env', async () => { const child = new ChildProcessMock(); - vi.mocked(runner.spawn).mockResolvedValue(child); + mockSpawnWithAutoClose(child); getStartFiddleOptionsMock.mockResolvedValueOnce( makeOptions({ env: { NODE_OPTIONS: '--inspect-brk' } }), ); @@ -182,7 +193,7 @@ describe('fiddle-core', () => { it('runs with logging when enabled', async () => { const child = new ChildProcessMock(); - vi.mocked(runner.spawn).mockResolvedValue(child); + mockSpawnWithAutoClose(child); getStartFiddleOptionsMock.mockResolvedValueOnce( makeOptions({ enableElectronLogging: true }), ); @@ -205,7 +216,7 @@ describe('fiddle-core', () => { it('can set ELECTRON_ENABLE_LOGGING in env', async () => { const child = new ChildProcessMock(); - vi.mocked(runner.spawn).mockResolvedValue(child); + mockSpawnWithAutoClose(child); getStartFiddleOptionsMock.mockResolvedValueOnce( makeOptions({ env: { ELECTRON_ENABLE_LOGGING: 'true' } }), ); @@ -226,7 +237,7 @@ describe('fiddle-core', () => { it('strips blocked env keys from the renderer-supplied env', async () => { const child = new ChildProcessMock(); - vi.mocked(runner.spawn).mockResolvedValue(child); + mockSpawnWithAutoClose(child); getStartFiddleOptionsMock.mockResolvedValueOnce( makeOptions({ env: { @@ -257,7 +268,7 @@ describe('fiddle-core', () => { it('uses localPath when the version resolves to a local build with an existing executable', async () => { const child = new ChildProcessMock(); - vi.mocked(runner.spawn).mockResolvedValue(child); + mockSpawnWithAutoClose(child); const localPath = '/some/local/electron'; const localVersion = '0.0.0-local.123'; getLocalVersionsMock.mockReturnValueOnce([ @@ -281,7 +292,7 @@ describe('fiddle-core', () => { it('falls back to version when the local build executable does not exist', async () => { const child = new ChildProcessMock(); - vi.mocked(runner.spawn).mockResolvedValue(child); + mockSpawnWithAutoClose(child); const localPath = '/nonexistent/electron'; const localVersion = '0.0.0-local.456'; getLocalVersionsMock.mockReturnValueOnce([ @@ -303,7 +314,7 @@ describe('fiddle-core', () => { it('strips null bytes from execution flags', async () => { const child = new ChildProcessMock(); - vi.mocked(runner.spawn).mockResolvedValue(child); + mockSpawnWithAutoClose(child); getStartFiddleOptionsMock.mockResolvedValueOnce( makeOptions({ executionFlags: ['safe-flag', 'evil\0flag', '--js-flags=ok'], @@ -323,30 +334,24 @@ describe('fiddle-core', () => { it('cleans up the temp dir and user data after the child process closes', async () => { const child = new ChildProcessMock(); - vi.mocked(runner.spawn).mockResolvedValue(child); + mockSpawnWithAutoClose(child); getStartFiddleOptionsMock.mockResolvedValueOnce(makeOptions()); await startFiddle(new WebContentsMock() as unknown as WebContents); - child.emit('close', 0, null); - await new Promise(process.nextTick); - expect(filesMod.cleanupDirectory).toHaveBeenCalledWith(TEMP_DIR); expect(filesMod.deleteUserData).toHaveBeenCalledWith('test-app'); }); it('does not delete user data when isKeepingUserDataDirs is true', async () => { const child = new ChildProcessMock(); - vi.mocked(runner.spawn).mockResolvedValue(child); + mockSpawnWithAutoClose(child); getStartFiddleOptionsMock.mockResolvedValueOnce( makeOptions({ isKeepingUserDataDirs: true }), ); await startFiddle(new WebContentsMock() as unknown as WebContents); - child.emit('close', 0, null); - await new Promise(process.nextTick); - expect(filesMod.cleanupDirectory).toHaveBeenCalledWith(TEMP_DIR); expect(filesMod.deleteUserData).not.toHaveBeenCalled(); }); From 9ffed57f6ceb7323169e6899b547a92f99224f93 Mon Sep 17 00:00:00 2001 From: David Sanders Date: Fri, 8 May 2026 16:43:40 -0400 Subject: [PATCH 13/22] refactor(renderer): do not call startFiddle from runFiddle --- src/main/fiddle-core.ts | 2 ++ src/main/menu.ts | 8 +++++++- src/renderer/components/commands-runner.tsx | 2 +- src/renderer/runner.ts | 14 +------------- tests/main/menu.spec.ts | 16 +++++++++++++--- 5 files changed, 24 insertions(+), 18 deletions(-) diff --git a/src/main/fiddle-core.ts b/src/main/fiddle-core.ts index 6cb7c22fb6..f04358d405 100644 --- a/src/main/fiddle-core.ts +++ b/src/main/fiddle-core.ts @@ -123,6 +123,8 @@ export async function startFiddle( version, } = options; + ipcMainManager.send(IpcEvents.FIDDLE_RUN, [], webContents); + // Look up local Electron builds by version string. Local builds use a // version of the form `0.0.0-local.`, so only consult the // stored local versions when the version string contains `-local`. diff --git a/src/main/menu.ts b/src/main/menu.ts index b9856dce84..27371e1115 100644 --- a/src/main/menu.ts +++ b/src/main/menu.ts @@ -6,6 +6,7 @@ import { shell, } from 'electron'; +import { startFiddle } from './fiddle-core'; import { saveFiddle, saveFiddleAs, @@ -317,7 +318,12 @@ function getTasksMenu(): MenuItemConstructorOptions { { label: 'Run Fiddle...', accelerator: 'F5', - click: () => ipcMainManager.send(IpcEvents.FIDDLE_RUN), + click: (_, focusedWindow) => { + if (focusedWindow) + startFiddle((focusedWindow as BrowserWindow).webContents).catch( + console.error, + ); + }, }, { label: 'Package Fiddle...', diff --git a/src/renderer/components/commands-runner.tsx b/src/renderer/components/commands-runner.tsx index 192deb82e7..d0ed61d4d6 100644 --- a/src/renderer/components/commands-runner.tsx +++ b/src/renderer/components/commands-runner.tsx @@ -60,7 +60,7 @@ export const Runner = observer( props.icon = 'stop'; } else { props.text = 'Run'; - props.onClick = () => (window.app.runner as any).runFiddle(); + props.onClick = () => window.ElectronFiddle.startFiddle(); props.icon = 'play'; } break; diff --git a/src/renderer/runner.ts b/src/renderer/runner.ts index 29d34c2872..ba0cdfca29 100644 --- a/src/renderer/runner.ts +++ b/src/renderer/runner.ts @@ -133,11 +133,7 @@ export class Runner { } /** - * Executes the fiddle with either local electron build - * or the user selected electron version. The main process owns the - * temp directory, module installation, spawning Electron, and cleanup — - * the renderer simply triggers the run, streams output back, and waits - * for `startFiddle()` to resolve. + * Update the UI for running the fiddle. */ private async runFiddle(): Promise { const { clearConsole, isClearingConsoleOnRun, pushOutput, flushOutput } = @@ -205,14 +201,6 @@ export class Runner { }); this.appState.isRunning = true; - - try { - await window.ElectronFiddle.startFiddle(); - } catch (e: any) { - pushOutput(`Failed to spawn Fiddle: ${e.message}`); - } finally { - cleanup(); - } } /** diff --git a/tests/main/menu.spec.ts b/tests/main/menu.spec.ts index 61fbf7b259..77e21f2385 100644 --- a/tests/main/menu.spec.ts +++ b/tests/main/menu.spec.ts @@ -13,8 +13,9 @@ import { vi, } from 'vitest'; -import { BlockableAccelerator, MAIN_JS } from '../../src/interfaces'; +import { BlockableAccelerator, MAIN_JS, RunResult } from '../../src/interfaces'; import { IpcEvents } from '../../src/ipc-events'; +import { startFiddle } from '../../src/main/fiddle-core'; import { saveFiddle, saveFiddleAs, @@ -28,6 +29,7 @@ import { createMainWindow } from '../../src/main/windows'; import { overridePlatform, resetPlatform } from '../utils'; vi.mock('../../src/main/files'); +vi.mock('../../src/main/fiddle-core'); vi.mock('../../src/main/templates'); vi.mock('../../src/main/windows'); vi.mock('../../src/main/ipc'); @@ -311,8 +313,16 @@ describe('menu', () => { }); it('runs the fiddle', () => { - tasks.submenu[0].click(); - expect(ipcMainManager.send).toHaveBeenCalledWith(IpcEvents.FIDDLE_RUN); + vi.mocked(startFiddle).mockResolvedValueOnce(RunResult.SUCCESS); + const mocks = { + send: vi.fn(), + }; + const focusedWindow = { + webContents: + mocks as Partial as electron.WebContents, + } as Partial as electron.BrowserWindow; + tasks.submenu[0].click(undefined, focusedWindow); + expect(startFiddle).toHaveBeenCalledWith(focusedWindow.webContents); }); it('packages the fiddle', () => { From 9e346c49b68b59d44cc1ee97ada1f07d1a6893b6 Mon Sep 17 00:00:00 2001 From: David Sanders Date: Fri, 8 May 2026 17:09:27 -0400 Subject: [PATCH 14/22] test: update test coverage --- tests/main/autobisect.spec.ts | 163 ++++++++++++++++++ tests/renderer/runner.spec.tsx | 296 ++++++++++++++++----------------- 2 files changed, 304 insertions(+), 155 deletions(-) create mode 100644 tests/main/autobisect.spec.ts diff --git a/tests/main/autobisect.spec.ts b/tests/main/autobisect.spec.ts new file mode 100644 index 0000000000..683858f112 --- /dev/null +++ b/tests/main/autobisect.spec.ts @@ -0,0 +1,163 @@ +/** + * @vitest-environment node + */ + +import { WebContents } from 'electron'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +import { RunResult, RunnableVersion } from '../../src/interfaces'; +import { IpcEvents } from '../../src/ipc-events'; +import { setupAutobisect } from '../../src/main/autobisect'; +import { startFiddle } from '../../src/main/fiddle-core'; +import { ipcMainManager } from '../../src/main/ipc'; +import { pushOutputLine } from '../../src/main/utils/push-output'; +import { setVersion } from '../../src/main/utils/set-version'; +import { VersionsMock } from '../mocks/electron-versions'; +import { WebContentsMock } from '../mocks/web-contents'; + +vi.mock('../../src/main/fiddle-core'); +vi.mock('../../src/main/ipc'); +vi.mock('../../src/main/utils/push-output'); +vi.mock('../../src/main/utils/set-version'); + +describe('autobisect', () => { + let webContents: WebContents; + let mockVersions: Record; + let mockVersionsArray: RunnableVersion[]; + let invokeAutobisect: (versions: Array) => Promise; + + beforeEach(() => { + ({ mockVersions, mockVersionsArray } = new VersionsMock()); + webContents = new WebContentsMock() as unknown as WebContents; + + setupAutobisect(); + + const call = vi + .mocked(ipcMainManager.on) + .mock.calls.find( + ([channelName]) => channelName === IpcEvents.AUTOBISECT_FIDDLE, + ); + if (!call?.length || call.length < 2) { + throw new Error('Could not find AUTOBISECT_FIDDLE listener'); + } + const handler = call[1]; + invokeAutobisect = async (versions) => { + handler( + { sender: webContents } as unknown as Electron.IpcMainEvent, + versions, + ); + // The IPC listener triggers autobisect() but doesn't return its promise. + // Wait for the trailing IS_AUTO_BISECTING [false] signal to know the + // bisection has completed. + await vi.waitFor(() => { + const sent = vi + .mocked(ipcMainManager.send) + .mock.calls.some( + ([channel, args]) => + channel === IpcEvents.IS_AUTO_BISECTING && + Array.isArray(args) && + args[0] === false, + ); + if (!sent) throw new Error('autobisect not finished'); + }); + }; + }); + + it('registers the IPC listener', () => { + expect(ipcMainManager.on).toHaveBeenCalledWith( + IpcEvents.AUTOBISECT_FIDDLE, + expect.any(Function), + ); + }); + + it('reports IS_AUTO_BISECTING true while running and false after', async () => { + vi.mocked(startFiddle).mockResolvedValue(RunResult.SUCCESS); + + await invokeAutobisect([mockVersions['2.0.1']]); + + const autoBisectingCalls = vi + .mocked(ipcMainManager.send) + .mock.calls.filter( + ([channel]) => channel === IpcEvents.IS_AUTO_BISECTING, + ); + expect(autoBisectingCalls).toHaveLength(2); + expect(autoBisectingCalls[0][1]).toEqual([true]); + expect(autoBisectingCalls[1][1]).toEqual([false]); + }); + + it('bails out if not enough versions to bisect', async () => { + await invokeAutobisect([mockVersions['2.0.1']]); + + expect(pushOutputLine).toHaveBeenLastCalledWith( + webContents, + 'Runner: autobisect needs at least two Electron versions', + ); + }); + + it('completes a bisection with good and bad bounds', async () => { + const LAST_GOOD = '2.0.1'; + const FIRST_BAD = '2.0.2'; + expect(mockVersions[LAST_GOOD]).toEqual(expect.anything()); + expect(mockVersions[FIRST_BAD]).toEqual(expect.anything()); + + let currentVersion: string | undefined; + vi.mocked(setVersion).mockImplementation(async (_wc, version: string) => { + currentVersion = version; + }); + vi.mocked(startFiddle).mockImplementation(async () => { + // Run succeeds for versions <= LAST_GOOD, fails for newer ones + if (!currentVersion) { + throw new Error('setVersion was not called before startFiddle'); + } + const [a, b, c] = currentVersion.split('.').map(Number); + const [la, lb, lc] = LAST_GOOD.split('.').map(Number); + const cmp = a !== la ? a - la : b !== lb ? b - lb : c - lc; + return cmp <= 0 ? RunResult.SUCCESS : RunResult.FAILURE; + }); + + const bisectRange = [...mockVersionsArray].reverse(); + await invokeAutobisect(bisectRange); + + expect(setVersion).toHaveBeenCalled(); + expect(pushOutputLine).toHaveBeenLastCalledWith( + webContents, + `https://github.com/electron/electron/compare/v${LAST_GOOD}...v${FIRST_BAD}`, + ); + }); + + it('aborts the bisection when startFiddle returns INVALID', async () => { + vi.mocked(startFiddle).mockResolvedValue(RunResult.INVALID); + + const bisectRange = [...mockVersionsArray].reverse(); + await invokeAutobisect(bisectRange); + + expect(pushOutputLine).toHaveBeenLastCalledWith( + webContents, + expect.stringContaining('finished test ❓ invalid'), + ); + }); + + it('reports a tie when good and bad both succeed', async () => { + vi.mocked(startFiddle).mockResolvedValue(RunResult.SUCCESS); + + const bisectRange = [...mockVersionsArray].reverse(); + await invokeAutobisect(bisectRange); + + expect(pushOutputLine).toHaveBeenLastCalledWith( + webContents, + expect.stringMatching('both returned'), + ); + }); + + it('reports a tie when good and bad both fail', async () => { + vi.mocked(startFiddle).mockResolvedValue(RunResult.FAILURE); + + const bisectRange = [...mockVersionsArray].reverse(); + await invokeAutobisect(bisectRange); + + expect(pushOutputLine).toHaveBeenLastCalledWith( + webContents, + expect.stringMatching('both returned'), + ); + }); +}); diff --git a/tests/renderer/runner.spec.tsx b/tests/renderer/runner.spec.tsx index 7e6a8e189b..b1727972f9 100644 --- a/tests/renderer/runner.spec.tsx +++ b/tests/renderer/runner.spec.tsx @@ -1,8 +1,8 @@ -import * as semver from 'semver'; import { beforeEach, describe, expect, it, vi } from 'vitest'; import { InstallState, + MAIN_MJS, RunResult, RunnableVersion, VersionSource, @@ -18,10 +18,9 @@ describe('Runner component', () => { let store: StateMock; let instance: Runner; let mockVersions: Record; - let mockVersionsArray: RunnableVersion[]; beforeEach(() => { - ({ mockVersions, mockVersionsArray } = new VersionsMock()); + ({ mockVersions } = new VersionsMock()); ({ state: store } = window.app as unknown as AppMock); store.initVersions('2.0.2', { ...mockVersions }); store.getName.mockResolvedValue('test-app-name'); @@ -30,6 +29,9 @@ describe('Runner component', () => { vi.mocked( window.ElectronFiddle.getIsPackageManagerInstalled, ).mockResolvedValue(true); + vi.mocked(window.ElectronFiddle.getLocalVersionState).mockReturnValue( + InstallState.installed, + ); instance = new Runner(store as unknown as AppState); }); @@ -75,62 +77,21 @@ describe('Runner component', () => { }); }); - describe('run()', () => { - it('runs', async () => { - // wait for run() to get running - const runPromise = instance.run(); - await vi.waitUntil(() => store.isRunning); + describe('run-fiddle event', () => { + it('updates UI state when running starts', () => { + emitEvent('run-fiddle'); expect(store.isRunning).toBe(true); - - // fiddle exits with success - setTimeout(() => emitEvent('fiddle-stopped', 0)); - const result = await runPromise; - - expect(result).toBe(RunResult.SUCCESS); - expect(store.isRunning).toBe(false); - expect(window.ElectronFiddle.startFiddle).toHaveBeenCalled(); - }); - - it('exposes run-time options to the main process', async () => { - store.isEnablingElectronLogging = true; - const options = await instance.getStartFiddleOptions(); - expect(options).toEqual( - expect.objectContaining({ enableElectronLogging: true }), - ); - }); - - it('runs with logging when enabled', async () => { - store.isEnablingElectronLogging = true; - - // wait for run() to get running - const runPromise = instance.run(); - await vi.waitUntil(() => store.isRunning); - expect(store.isRunning).toBe(true); - - // fiddle exits with success - setTimeout(() => emitEvent('fiddle-stopped', 0)); - const result = await runPromise; - - expect(result).toBe(RunResult.SUCCESS); - expect(store.isRunning).toBe(false); - expect(window.ElectronFiddle.startFiddle).toHaveBeenCalled(); + expect(store.isConsoleShowing).toBe(true); }); - it('emits output with exitCode', async () => { - // wait for run() to get running - const runPromise = instance.run(); - await vi.waitUntil(() => store.isRunning); - expect(store.isRunning).toBe(true); + it('emits output via fiddle-runner-output and exit message via fiddle-stopped', () => { + emitEvent('run-fiddle'); - // mock fiddle gives output, - // then exits with exitCode 0 + // mock fiddle gives output, then exits with exitCode 0 emitEvent('fiddle-runner-output', 'hi'); emitEvent('fiddle-runner-output', 'hi'); emitEvent('fiddle-stopped', 0); - const result = await runPromise; - - expect(result).toBe(RunResult.SUCCESS); expect(store.isRunning).toBe(false); expect(store.pushOutput).toHaveBeenCalledTimes(3); expect(store.flushOutput).toHaveBeenCalledTimes(1); @@ -139,19 +100,12 @@ describe('Runner component', () => { ); }); - it('returns failure when app exits nonzero', async () => { + it('reports a non-zero exit code', () => { const ARBITRARY_FAIL_CODE = 50; - // wait for run() to get running - const runPromise = instance.run(); - await vi.waitUntil(() => store.isRunning); - expect(store.isRunning).toBe(true); - - // mock fiddle exits with ARBITRARY_FAIL_CODE + emitEvent('run-fiddle'); emitEvent('fiddle-stopped', ARBITRARY_FAIL_CODE); - const result = await runPromise; - expect(result).toBe(RunResult.FAILURE); expect(store.isRunning).toBe(false); expect(store.flushOutput).toHaveBeenCalledTimes(1); expect(store.pushOutput).toHaveBeenLastCalledWith( @@ -159,42 +113,14 @@ describe('Runner component', () => { ); }); - it('shows a dialog and returns invalid when the current version is missing', async () => { - store.showErrorDialog = vi.fn().mockResolvedValueOnce(true); - store.currentElectronVersion = { - version: 'test-0', - localPath: '/i/definitely/do/not/exist', - state: InstallState.installed, - source: VersionSource.local, - } as const; - - const err = `Local Electron build missing for version ${store.currentElectronVersion.version} - please verify it is in the correct location or remove and re-add it.`; - store.isVersionUsable = vi.fn().mockReturnValueOnce({ err }); - - const result = await instance.run(); - expect(result).toBe(RunResult.INVALID); - - expect(store.showErrorDialog).toHaveBeenCalledWith( - expect.stringMatching(err), - ); - }); - - it('emits output without exitCode', async () => { - // wait for run() to get running - const runPromise = instance.run(); - await vi.waitUntil(() => store.isRunning); - expect(store.isRunning).toBe(true); - + it('reports a signal exit when there is no exit code', () => { const signal = 'SIGTERM'; - // mock fiddle gives output, - // then exits without an explicit exitCode + emitEvent('run-fiddle'); emitEvent('fiddle-runner-output', 'hi'); emitEvent('fiddle-runner-output', 'hi'); emitEvent('fiddle-stopped', null, signal); - const result = await runPromise; - expect(result).toBe(RunResult.FAILURE); expect(store.isRunning).toBe(false); expect(store.flushOutput).toHaveBeenCalledTimes(1); expect(store.pushOutput).toHaveBeenCalledTimes(3); @@ -203,101 +129,161 @@ describe('Runner component', () => { ); }); - it('does not run version not yet downloaded', async () => { + it('clears the installing-modules flag when modules are installed', () => { + emitEvent('run-fiddle'); + store.isInstallingModules = true; + emitEvent('fiddle-modules-installed'); + expect(store.isInstallingModules).toBe(false); + }); + + it('does not run version not yet downloaded', () => { store.currentElectronVersion.state = InstallState.missing; - expect(await instance.run()).toBe(RunResult.INVALID); + vi.mocked(window.ElectronFiddle.getLocalVersionState).mockReturnValue( + InstallState.missing, + ); + + emitEvent('run-fiddle'); + + expect(store.isRunning).toBe(false); + expect(store.pushOutput).toHaveBeenCalledWith( + expect.stringContaining('not downloaded yet'), + expect.objectContaining({ isNotPre: true }), + ); }); - it('automatically cleans the console when enabled', async () => { - store.isClearingConsoleOnRun = true; + it('runs a local build when getLocalVersionState reports installed', () => { + store.currentElectronVersion = { + ...store.currentElectronVersion, + state: InstallState.missing, + source: VersionSource.local, + }; + vi.mocked(window.ElectronFiddle.getLocalVersionState).mockReturnValue( + InstallState.installed, + ); - setTimeout(() => emitEvent('fiddle-stopped', 0)); - const result = await instance.run(); + emitEvent('run-fiddle'); + expect(store.isRunning).toBe(true); + }); - expect(result).toBe(RunResult.SUCCESS); + it('automatically clears the console when enabled', () => { + store.isClearingConsoleOnRun = true; + emitEvent('run-fiddle'); expect(store.clearConsole).toHaveBeenCalled(); }); + + it('runs in response to the IPC event', () => { + // Confirm the event listener has been registered for 'run-fiddle'. + const calls = vi.mocked(window.ElectronFiddle.addEventListener).mock + .calls; + expect(calls.some((c) => (c as any[])[0] === 'run-fiddle')).toBe(true); + }); }); - describe('autobisect()', () => { - it('returns success if bisection succeeds', async () => { - // make sure good, bad exist in our mock - const LAST_GOOD = '2.0.1'; - const FIRST_BAD = '2.0.2'; - expect(mockVersions[LAST_GOOD]).toEqual(expect.anything()); - expect(mockVersions[FIRST_BAD]).toEqual(expect.anything()); - - const spy = vi.spyOn(store, 'setVersion'); - instance.run = vi.fn().mockImplementation(() => { - // test succeeds iff version <= LAST_GOOD - if (typeof store.version !== 'string') { - throw new Error( - 'Need to pass version string into this implementation!', - ); - } - return semver.compare(store.version, LAST_GOOD) <= 0 - ? RunResult.SUCCESS - : RunResult.FAILURE; - }); + describe('getStartFiddleOptions()', () => { + it('exposes run-time options to the main process', async () => { + store.isEnablingElectronLogging = true; + const options = await instance.getStartFiddleOptions(); + expect(options).toEqual( + expect.objectContaining({ enableElectronLogging: true }), + ); + }); - const bisectRange = [...mockVersionsArray].reverse(); - const result = await instance.autobisect(bisectRange); + it('returns options reflecting the current AppState', async () => { + store.executionFlags = ['--inspect-brk']; + (store as any).packageManager = 'npm'; + (store as any).isUsingSocketFirewall = true; + (store as any).isKeepingUserDataDirs = true; - expect(result).toBe(RunResult.SUCCESS); - expect(store.setVersion).toHaveBeenCalledTimes(2); - expect(spy).toHaveBeenNthCalledWith(1, LAST_GOOD); - expect(spy).toHaveBeenNthCalledWith(2, FIRST_BAD); - expect(store.pushOutput).toHaveBeenLastCalledWith( - `https://github.com/electron/electron/compare/v${LAST_GOOD}...v${FIRST_BAD}`, + const options = await instance.getStartFiddleOptions(); + expect(options).toEqual( + expect.objectContaining({ + version: '2.0.2', + executionFlags: ['--inspect-brk'], + packageManager: 'npm', + useSocketFirewall: true, + isKeepingUserDataDirs: true, + }), ); - - spy.mockRestore(); }); - it('returns invalid if unable to run', async () => { - const spy = vi.spyOn(store, 'setVersion'); - instance.run = vi.fn().mockImplementation(() => RunResult.INVALID); + it('marks modules as installing when there are modules to install', async () => { + store.modules = new Map([['cow', '*']]); - const bisectRange = [...mockVersionsArray].reverse(); - const result = await instance.autobisect(bisectRange); + await instance.getStartFiddleOptions(); - expect(result).toBe(RunResult.INVALID); - expect(store.pushOutput).toHaveBeenLastCalledWith( - 'Runner: autobisect Electron 2.0.1 - finished test ❓ invalid', - ); + expect(store.isInstallingModules).toBe(true); + }); + + it('does not mark modules as installing when there are no modules', async () => { + store.modules = new Map(); + + await instance.getStartFiddleOptions(); - spy.mockRestore(); + expect(store.isInstallingModules).toBeFalsy(); }); - it('returns invalid if not enough versions to bisect', async () => { - const bisectRange = [mockVersions['2.0.1']]; + it('shows a dialog and throws when the current version is unusable', async () => { + store.showErrorDialog = vi.fn().mockResolvedValueOnce(true); + store.currentElectronVersion = { + version: 'test-0', + localPath: '/i/definitely/do/not/exist', + state: InstallState.installed, + source: VersionSource.local, + } as const; - const result = await instance.autobisect(bisectRange); + const err = `Local Electron build missing for version ${store.currentElectronVersion.version} - please verify it is in the correct location or remove and re-add it.`; + store.isVersionUsable = vi.fn().mockReturnValueOnce({ err }); - expect(result).toBe(RunResult.INVALID); - expect(store.pushOutput).toHaveBeenLastCalledWith( - 'Runner: autobisect needs at least two Electron versions', + await expect(instance.getStartFiddleOptions()).rejects.toThrow( + RunResult.INVALID, + ); + + expect(store.showErrorDialog).toHaveBeenCalledWith( + expect.stringMatching(err), ); }); - async function allRunsReturn(runResult: RunResult) { - instance.run = vi.fn().mockImplementation(() => runResult); - const bisectRange = [...mockVersionsArray].reverse(); + it('rejects ESM main entry on Electron versions older than 28', async () => { + store.showErrorDialog = vi.fn().mockResolvedValueOnce(true); + store.currentElectronVersion = { + version: '27.0.0', + state: InstallState.installed, + source: VersionSource.remote, + } as const; + store.isVersionUsable = vi.fn().mockReturnValueOnce({ + ver: store.currentElectronVersion, + }); + store.editorMosaic.mainEntryPointFile = vi.fn(() => MAIN_MJS) as any; - const bisectResult = await instance.autobisect(bisectRange); + await expect(instance.getStartFiddleOptions()).rejects.toThrow( + RunResult.INVALID, + ); - expect(bisectResult).toBe(RunResult.INVALID); - expect(store.pushOutput).toHaveBeenLastCalledWith( - expect.stringMatching('both returned'), + expect(store.showErrorDialog).toHaveBeenCalledWith( + expect.stringContaining('ESM main entry points'), ); - } + }); + }); + + describe('is-auto-bisecting event', () => { + it('updates AppState.isAutoBisecting when toggled by main', () => { + emitEvent('is-auto-bisecting', true); + expect(store.isAutoBisecting).toBe(true); - it('returns invalid if a bad version cannot be found', () => { - allRunsReturn(RunResult.SUCCESS); + emitEvent('is-auto-bisecting', false); + expect(store.isAutoBisecting).toBe(false); }); + }); + + describe('onSetVersion handler', () => { + it('forwards the version to AppState.setVersion', async () => { + const calls = vi.mocked(window.ElectronFiddle.onSetVersion).mock.calls; + expect(calls).toHaveLength(1); + const callback = calls[0][0]; + + await callback('2.0.1'); - it('returns invalid if a good version cannot be found', async () => { - allRunsReturn(RunResult.FAILURE); + expect(store.setVersion).toHaveBeenCalledWith('2.0.1'); }); }); From cd3fae1a0048be1f1dec4fc9d05152d682744e50 Mon Sep 17 00:00:00 2001 From: David Sanders Date: Fri, 8 May 2026 17:22:24 -0400 Subject: [PATCH 15/22] chore: handle error case with getStartFiddleOptions --- src/main/fiddle-core.ts | 10 +++++++++- src/main/utils/get-start-fiddle-options.ts | 13 +++++++++++-- src/preload/preload.ts | 10 ++++++++-- 3 files changed, 28 insertions(+), 5 deletions(-) diff --git a/src/main/fiddle-core.ts b/src/main/fiddle-core.ts index f04358d405..78ad87d683 100644 --- a/src/main/fiddle-core.ts +++ b/src/main/fiddle-core.ts @@ -23,6 +23,7 @@ import { PACKAGE_NAME, ProgressObject, RunResult, + StartFiddleOptions, } from '../interfaces'; import { IpcEvents } from '../ipc-events'; import { maybePlural } from '../utils/plural-maybe'; @@ -111,7 +112,14 @@ export async function installModules( export async function startFiddle( webContents: WebContents, ): Promise { - const options = await getStartFiddleOptions(webContents); + let options: StartFiddleOptions; + + try { + options = await getStartFiddleOptions(webContents); + } catch { + return RunResult.INVALID; + } + const { enableElectronLogging, env: rendererEnv, diff --git a/src/main/utils/get-start-fiddle-options.ts b/src/main/utils/get-start-fiddle-options.ts index d68c2c0eba..34bd065a83 100644 --- a/src/main/utils/get-start-fiddle-options.ts +++ b/src/main/utils/get-start-fiddle-options.ts @@ -7,11 +7,13 @@ import { ipcMainManager } from '../ipc'; /** * Asks the renderer for everything the main process needs to run the * current fiddle (version, env, modules, packageManager, etc.). + * + * Rejects if the renderer throws while building the options. */ export function getStartFiddleOptions( webContents: Electron.WebContents, ): Promise { - return new Promise((resolve) => { + return new Promise((resolve, reject) => { const { port1, port2 } = new MessageChannelMain(); ipcMainManager.postMessage( IpcEvents.GET_START_FIDDLE_OPTIONS, @@ -20,8 +22,15 @@ export function getStartFiddleOptions( webContents, ); port2.once('message', (event) => { - resolve(event.data as StartFiddleOptions); port2.close(); + const data = event.data as + | { result: StartFiddleOptions } + | { error: string }; + if ('error' in data) { + reject(new Error(data.error)); + } else { + resolve(data.result); + } }); port2.start(); }); diff --git a/src/preload/preload.ts b/src/preload/preload.ts index bd3ac43112..f32dd1e44a 100644 --- a/src/preload/preload.ts +++ b/src/preload/preload.ts @@ -192,8 +192,14 @@ export async function setupFiddleGlobal() { onGetStartFiddleOptions(callback: () => Promise) { ipcRenderer.removeAllListeners(IpcEvents.GET_START_FIDDLE_OPTIONS); ipcRenderer.on(IpcEvents.GET_START_FIDDLE_OPTIONS, async (e) => { - const options = await callback(); - e.ports[0].postMessage(options); + try { + const options = await callback(); + e.ports[0].postMessage({ result: options }); + } catch (error) { + const message = + error instanceof Error ? error.message : String(error); + e.ports[0].postMessage({ error: message }); + } }); }, onSetVersion(callback: (version: string) => Promise) { From 6c4a09f8ff377cf0f83750805a10f0083298f4b8 Mon Sep 17 00:00:00 2001 From: David Sanders Date: Fri, 8 May 2026 17:45:50 -0400 Subject: [PATCH 16/22] fix: prevent simultaneous runs --- src/main/fiddle-core.ts | 46 +++++++++++++++++++++++++++++++++++++++++ src/main/menu.ts | 4 +++- 2 files changed, 49 insertions(+), 1 deletion(-) diff --git a/src/main/fiddle-core.ts b/src/main/fiddle-core.ts index 78ad87d683..06595d2380 100644 --- a/src/main/fiddle-core.ts +++ b/src/main/fiddle-core.ts @@ -6,7 +6,9 @@ import { BrowserWindow, IpcMainEvent, IpcMainInvokeEvent, + Menu, WebContents, + app, } from 'electron'; import { ELECTRON_DOWNLOAD_PATH, ELECTRON_INSTALL_PATH } from './constants'; @@ -50,9 +52,34 @@ const BLOCKED_ENV_KEYS = new Set([ // Keep track of which fiddle process belongs to which WebContents const fiddleProcesses = new WeakMap(); +// Keep track of active runs to prevent double runs +const activeRuns = new WeakMap>(); + const downloadingVersions = new Map>(); const removingVersions = new Map>(); +/** + * Whether the focused window's "Run Fiddle..." menu item should be + * enabled. False if the focused window already has a fiddle running. + */ +export function isRunFiddleEnabled(): boolean { + const focused = BrowserWindow.getFocusedWindow(); + return !focused || !activeRuns.has(focused.webContents); +} + +/** + * Update the enabled state of the "Run Fiddle..." menu item to reflect + * whether the focused window already has a fiddle running. Used to + * refresh the existing menu item between full menu rebuilds. + */ +export function updateRunFiddleMenuItem(): void { + const menu = Menu.getApplicationMenu(); + const item = menu?.getMenuItemById('run-fiddle'); + if (!item) return; + + item.enabled = isRunFiddleEnabled(); +} + /** * Installs the specified modules */ @@ -112,6 +139,21 @@ export async function installModules( export async function startFiddle( webContents: WebContents, ): Promise { + // Ignore concurrent run attempts from the same WebContents. + if (!activeRuns.has(webContents)) { + activeRuns.set(webContents, startFiddleImpl(webContents)); + updateRunFiddleMenuItem(); + } + + try { + return await activeRuns.get(webContents)!; + } finally { + activeRuns.delete(webContents); + updateRunFiddleMenuItem(); + } +} + +async function startFiddleImpl(webContents: WebContents): Promise { let options: StartFiddleOptions; try { @@ -301,6 +343,10 @@ export async function setupFiddleCore(versions: ElectronVersions) { runner = await Runner.create({ installer, versions }); + // Refresh the "Run Fiddle..." menu item when window focus changes so it + // reflects the focused window's run state. + app.on('browser-window-focus', updateRunFiddleMenuItem); + ipcMainManager.on( IpcEvents.GET_VERSION_STATE, (event: IpcMainEvent, version: string) => { diff --git a/src/main/menu.ts b/src/main/menu.ts index 27371e1115..cfd4241f17 100644 --- a/src/main/menu.ts +++ b/src/main/menu.ts @@ -6,7 +6,7 @@ import { shell, } from 'electron'; -import { startFiddle } from './fiddle-core'; +import { isRunFiddleEnabled, startFiddle } from './fiddle-core'; import { saveFiddle, saveFiddleAs, @@ -316,8 +316,10 @@ function getQuitItems(): Array { function getTasksMenu(): MenuItemConstructorOptions { const tasksMenu: Array = [ { + id: 'run-fiddle', label: 'Run Fiddle...', accelerator: 'F5', + enabled: isRunFiddleEnabled(), click: (_, focusedWindow) => { if (focusedWindow) startFiddle((focusedWindow as BrowserWindow).webContents).catch( From 06f434e259ed2ea462ebdf63aabae9ad0fedc0a2 Mon Sep 17 00:00:00 2001 From: David Sanders Date: Fri, 8 May 2026 19:02:07 -0400 Subject: [PATCH 17/22] chore: dead code cleanup --- src/main/fiddle-core.ts | 8 +------- tests/mocks/runner.ts | 8 +------- 2 files changed, 2 insertions(+), 14 deletions(-) diff --git a/src/main/fiddle-core.ts b/src/main/fiddle-core.ts index 06595d2380..66e6fd2a9f 100644 --- a/src/main/fiddle-core.ts +++ b/src/main/fiddle-core.ts @@ -21,8 +21,8 @@ import { pushError, pushOutput, pushOutputLine } from './utils/push-output'; import { getLocalVersions } from './versions'; import { DownloadVersionParams, - IPackageManager, PACKAGE_NAME, + PMOperationOptions, ProgressObject, RunResult, StartFiddleOptions, @@ -30,12 +30,6 @@ import { import { IpcEvents } from '../ipc-events'; import { maybePlural } from '../utils/plural-maybe'; -export interface PMOperationOptions { - dir: string; - packageManager: IPackageManager; - useSocketFirewall?: boolean; -} - let installer: Installer; let runner: Runner; diff --git a/tests/mocks/runner.ts b/tests/mocks/runner.ts index 1c4b2612c7..36c345a0ce 100644 --- a/tests/mocks/runner.ts +++ b/tests/mocks/runner.ts @@ -1,7 +1 @@ -import { vi } from 'vitest'; - -export class RunnerMock { - public autobisect = vi.fn(); - public run = vi.fn(); - public stop = vi.fn(); -} +export class RunnerMock {} From 433cba9cb6f5916bc7c8ff6177b7cf89223e48ba Mon Sep 17 00:00:00 2001 From: David Sanders Date: Fri, 8 May 2026 19:05:19 -0400 Subject: [PATCH 18/22] fix: restore security check in deleteUserData --- src/main/files.ts | 4 ++++ tests/main/files.spec.ts | 46 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 50 insertions(+) diff --git a/src/main/files.ts b/src/main/files.ts index c141bc94cf..020b96574b 100644 --- a/src/main/files.ts +++ b/src/main/files.ts @@ -144,6 +144,10 @@ export async function cleanupDirectory(dir?: string): Promise { } export async function deleteUserData(name: string) { + if (!isSafeDataName(name)) { + console.warn(`deleteUserData: rejected unsafe name: ${name}`); + return; + } const appData = path.join(app.getPath('appData'), name); console.log(`Cleanup: Deleting data dir ${appData}`); await cleanupDirectory(appData); diff --git a/tests/main/files.spec.ts b/tests/main/files.spec.ts index f464e5e712..a57dc941ee 100644 --- a/tests/main/files.spec.ts +++ b/tests/main/files.spec.ts @@ -10,6 +10,7 @@ import { MAIN_JS } from '../../src/interfaces'; import { IpcEvents } from '../../src/ipc-events'; import { cleanupDirectory, + deleteUserData, saveFiddle, saveFiddleAs, saveFiddleAsForgeProject, @@ -206,6 +207,51 @@ describe('files', () => { }); }); + describe('deleteUserData()', () => { + beforeEach(() => { + vi.mocked(app.getPath).mockReturnValue('/fake/appData'); + }); + + it('removes the user-data directory for a safe name', async () => { + vi.mocked(fs.existsSync).mockReturnValueOnce(true); + + await deleteUserData('my-fiddle'); + + expect(fs.remove).toHaveBeenCalledWith('/fake/appData/my-fiddle'); + }); + + it.each([ + ['parent-traversal segment', '../../etc'], + ['absolute path', '/etc/passwd'], + ['empty string', ''], + ['nested path segment', 'a/b'], + ])('rejects an unsafe name (%s)', async (_label, name) => { + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + + await deleteUserData(name); + + expect(fs.remove).not.toHaveBeenCalled(); + expect(warn).toHaveBeenCalledWith( + expect.stringContaining('rejected unsafe name'), + ); + + warn.mockRestore(); + }); + + it('rejects a non-string name', async () => { + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + + await deleteUserData(undefined as unknown as string); + + expect(fs.remove).not.toHaveBeenCalled(); + expect(warn).toHaveBeenCalledWith( + expect.stringContaining('rejected unsafe name'), + ); + + warn.mockRestore(); + }); + }); + describe('saveFiles()', () => { it('saves all non-empty files in Fiddle', async () => { const values = { ...editorValues }; From 057336fc03e9af2030fa35820bf4b20735d27419 Mon Sep 17 00:00:00 2001 From: David Sanders Date: Fri, 8 May 2026 19:19:17 -0400 Subject: [PATCH 19/22] fix: send fiddle stopped event on error --- src/main/fiddle-core.ts | 2 ++ src/renderer/runner.ts | 6 ++++-- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/src/main/fiddle-core.ts b/src/main/fiddle-core.ts index 66e6fd2a9f..9ce075c10a 100644 --- a/src/main/fiddle-core.ts +++ b/src/main/fiddle-core.ts @@ -226,6 +226,7 @@ async function startFiddleImpl(webContents: WebContents): Promise { pushError(webContents, 'Could not install modules', error); await cleanup(); + ipcMainManager.send(IpcEvents.FIDDLE_STOPPED, [null, null], webContents); return RunResult.FAILURE; } @@ -262,6 +263,7 @@ async function startFiddleImpl(webContents: WebContents): Promise { } catch (error: any) { pushError(webContents, 'Failed to spawn Fiddle', error); await cleanup(); + ipcMainManager.send(IpcEvents.FIDDLE_STOPPED, [null, null], webContents); return RunResult.FAILURE; } fiddleProcesses.set(webContents, child); diff --git a/src/renderer/runner.ts b/src/renderer/runner.ts index ba0cdfca29..c8cabc4528 100644 --- a/src/renderer/runner.ts +++ b/src/renderer/runner.ts @@ -189,10 +189,12 @@ export class Runner { window.ElectronFiddle.addEventListener('fiddle-stopped', (code, signal) => { cleanup(); - if (typeof code !== 'number') { + if (typeof code !== 'number' && typeof signal === 'string') { pushOutput(`Electron exited with signal ${signal}.`); - } else { + } else if (typeof code === 'number') { pushOutput(`Electron exited with code ${code}.`); + } else { + pushOutput('Electron exited.'); } }); From 197c21ec15831e661bf28145a2a04a71d2808b98 Mon Sep 17 00:00:00 2001 From: David Sanders Date: Fri, 8 May 2026 21:11:07 -0400 Subject: [PATCH 20/22] fix: restore getFiles options --- src/main/fiddle-core.ts | 7 ++++++- src/main/utils/get-files.ts | 9 +++++++-- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/src/main/fiddle-core.ts b/src/main/fiddle-core.ts index 9ce075c10a..012ea80ced 100644 --- a/src/main/fiddle-core.ts +++ b/src/main/fiddle-core.ts @@ -182,7 +182,12 @@ async function startFiddleImpl(webContents: WebContents): Promise { // Get the fiddle's files from the renderer. const files = new Map( - (await getFiles(BrowserWindow.fromWebContents(webContents)!, [])).files, + ( + await getFiles(BrowserWindow.fromWebContents(webContents)!, [], { + includeDependencies: false, + includeElectron: false, + }) + ).files, ); // Pull the project name out of the fiddle's package.json — that's the diff --git a/src/main/utils/get-files.ts b/src/main/utils/get-files.ts index 58067db555..5121ad1c10 100644 --- a/src/main/utils/get-files.ts +++ b/src/main/utils/get-files.ts @@ -1,6 +1,10 @@ import { MessageChannelMain } from 'electron'; -import { FileTransformOperation, Files } from '../../interfaces'; +import { + FileTransformOperation, + Files, + PackageJsonOptions, +} from '../../interfaces'; import { IpcEvents } from '../../ipc-events'; import { ipcMainManager } from '../ipc'; @@ -10,12 +14,13 @@ import { ipcMainManager } from '../ipc'; export function getFiles( window: Electron.BrowserWindow, transforms: Array, + options?: PackageJsonOptions, ): Promise<{ localPath?: string; files: Files }> { return new Promise((resolve) => { const { port1, port2 } = new MessageChannelMain(); ipcMainManager.postMessage( IpcEvents.GET_FILES, - { options: undefined, transforms }, + { options, transforms }, [port1], window.webContents, ); From 3304486c099a3e5e7cb781554b73a7f768a5e44e Mon Sep 17 00:00:00 2001 From: David Sanders Date: Tue, 12 May 2026 15:58:04 -0700 Subject: [PATCH 21/22] fix: start fiddle directly from context menu --- src/main/context-menu.ts | 9 ++++++++- tests/main/context-menu.spec.ts | 23 +++++++++++++++++++++-- 2 files changed, 29 insertions(+), 3 deletions(-) diff --git a/src/main/context-menu.ts b/src/main/context-menu.ts index 5d89b28a1f..909378556b 100644 --- a/src/main/context-menu.ts +++ b/src/main/context-menu.ts @@ -5,6 +5,7 @@ import { MenuItemConstructorOptions, } from 'electron'; +import { isRunFiddleEnabled, startFiddle } from './fiddle-core'; import { ipcMainManager } from './ipc'; import { isDevMode } from './utils/devmode'; import { IpcEvents } from '../ipc-events'; @@ -17,7 +18,13 @@ export function getRunItems(): Array { { id: 'run', label: 'Run Fiddle', - click: () => ipcMainManager.send(IpcEvents.FIDDLE_RUN), + enabled: isRunFiddleEnabled(), + click: (_, focusedWindow) => { + if (focusedWindow) + startFiddle((focusedWindow as BrowserWindow).webContents).catch( + console.error, + ); + }, }, { id: 'clear_console', diff --git a/tests/main/context-menu.spec.ts b/tests/main/context-menu.spec.ts index 8ceaedd2a2..710898aa52 100644 --- a/tests/main/context-menu.spec.ts +++ b/tests/main/context-menu.spec.ts @@ -2,6 +2,7 @@ * @vitest-environment node */ +import * as electron from 'electron'; import { BrowserWindow, ContextMenuParams, Menu } from 'electron'; import { beforeEach, describe, expect, it, vi } from 'vitest'; @@ -11,6 +12,7 @@ import { getMonacoItems, getRunItems, } from '../../src/main/context-menu'; +import { startFiddle } from '../../src/main/fiddle-core'; import { ipcMainManager } from '../../src/main/ipc'; import { isDevMode } from '../../src/main/utils/devmode'; import { BrowserWindowMock } from '../mocks/browser-window'; @@ -18,6 +20,7 @@ import { WebContentsMock } from '../mocks/web-contents'; vi.mock('../../src/main/utils/devmode'); vi.mock('../../src/main/ipc'); +vi.mock('../../src/main/fiddle-core'); describe('context-menu', () => { let mockWindow: BrowserWindow; @@ -225,9 +228,25 @@ describe('context-menu', () => { }); describe('getRunItems()', () => { - it('executes an IPC send() on click', () => { + it('starts the fiddle on click', () => { + vi.mocked(startFiddle).mockResolvedValueOnce(undefined as any); const result = getRunItems(); - (result[0] as any).click(); + const mocks = { + send: vi.fn(), + }; + const focusedWindow = { + webContents: + mocks as Partial as electron.WebContents, + } as Partial as electron.BrowserWindow; + expect(result[0].id).toBe('run'); + result[0].click?.({} as any, focusedWindow, {} as any); + expect(startFiddle).toHaveBeenCalledWith(focusedWindow.webContents); + }); + + it('executes an IPC send() on clear console click', () => { + const result = getRunItems(); + expect(result[1].id).toBe('clear_console'); + result[1].click?.({} as any, {} as any, {} as any); expect(ipcMainManager.send).toHaveBeenCalledTimes(1); }); }); From ee4e03cbc7b7d0329175a086c2b786f1855237ea Mon Sep 17 00:00:00 2001 From: David Sanders Date: Tue, 12 May 2026 18:57:59 -0700 Subject: [PATCH 22/22] test: fix path checking on Windows --- tests/main/files.spec.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/tests/main/files.spec.ts b/tests/main/files.spec.ts index a57dc941ee..76957552e2 100644 --- a/tests/main/files.spec.ts +++ b/tests/main/files.spec.ts @@ -2,6 +2,8 @@ * @vitest-environment node */ +import path from 'node:path'; + import { BrowserWindow, app, dialog } from 'electron'; import fs from 'fs-extra'; import { type Mock, beforeEach, describe, expect, it, vi } from 'vitest'; @@ -217,7 +219,9 @@ describe('files', () => { await deleteUserData('my-fiddle'); - expect(fs.remove).toHaveBeenCalledWith('/fake/appData/my-fiddle'); + expect(fs.remove).toHaveBeenCalledWith( + path.normalize('/fake/appData/my-fiddle'), + ); }); it.each([