diff --git a/packages/api/cli/src/electron-forge-start.ts b/packages/api/cli/src/electron-forge-start.ts index 6dd8cf657c..e8d3c0a848 100644 --- a/packages/api/cli/src/electron-forge-start.ts +++ b/packages/api/cli/src/electron-forge-start.ts @@ -93,7 +93,9 @@ import packageJSON from '../package.json' with { type: 'json' }; }; onExit = (code: number) => { removeListeners(); - if (spawned.restarted) return; + // `child`, not `spawned`: the first child's `restarted` flag stays true + // forever, so using it would ignore every exit after the first restart. + if (child.restarted) return; if (code !== 0) { process.exit(code); } diff --git a/packages/api/core/spec/fast/start.spec.ts b/packages/api/core/spec/fast/start.spec.ts index 2096eb8947..97cd2418c2 100644 --- a/packages/api/core/spec/fast/start.spec.ts +++ b/packages/api/core/spec/fast/start.spec.ts @@ -1,9 +1,12 @@ import { ChildProcess, spawn } from 'node:child_process'; +import { EventEmitter } from 'node:events'; +import { requestAppRestart } from '@electron-forge/core-utils/restart'; import { ElectronProcess } from '@electron-forge/shared-types'; -import { describe, expect, it, vi } from 'vitest'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; import start from '../../src/api/start'; +import locateElectronExecutable from '../../src/util/electron-executable.js'; import findConfig from '../../src/util/forge-config.js'; import { readMutatedPackageJson } from '../../src/util/read-package-json.js'; import resolveDir from '../../src/util/resolve-dir.js'; @@ -243,6 +246,127 @@ describe('start', () => { ).rejects.toThrowError("Please set your application's 'version' in"); }); + describe('restarting', () => { + // A stand-in for a spawned Electron process that dies when killed. + const fakeChild = () => { + const child = new EventEmitter() as ElectronProcess & { + kill: ReturnType; + }; + child.kill = vi.fn(() => { + child.emit('exit'); + child.emit('close'); + return true; + }); + return child; + }; + + const spawnsInOrder = (...children: ReturnType[]) => { + const mock = vi.mocked(spawn); + for (const child of children) mock.mockReturnValueOnce(child); + return children; + }; + + beforeEach(() => { + // Silence the "Restarting Electron app" line. + vi.spyOn(console, 'info').mockImplementation(() => undefined); + }); + + it('kills the running app and hands back its replacement', async () => { + const [first, second] = spawnsInOrder(fakeChild(), fakeChild()); + + const spawned = await start({ + dir: import.meta.dirname, + interactive: false, + }); + expect(spawned).toBe(first); + + const replaced = new Promise((resolve) => + spawned.on('restarted', resolve), + ); + + expect(requestAppRestart()).toBe(true); + expect(first.restarted).toBe(true); + expect(first.kill).toHaveBeenCalledWith('SIGTERM'); + + // The event has to fire on the *exiting* child: that is what re-attaches + // the CLI's exit handling to the replacement. + await expect(replaced).resolves.toBe(second); + expect(vi.mocked(spawn)).toHaveBeenCalledTimes(2); + }); + + it('initializes `restarted` instead of leaving it undefined', async () => { + spawnsInOrder(fakeChild()); + + const spawned = await start({ + dir: import.meta.dirname, + interactive: false, + }); + + expect(spawned.restarted).toBe(false); + }); + + it('declines to restart when no app is running', async () => { + // `spawn` is mocked with no return value, so nothing is ever running. + await start({ dir: import.meta.dirname, interactive: false }); + + expect(requestAppRestart()).toBe(false); + }); + + it('still restarts once more when asked mid-restart', async () => { + spawnsInOrder(fakeChild(), fakeChild(), fakeChild()); + + await start({ dir: import.meta.dirname, interactive: false }); + + expect(requestAppRestart()).toBe(true); + // A request landing mid-restart must not be dropped, or the app keeps + // running the code that was just replaced. + expect(requestAppRestart()).toBe(true); + + await vi.waitFor(() => expect(vi.mocked(spawn)).toHaveBeenCalledTimes(3)); + }); + + it('keeps the replacement app when the old one closes late', async () => { + const [first, second] = spawnsInOrder(fakeChild(), fakeChild()); + first.kill = vi.fn(() => { + first.emit('exit'); + return true; + }); + + const spawned = await start({ + dir: import.meta.dirname, + interactive: false, + }); + const replaced = new Promise((resolve) => + spawned.on('restarted', resolve), + ); + requestAppRestart(); + await replaced; + + // A late `close` from the old child must not clear the live replacement. + first.emit('close'); + + expect(requestAppRestart()).toBe(true); + expect(second.kill).toHaveBeenCalledWith('SIGTERM'); + }); + + it('reports a failed relaunch rather than rejecting unobserved', async () => { + spawnsInOrder(fakeChild()); + const consoleError = vi + .spyOn(console, 'error') + .mockImplementation(() => undefined); + + await start({ dir: import.meta.dirname, interactive: false }); + + vi.mocked(locateElectronExecutable).mockRejectedValueOnce( + new Error('electron is gone'), + ); + requestAppRestart(); + + // An unhandled rejection here would take down the whole Forge process. + await vi.waitFor(() => expect(consoleError).toHaveBeenCalled()); + }); + }); + // TODO(erickzhao): improve test coverage it.todo( 'allows plugin to override the start command with a custom spawn string', diff --git a/packages/api/core/src/api/start.ts b/packages/api/core/src/api/start.ts index 8be3804af4..040fccc414 100644 --- a/packages/api/core/src/api/start.ts +++ b/packages/api/core/src/api/start.ts @@ -6,6 +6,10 @@ import { getElectronVersion, listrCompatibleRebuildHook, } from '@electron-forge/core-utils'; +import { + requestAppRestart, + setAppRestartHandler, +} from '@electron-forge/core-utils/restart'; import { ElectronProcess, ForgeArch, @@ -265,6 +269,10 @@ export default autoTrace( const spawned = await forgeSpawn(); // When the child app is closed we should stop listening for stdin if (spawned) { + // `restarted` is non-optional on `ElectronProcess`, so don't leave it + // `undefined` until the first restart. + spawned.restarted = false; + if (interactive && process.stdin.isPaused()) { process.stdin.resume(); } @@ -278,9 +286,12 @@ export default autoTrace( } }); - // On close, reset lastSpawned, it's dead + // On close, reset lastSpawned, it's dead. A restart may already have put + // a replacement there, so only clear our own child. spawned.on('close', () => { - lastSpawned = null; + if (lastSpawned === spawned) { + lastSpawned = null; + } }); } else if (interactive && !process.stdin.isPaused()) { process.stdin.pause(); @@ -290,25 +301,76 @@ export default autoTrace( return lastSpawned; }; + // A restart spans kill -> exit -> respawn, during which `lastSpawned` is + // briefly null. Track that window so a request landing in it gets queued + // rather than mistaken for "there is nothing to restart". + let restartInFlight = false; + let restartPending = false; + + const restartRunningApp = (): boolean => { + if (restartInFlight) { + d('a restart is already in flight, queueing a follow-up restart'); + restartPending = true; + return true; + } + + if (!lastSpawned || lastSpawned.restarted) { + d('restart requested, but no Electron app is running'); + return false; + } + + const dying = lastSpawned; + restartInFlight = true; + console.info( + `${styleText('green', '✔ ')}${styleText('dim', 'Restarting Electron app')}`, + ); + dying.restarted = true; + dying.on('exit', () => { + forgeSpawnWrapper().then( + (child) => { + restartInFlight = false; + // Emit on the *exiting* child: its `restarted` listeners are what + // re-attach the CLI's exit handling to the replacement. + dying.emit('restarted', child); + + if (restartPending) { + restartPending = false; + restartRunningApp(); + } + }, + (err) => { + restartInFlight = false; + restartPending = false; + console.error( + styleText( + 'red', + 'Failed to relaunch the Electron app after a restart, so it is no longer running.', + ), + ); + console.error(err); + }, + ); + }); + dying.kill('SIGTERM'); + return true; + }; + + setAppRestartHandler(restartRunningApp); + if (interactive) { process.stdin.on('data', (data) => { - if ( - data.toString().trim() === 'rs' && - lastSpawned && - !lastSpawned.restarted - ) { + if (data.toString().trim() !== 'rs') return; + + // Erase the echoed `rs` only when the "Restarting Electron app" line is + // about to take its place; otherwise we would eat a line of the app's + // own output. + if (lastSpawned && !lastSpawned.restarted) { readline.moveCursor(process.stdout, 0, -1); readline.clearLine(process.stdout, 0); readline.cursorTo(process.stdout, 0); - console.info( - `${styleText('green', '✔ ')}${styleText('dim', 'Restarting Electron app')}`, - ); - lastSpawned.restarted = true; - lastSpawned.on('exit', async () => { - lastSpawned!.emit('restarted', await forgeSpawnWrapper()); - }); - lastSpawned.kill('SIGTERM'); } + + requestAppRestart(); }); process.stdin.resume(); diff --git a/packages/plugin/vite/README.md b/packages/plugin/vite/README.md index b0eba8c88a..7bc181ebe7 100644 --- a/packages/plugin/vite/README.md +++ b/packages/plugin/vite/README.md @@ -36,3 +36,35 @@ module.exports = { ] }; ``` + +### Main process hot restart + +Renderer code is hot-reloaded by Vite out of the box, but main process code is +not: the app has to be relaunched to pick it up. Set `hotRestart` to have +`electron-forge start` do that for you whenever a main process bundle rebuilds: + +```javascript +// forge.config.js + +module.exports = { + plugins: [ + { + name: '@electron-forge/plugin-vite', + config: { + hotRestart: true, + build: [ + { + entry: 'src/main.js', + config: 'vite.main.config.mjs' + } + ], + renderer: [] + } + } + ] +}; +``` + +This is off by default, so main process changes otherwise only take effect when +you type `rs` in the terminal or restart `electron-forge start`. The option has +no effect when packaging. diff --git a/packages/plugin/vite/spec/ViteConfig.spec.ts b/packages/plugin/vite/spec/ViteConfig.spec.ts index 938788f40d..3e3a6925ab 100644 --- a/packages/plugin/vite/spec/ViteConfig.spec.ts +++ b/packages/plugin/vite/spec/ViteConfig.spec.ts @@ -46,9 +46,8 @@ describe('ViteConfigGenerator', () => { 'electron/main', ]); expect(buildConfig.clearScreen).toBe(false); - expect( - buildConfig.plugins?.map((plugin) => (plugin as Plugin).name), - ).toEqual(['@electron-forge/plugin-vite:hot-restart']); + // Hot restart is opt-in, so the main config carries no plugins by default. + expect(buildConfig.plugins).toEqual([]); expect(buildConfig.define).toEqual({}); expect(buildConfig.resolve).toEqual({ conditions: ['node'], @@ -56,6 +55,26 @@ describe('ViteConfigGenerator', () => { }); }); + it('getBuildConfigs:main adds the hot restart plugin when hotRestart is enabled', async () => { + const forgeConfig: VitePluginConfig = { + build: [ + { + entry: 'src/main.js', + config: path.join(configRoot, 'vite.main.config.mjs'), + target: 'main', + }, + ], + renderer: [], + hotRestart: true, + }; + const generator = new ViteConfigGenerator(forgeConfig, configRoot, true); + const buildConfig = (await generator.getBuildConfigs())[0]; + + expect( + buildConfig.plugins?.map((plugin) => (plugin as Plugin).name), + ).toEqual(['@electron-forge/plugin-vite:hot-restart']); + }); + it('getBuildConfigs:preload', async () => { const forgeConfig: VitePluginConfig = { build: [ @@ -89,9 +108,10 @@ describe('ViteConfigGenerator', () => { assetFileNames: '[name].[ext]', }); expect(buildConfig.clearScreen).toBe(false); + // Preload scripts are reloaded, never restarted, regardless of `hotRestart`. expect( buildConfig.plugins?.map((plugin) => (plugin as Plugin).name), - ).toEqual(['@electron-forge/plugin-vite:hot-restart']); + ).toEqual(['@electron-forge/plugin-vite:hot-reload']); }); it('getRendererConfig:renderer', async () => { diff --git a/packages/plugin/vite/spec/config/vite.base.config.spec.ts b/packages/plugin/vite/spec/config/vite.base.config.spec.ts index 436b9f86bd..eecfc4e255 100644 --- a/packages/plugin/vite/spec/config/vite.base.config.spec.ts +++ b/packages/plugin/vite/spec/config/vite.base.config.spec.ts @@ -1,12 +1,14 @@ import path from 'node:path'; +import { setAppRestartHandler } from '@electron-forge/core-utils/restart'; import { createServer } from 'vite'; -import { describe, expect, it } from 'vitest'; +import { afterEach, describe, expect, it, vi } from 'vitest'; import { getBuildDefine, getDefineKeys, pluginExposeRenderer, + pluginHotRestart, } from '../../src/config/vite.base.config'; import type { VitePluginConfig } from '../../src/Config'; @@ -110,4 +112,87 @@ describe('vite.base.config', () => { expect(define1).toEqual(define2); }); + + describe('pluginHotRestart', () => { + let dispose: (() => void) | undefined; + + // `closeBundle` receives the build error rollup is about to rethrow, if any. + const closeBundle = (plugin: ReturnType) => + plugin.closeBundle as (error?: Error) => void; + + afterEach(() => { + dispose?.(); + dispose = undefined; + }); + + const handleRestarts = (accepted = true) => { + const handler = vi.fn(() => accepted); + dispose = setAppRestartHandler(handler); + return handler; + }; + + it('names the plugin after its mode', () => { + expect(pluginHotRestart('restart').name).toEqual( + '@electron-forge/plugin-vite:hot-restart', + ); + expect(pluginHotRestart('reload').name).toEqual( + '@electron-forge/plugin-vite:hot-reload', + ); + }); + + it('requests a restart once the bundle closes', () => { + const handler = handleRestarts(); + + closeBundle(pluginHotRestart('restart'))(); + + expect(handler).toHaveBeenCalledOnce(); + }); + + it('does not request a restart when the build failed', () => { + const handler = handleRestarts(); + + // The bundle on disk is stale, so restarting would run the previous code. + closeBundle(pluginHotRestart('restart'))(new Error('syntax error')); + + expect(handler).not.toHaveBeenCalled(); + }); + + it('never requests a restart in reload mode', () => { + const handler = handleRestarts(); + + // Preload rebuilds reload the renderers; they must not restart the app. + closeBundle(pluginHotRestart('reload'))(); + + expect(handler).not.toHaveBeenCalled(); + }); + + it('warns when a rebuild fails to reach the running app', () => { + const consoleWarn = vi + .spyOn(console, 'warn') + .mockImplementation(() => undefined); + handleRestarts(false); + const plugin = pluginHotRestart('restart'); + + // The first build runs before the app is spawned, so an unhonored request + // is expected there. + closeBundle(plugin)(); + expect(consoleWarn).not.toHaveBeenCalled(); + + closeBundle(plugin)(); + expect(consoleWarn).toHaveBeenCalledOnce(); + }); + + it('stays quiet when a rebuild does reach the running app', () => { + const consoleWarn = vi + .spyOn(console, 'warn') + .mockImplementation(() => undefined); + handleRestarts(true); + const plugin = pluginHotRestart('restart'); + + closeBundle(plugin)(); + closeBundle(plugin)(); + + expect(consoleWarn).not.toHaveBeenCalled(); + }); + }); }); diff --git a/packages/plugin/vite/src/Config.ts b/packages/plugin/vite/src/Config.ts index b7cd6847be..43b3910178 100644 --- a/packages/plugin/vite/src/Config.ts +++ b/packages/plugin/vite/src/Config.ts @@ -49,4 +49,11 @@ export interface VitePluginConfig { * @defaultValue `true` */ concurrent?: boolean | number; + + /** + * Restart the running app whenever the main process bundle is rebuilt during + * `electron-forge start`. Has no effect when packaging. + * @defaultValue `false` + */ + hotRestart?: boolean; } diff --git a/packages/plugin/vite/src/VitePlugin.ts b/packages/plugin/vite/src/VitePlugin.ts index 7219b25051..52730783c4 100644 --- a/packages/plugin/vite/src/VitePlugin.ts +++ b/packages/plugin/vite/src/VitePlugin.ts @@ -336,7 +336,8 @@ the generated files). Instead, it is ${JSON.stringify(pj.main)}.`); /** * Serializable snapshot of the plugin config to pass to subprocess workers. * We only include build[] and renderer[] — the worker needs the full renderer - * list for defines even when building a single main target. + * list for defines even when building a single main target. `hotRestart` is + * moot here: workers only run when packaging. */ private get serializableConfig(): Pick< VitePluginConfig, diff --git a/packages/plugin/vite/src/config/vite.base.config.ts b/packages/plugin/vite/src/config/vite.base.config.ts index e946a2a69b..9551db1484 100644 --- a/packages/plugin/vite/src/config/vite.base.config.ts +++ b/packages/plugin/vite/src/config/vite.base.config.ts @@ -1,4 +1,7 @@ import { builtinModules } from 'node:module'; +import { styleText } from 'node:util'; + +import { requestAppRestart } from '@electron-forge/core-utils/restart'; import type { AddressInfo } from 'node:net'; import type { ConfigEnv, Plugin, UserConfig, ViteDevServer } from 'vite'; @@ -92,19 +95,33 @@ export function pluginExposeRenderer(name: string): Plugin { } export function pluginHotRestart(command: 'reload' | 'restart'): Plugin { + let builtOnce = false; + return { - name: '@electron-forge/plugin-vite:hot-restart', - closeBundle() { + name: `@electron-forge/plugin-vite:hot-${command}`, + closeBundle(error) { + const isRebuild = builtOnce; + builtOnce = true; + + // Rollup passes the build error here before rethrowing it. The bundle on + // disk is stale in that case, so reloading or restarting would silently + // run the previous build's code. + if (error) return; + if (command === 'reload') { for (const server of Object.values(viteDevServers)) { // Preload scripts hot reload. server.ws.send({ type: 'full-reload' }); } - } else if (command === 'restart') { - // Main process hot restart. - // https://github.com/electron/forge/blob/v7.2.0/packages/api/core/src/api/start.ts#L216-L223 - // TODO: blocked in #3380 - // process.stdin.emit('data', 'rs'); + } else if (command === 'restart' && !requestAppRestart() && isRebuild) { + // The first build finishes before the app is spawned, so only a rebuild + // that fails to reach it is worth warning about. + console.warn( + styleText( + 'yellow', + '[@electron-forge/plugin-vite] Rebuilt the main process bundle, but the running app was not restarted, so it is still running the previous code.', + ), + ); } }, }; diff --git a/packages/plugin/vite/src/config/vite.main.config.ts b/packages/plugin/vite/src/config/vite.main.config.ts index ab6d1bc2d1..385d918faa 100644 --- a/packages/plugin/vite/src/config/vite.main.config.ts +++ b/packages/plugin/vite/src/config/vite.main.config.ts @@ -20,7 +20,9 @@ export function getConfig( external: [...external, 'electron/main'], }, }, - plugins: [pluginHotRestart('restart')], + plugins: [ + ...(forgeEnv.forgeConfig.hotRestart ? [pluginHotRestart('restart')] : []), + ], define, resolve: { // Load the Node.js entry. diff --git a/packages/utils/core-utils/package.json b/packages/utils/core-utils/package.json index 1272409d8b..477736bc15 100644 --- a/packages/utils/core-utils/package.json +++ b/packages/utils/core-utils/package.json @@ -6,7 +6,10 @@ "repository": "https://github.com/electron/forge", "author": "Samuel Attard", "license": "MIT", - "exports": "./dist/index.js", + "exports": { + ".": "./dist/index.js", + "./restart": "./dist/restart.js" + }, "typings": "dist/index.d.ts", "dependencies": { "@electron-forge/shared-types": "workspace:*", diff --git a/packages/utils/core-utils/spec/restart.spec.ts b/packages/utils/core-utils/spec/restart.spec.ts new file mode 100644 index 0000000000..9679c79089 --- /dev/null +++ b/packages/utils/core-utils/spec/restart.spec.ts @@ -0,0 +1,91 @@ +import { afterEach, describe, expect, it, vi } from 'vitest'; + +import { requestAppRestart, setAppRestartHandler } from '../src/restart'; + +describe('requestAppRestart', () => { + // The handler slot is process-wide, so every test has to give it back. + const disposers: Array<() => void> = []; + + const register = (handler: () => boolean) => { + const dispose = setAppRestartHandler(handler); + disposers.push(dispose); + return dispose; + }; + + afterEach(() => { + while (disposers.length) disposers.pop()!(); + }); + + it('reports failure when no handler is registered', () => { + expect(requestAppRestart()).toBe(false); + }); + + it('delegates to the registered handler on every request', () => { + const handler = vi.fn(() => true); + register(handler); + + expect(requestAppRestart()).toBe(true); + expect(requestAppRestart()).toBe(true); + // Guards against `once` semantics: Vite rebuilds request a restart every time. + expect(handler).toHaveBeenCalledTimes(2); + }); + + it('passes on a handler that could not honor the request', () => { + register(() => false); + + expect(requestAppRestart()).toBe(false); + }); + + it('replaces the previously registered handler', () => { + const stale = vi.fn(() => true); + const current = vi.fn(() => true); + register(stale); + register(current); + + requestAppRestart(); + + expect(stale).not.toHaveBeenCalled(); + expect(current).toHaveBeenCalledOnce(); + }); + + it('reports a throwing handler as a failed restart rather than propagating', () => { + const consoleError = vi + .spyOn(console, 'error') + .mockImplementation(() => undefined); + register(() => { + throw new Error('kill failed'); + }); + + // Callers are bundler hooks; a throw here would be reported to the user as + // a build failure. + expect(() => requestAppRestart()).not.toThrow(); + expect(requestAppRestart()).toBe(false); + expect(consoleError).toHaveBeenCalled(); + }); +}); + +describe('setAppRestartHandler', () => { + it('returns a disposer that unregisters the handler', () => { + const handler = vi.fn(() => true); + const dispose = setAppRestartHandler(handler); + + dispose(); + + expect(requestAppRestart()).toBe(false); + expect(handler).not.toHaveBeenCalled(); + }); + + it('does not let a stale disposer unregister a newer handler', () => { + const stale = vi.fn(() => true); + const current = vi.fn(() => true); + const disposeStale = setAppRestartHandler(stale); + const disposeCurrent = setAppRestartHandler(current); + + disposeStale(); + + expect(requestAppRestart()).toBe(true); + expect(current).toHaveBeenCalledOnce(); + + disposeCurrent(); + }); +}); diff --git a/packages/utils/core-utils/src/restart.ts b/packages/utils/core-utils/src/restart.ts new file mode 100644 index 0000000000..c9ed0ed7b3 --- /dev/null +++ b/packages/utils/core-utils/src/restart.ts @@ -0,0 +1,69 @@ +import debug from 'debug'; + +const d = debug('electron-forge:restart'); + +/** + * Restarts the running Electron app on behalf of {@link requestAppRestart}. + * Returns `true` if the restart was started or queued behind one already in + * flight, `false` if it could not be honored. + * + * @internal + */ +export type AppRestartHandler = () => boolean; + +// A single slot rather than an event emitter: two handlers would race each other +// to kill and respawn the same child process. +let restartHandler: AppRestartHandler | null = null; + +/** + * Requests that the running Electron app be restarted, reporting whether the + * request was honored. + * + * Returns `false` rather than throwing when there is nothing to restart, which + * is expected before the app is first spawned and in any process that isn't + * running `electron-forge start` — such as the Vite build subprocess used when + * packaging. Callers that can tell those cases apart should surface an unhonored + * request, since a rebuilt bundle that never reaches the app is invisible. + * + * @internal + */ +export function requestAppRestart(): boolean { + if (!restartHandler) { + d('no restart handler is registered, ignoring the restart request'); + return false; + } + + try { + return restartHandler(); + } catch (err) { + // Callers are usually bundler hooks, where a throw would be reported to the + // user as a *build* failure. + console.error( + 'Failed to restart the Electron app:', + err instanceof Error ? (err.stack ?? err.message) : err, + ); + return false; + } +} + +/** + * Installs the handler that {@link requestAppRestart} delegates to, replacing + * any existing one, and returns a function that uninstalls it again. + * + * The handler is process-wide, so two overlapping `start()` calls contend for it + * and only the most recent app stays restartable. + * + * @internal + */ +export function setAppRestartHandler(handler: AppRestartHandler): () => void { + if (restartHandler) { + d('replacing the previously registered restart handler'); + } + restartHandler = handler; + + return () => { + if (restartHandler === handler) { + restartHandler = null; + } + }; +}