React-free hotkeys implementation (#6149)

* initial implementation

* implement higher-level interface for hotkeys
automatically compute instructions

* extract onKeyPress to own module
for mocking

* add tests

* add changeset

* add --x-dev-env flagged run of fixtures/interactive-dev-tests

* add test for handlers throwing errors

* chore: cleanup teardown

* fix: onKeyPress keeps the process alive after unregistering

Listening for events on process.stdin (eg .on('keypress')) causes it to go into 'old mode'
which keeps this nodejs process alive even after calling .off('keypress')

WORKAROUND: piping stdin via a transform stream allows us to call stream.destroy()
which then allows this nodejs process to close cleanly

* hotkeys integration

* implement Logger bottom float

* use registerGlobalBottomFloat for floating hotkeys instructions

* move registerGlobalBottomFloat calls into hotkeys implementation

* add test steps for unregistered hotkeys

+ update mock to return unregisterHotKeys mock

* fix lints

* setIsTTY(true) in tests

* add docs link to comment

* unregisterHotKeys before devEnv.teardown

* enable modifier key detection in hotkeys api

* fix: store previousBottomFloatLineCount in case the line count changes
+ ensures no existing logs (from before registerGlobalBottomFloat was called) are cleared

* fix: usages of console.dir/table -> logger.dir/table
+ use static methods of miniflare Log which are now stateful -- rather than reimplement in Wrangler's Logger not sharing state

* mark methods with unsafe_

* fixup

* use setIsTTY inside beforeEach

* use readline.moveCursor(stdout,...) instead of stdout.moveCursor(...)

* abstract knowledge of bototm float from logger implementations

* fix lints

* use readline.clearScreenDown

* handle forceLocal (disabled hotkey options)

* refactor

* update changeset type to refactor

* implement logger.console

* unregister hotkeys during auth prompt

* update AbortError checks

* fix logger.console types

* change getAccountId to requireAuth

* check isTTY in unregister keypress

* fix RemoteRuntimeController shutdown log

* use isNonInteractiveOrCI util

* hookify the hotkeys disabled option

* use isInteractive() instead of !isNonIteractiveOrCI()

.changeset/few-days-tap.md

@@ -0,0 +1,5 @@
+---
+"wrangler": patch
+---
+
+refactor: React-free hotkeys implementation, behind the `--x-dev-env` flag

fixtures/interactive-dev-tests/tests/index.test.ts

@@ -197,6 +197,7 @@ function getStartedWorkerdProcesses(): Process[] {
 
 const devScripts = [
 	{ args: ["dev"], expectedBody: "body" },
+	{ args: ["dev", "--x-dev-env"], expectedBody: "body" },
 	{ args: ["pages", "dev", "public"], expectedBody: "<p>body</p>" },
 ];
 const exitKeys = [

packages/miniflare/src/shared/log.ts

@@ -66,7 +66,18 @@ export class Log {
 	}
 
 	protected log(message: string): void {
+		Log.#beforeLogHook?.();
 		console.log(message);
+		Log.#afterLogHook?.();
+	}
+
+	static #beforeLogHook: (() => void) | undefined;
+	static unstable_registerBeforeLogHook(callback: (() => void) | undefined) {
+		this.#beforeLogHook = callback;
+	}
+	static #afterLogHook: (() => void) | undefined;
+	static unstable_registerAfterLogHook(callback: (() => void) | undefined) {
+		this.#afterLogHook = callback;
 	}
 
 	logWithLevel(level: LogLevel, message: string): void {

packages/wrangler/src/__tests__/cli-hotkeys.test.ts

@@ -0,0 +1,203 @@
+import { setTimeout } from "node:timers/promises";
+import { vitest } from "vitest";
+import registerHotKeys from "../cli-hotkeys";
+import { logger } from "../logger";
+import { mockConsoleMethods } from "./helpers/mock-console";
+import { useMockIsTTY } from "./helpers/mock-istty";
+import type { KeypressEvent } from "../utils/onKeyPress";
+
+const writeToMockedStdin = (input: string) =>
+	_internalKeyPressCallback({
+		name: input,
+		sequence: input,
+		ctrl: false,
+		meta: false,
+		shift: false,
+	});
+let _internalKeyPressCallback: (input: KeypressEvent) => void;
+vitest.mock("../utils/onKeyPress", async () => {
+	return {
+		onKeyPress(callback: () => void) {
+			_internalKeyPressCallback = callback;
+
+			return () => {};
+		},
+	};
+});
+
+describe("Hot Keys", () => {
+	const std = mockConsoleMethods();
+	const { setIsTTY } = useMockIsTTY();
+	beforeEach(() => {
+		setIsTTY(true);
+	});
+
+	describe("callbacks", () => {
+		it("calls handlers when a key is pressed", async () => {
+			const handlerA = vi.fn();
+			const handlerB = vi.fn();
+			const handlerC = vi.fn();
+			const options = [
+				{ keys: ["a"], label: "first option", handler: handlerA },
+				{ keys: ["b"], label: "second option", handler: handlerB },
+				{ keys: ["c"], label: "third option", handler: handlerC },
+			];
+
+			registerHotKeys(options);
+
+			writeToMockedStdin("a");
+			expect(handlerA).toHaveBeenCalled();
+			expect(handlerB).not.toHaveBeenCalled();
+			expect(handlerC).not.toHaveBeenCalled();
+			handlerA.mockClear();
+
+			writeToMockedStdin("b");
+			expect(handlerA).not.toHaveBeenCalled();
+			expect(handlerB).toHaveBeenCalled();
+			expect(handlerC).not.toHaveBeenCalled();
+			handlerB.mockClear();
+
+			writeToMockedStdin("c");
+			expect(handlerA).not.toHaveBeenCalled();
+			expect(handlerB).not.toHaveBeenCalled();
+			expect(handlerC).toHaveBeenCalled();
+			handlerC.mockClear();
+		});
+
+		it("handles CAPSLOCK", async () => {
+			const handlerA = vi.fn();
+			const options = [
+				{ keys: ["a"], label: "first option", handler: handlerA },
+			];
+
+			registerHotKeys(options);
+
+			writeToMockedStdin("a");
+			expect(handlerA).toHaveBeenCalled();
+			handlerA.mockClear();
+
+			writeToMockedStdin("A");
+			expect(handlerA).toHaveBeenCalled();
+			handlerA.mockClear();
+		});
+
+		it("ignores unbound keys", async () => {
+			const handlerA = vi.fn();
+			const handlerD = vi.fn();
+			const options = [
+				{ keys: ["a"], label: "first option", handler: handlerA },
+				{ keys: ["d"], label: "disabled", disabled: true, handler: handlerD },
+			];
+
+			registerHotKeys(options);
+
+			writeToMockedStdin("z");
+			expect(handlerA).not.toHaveBeenCalled();
+
+			writeToMockedStdin("d");
+			expect(handlerD).not.toHaveBeenCalled();
+		});
+
+		it("calls handler if any additional key bindings are pressed", async () => {
+			const handlerA = vi.fn();
+			const options = [
+				{ keys: ["a", "b", "c"], label: "first option", handler: handlerA },
+			];
+
+			registerHotKeys(options);
+
+			writeToMockedStdin("a");
+			expect(handlerA).toHaveBeenCalled();
+			handlerA.mockClear();
+
+			writeToMockedStdin("b");
+			expect(handlerA).toHaveBeenCalled();
+			handlerA.mockClear();
+
+			writeToMockedStdin("c");
+			expect(handlerA).toHaveBeenCalled();
+			handlerA.mockClear();
+		});
+
+		it("surfaces errors in handlers", async () => {
+			const handlerA = vi.fn().mockImplementation(() => {
+				throw new Error("sync error");
+			});
+			const handlerB = vi.fn().mockRejectedValue("async error");
+			const options = [
+				{ keys: ["a"], label: "first option", handler: handlerA },
+				{ keys: ["b"], label: "second option", handler: handlerB },
+			];
+
+			registerHotKeys(options);
+
+			writeToMockedStdin("a");
+			expect(std.err).toMatchInlineSnapshot(`
+				"X [ERROR] Error while handling hotkey [a]
+
+				"
+			`);
+
+			writeToMockedStdin("b");
+			await setTimeout(0); //
+			expect(std.err).toMatchInlineSnapshot(`
+				"X [ERROR] Error while handling hotkey [a]
+
+
+				X [ERROR] Error while handling hotkey [b]
+
+				"
+			`);
+		});
+	});
+
+	describe("instructions", () => {
+		it("provides formatted instructions to Wrangler's & Miniflare's logger implementations", async () => {
+			const handlerA = vi.fn();
+			const handlerB = vi.fn();
+			const handlerC = vi.fn();
+			const handlerD = vi.fn();
+			const options = [
+				{ keys: ["a"], label: "first option", handler: handlerA },
+				{ keys: ["b"], label: "second option", handler: handlerB },
+				{ keys: ["c"], label: () => "third option", handler: handlerC },
+				{ keys: ["d"], label: "disabled", disabled: true, handler: handlerD },
+			];
+
+			// should print instructions immediately
+			const unregisterHotKeys = registerHotKeys(options);
+
+			expect(std.out).toMatchInlineSnapshot(`
+				"╭─────────────────────────────────────────────────────────╮
+				│  [a] first option, [b] second option, [c] third option  │
+				╰─────────────────────────────────────────────────────────╯"
+			`);
+
+			logger.log("something 1");
+
+			expect(std.out).toMatchInlineSnapshot(`
+				"╭─────────────────────────────────────────────────────────╮
+				│  [a] first option, [b] second option, [c] third option  │
+				╰─────────────────────────────────────────────────────────╯
+				something 1
+				╭─────────────────────────────────────────────────────────╮
+				│  [a] first option, [b] second option, [c] third option  │
+				╰─────────────────────────────────────────────────────────╯"
+			`);
+
+			unregisterHotKeys();
+			logger.log("something 2");
+
+			expect(std.out).toMatchInlineSnapshot(`
+				"╭─────────────────────────────────────────────────────────╮
+				│  [a] first option, [b] second option, [c] third option  │
+				╰─────────────────────────────────────────────────────────╯
+				something 1
+				╭─────────────────────────────────────────────────────────╮
+				│  [a] first option, [b] second option, [c] third option  │
+				╰─────────────────────────────────────────────────────────╯
+				something 2"
+			`);
+		});
+	});
+});

packages/wrangler/src/api/startDevWorker/BundlerController.ts

@@ -280,10 +280,14 @@ export class BundlerController extends Controller<BundlerControllerEventMap> {
 	}
 
 	async teardown() {
-		await this.#customBuildWatcher?.close();
-		await this.#bundlerCleanup?.();
+		logger.debug("BundlerController teardown beginning...");
 		this.#customBuildAborter?.abort();
 		this.#tmpDir?.remove();
+		await Promise.all([
+			this.#bundlerCleanup?.(),
+			this.#customBuildWatcher?.close(),
+		]);
+		logger.debug("BundlerController teardown complete");
 	}
 
 	emitBundleStartEvent(config: StartDevWorkerOptions) {

packages/wrangler/src/api/startDevWorker/ConfigController.ts

@@ -379,7 +379,9 @@ export class ConfigController extends Controller<ConfigControllerEventMap> {
 	// ******************
 
 	async teardown() {
+		logger.debug("ConfigController teardown beginning...");
 		await this.#configWatcher?.close();
+		logger.debug("ConfigController teardown complete");
 	}
 
 	// *********************

packages/wrangler/src/api/startDevWorker/DevEnv.ts

@@ -93,13 +93,23 @@ export class DevEnv extends EventEmitter {
 	// *********************
 
 	async teardown() {
+		logger.debug("DevEnv teardown beginning...");
+
 		await Promise.all([
 			this.config.teardown(),
 			this.bundler.teardown(),
 			...this.runtimes.map((runtime) => runtime.teardown()),
 			this.proxy.teardown(),
 		]);
+
+		this.config.removeAllListeners();
+		this.bundler.removeAllListeners();
+		this.runtimes.forEach((runtime) => runtime.removeAllListeners());
+		this.proxy.removeAllListeners();
+
 		this.emit("teardown");
+
+		logger.debug("DevEnv teardown complete");
 	}
 
 	emitErrorEvent(ev: ErrorEvent) {

packages/wrangler/src/api/startDevWorker/LocalRuntimeController.ts

@@ -239,12 +239,16 @@ export class LocalRuntimeController extends RuntimeController {
 	}
 
 	#teardown = async (): Promise<void> => {
+		logger.debug("LocalRuntimeController teardown beginning...");
+
 		if (this.#mf) {
 			logger.log(chalk.dim("⎔ Shutting down local server..."));
 		}
 
 		await this.#mf?.dispose();
 		this.#mf = undefined;
+
+		logger.debug("LocalRuntimeController teardown complete");
 	};
 	async teardown() {
 		return this.#mutex.runWith(this.#teardown);

packages/wrangler/src/api/startDevWorker/ProxyController.ts

@@ -485,7 +485,7 @@ export class ProxyController extends Controller<ProxyControllerEventMap> {
 
 	_torndown = false;
 	async teardown() {
-		logger.debug("ProxyController teardown");
+		logger.debug("ProxyController teardown beginning...");
 		this._torndown = true;
 
 		const { proxyWorker } = this;
@@ -499,6 +499,8 @@ export class ProxyController extends Controller<ProxyControllerEventMap> {
 					/* ignore */
 				}),
 		]);
+
+		logger.debug("ProxyController teardown complete");
 	}
 
 	// *********************

packages/wrangler/src/api/startDevWorker/RemoteRuntimeController.ts

@@ -51,6 +51,10 @@ export class RemoteRuntimeController extends RuntimeController {
 				this.#abortController.signal
 			);
 		} catch (err: unknown) {
+			if (err instanceof Error && err.name == "AbortError") {
+				return; // ignore
+			}
+
 			handlePreviewSessionCreationError(err, props.accountId);
 		}
 	}
@@ -98,6 +102,10 @@ export class RemoteRuntimeController extends RuntimeController {
 
 			return workerPreviewToken;
 		} catch (err: unknown) {
+			if (err instanceof Error && err.name == "AbortError") {
+				return; // ignore
+			}
+
 			const shouldRestartSession = handlePreviewSessionUploadError(
 				err,
 				props.accountId
@@ -110,6 +118,8 @@ export class RemoteRuntimeController extends RuntimeController {
 	}
 
 	async #onBundleComplete({ config, bundle }: BundleCompleteEvent, id: number) {
+		logger.log(chalk.dim("⎔ Starting remote preview..."));
+
 		try {
 			const routes = config.triggers
 				?.filter(
@@ -213,6 +223,10 @@ export class RemoteRuntimeController extends RuntimeController {
 				},
 			});
 		} catch (error) {
+			if (error instanceof Error && error.name == "AbortError") {
+				return; // ignore
+			}
+
 			this.emitErrorEvent({
 				type: "error",
 				reason: "Error reloading remote server",
@@ -236,7 +250,7 @@ export class RemoteRuntimeController extends RuntimeController {
 		const id = ++this.#currentBundleId;
 
 		if (!ev.config.dev?.remote) {
-			void this.teardown();
+			void this.#mutex.runWith(() => this.teardown());
 			return;
 		}
 
@@ -253,8 +267,13 @@ export class RemoteRuntimeController extends RuntimeController {
 	}
 
 	async teardown() {
+		if (this.#session) {
+			logger.log(chalk.dim("⎔ Shutting down remote preview..."));
+		}
+		logger.debug("RemoteRuntimeController teardown beginning...");
 		this.#session = undefined;
 		this.#abortController.abort();
+		logger.debug("RemoteRuntimeController teardown complete");
 	}
 
 	// *********************