final cleanups

Author: petebacondarwin

packages/vite-plugin-cloudflare/playground/__test-utils__/index.ts

@@ -8,8 +8,13 @@ export function failsIf(condition: boolean) {
 	return condition ? test.fails : test;
 }
 
+/**
+ * Makes a change to a file and restores it after the test is finished.
+ */
 export function mockFileChange(
+	/** The path to the file to change */
 	filePath: string,
+	/** A function that modifies the original content of the file */
 	mutateFn: (originalContent: string) => string
 ) {
 	const originalContent = fs.readFileSync(filePath, "utf-8");

packages/vite-plugin-cloudflare/playground/__test-utils__/responses.ts

@@ -1,5 +1,6 @@
 import { page, viteTestUrl } from "./index";
 
+/** Common options to use with `vi.waitFor()` */
 export const WAIT_FOR_OPTIONS = { timeout: 5_000, interval: 500 };
 
 export async function getTextResponse(path = "/"): Promise<string> {

packages/vite-plugin-cloudflare/playground/dev-vars/__tests__/with-specified-env/dev-vars-loading.spec.ts

@@ -1,15 +1,22 @@
 import fs from "node:fs";
 import { describe, expect, test, vi } from "vitest";
-import { getJsonResponse, isBuild, testDir } from "../../../__test-utils__";
+import {
+	getJsonResponse,
+	isBuild,
+	testDir,
+	WAIT_FOR_OPTIONS,
+} from "../../../__test-utils__";
 
 test("reading variables from a staging .dev.vars file", async () => {
-	await vi.waitFor(async () =>
-		expect(await getJsonResponse()).toEqual({
-			"variables present in .dev.vars.staging": {
-				MY_DEV_VAR_A: "my .dev.vars staging variable A",
-				MY_DEV_VAR_B: "my .dev.vars staging variable B",
-			},
-		})
+	await vi.waitFor(
+		async () =>
+			expect(await getJsonResponse()).toEqual({
+				"variables present in .dev.vars.staging": {
+					MY_DEV_VAR_A: "my .dev.vars staging variable A",
+					MY_DEV_VAR_B: "my .dev.vars staging variable B",
+				},
+			}),
+		WAIT_FOR_OPTIONS
 	);
 });
 

packages/vite-plugin-cloudflare/playground/dot-env/__tests__/with-specified-env/vars-changes.spec.ts

@@ -4,6 +4,7 @@ import {
 	getJsonResponse,
 	isBuild,
 	mockFileChange,
+	WAIT_FOR_OPTIONS,
 } from "../../../__test-utils__";
 
 test.runIf(!isBuild)(
@@ -26,18 +27,15 @@ test.runIf(!isBuild)(
 			content.replace(/my \.env staging/g, "my .env UPDATED staging")
 		);
 
-		await vi.waitFor(
-			async () => {
-				const updatedResponse = await getJsonResponse();
-				expect(updatedResponse).toEqual({
-					"variables loaded from .env and .env.staging": {
-						MY_DEV_VAR_A: "my .env UPDATED staging variable A",
-						MY_DEV_VAR_B: "my .env UPDATED staging variable B",
-						MY_DEV_VAR_C: "my .env UPDATED variable C", // Note that unlike .dev.vars, we merge .env files
-					},
-				});
-			},
-			{ timeout: 5000 }
-		);
+		await vi.waitFor(async () => {
+			const updatedResponse = await getJsonResponse();
+			expect(updatedResponse).toEqual({
+				"variables loaded from .env and .env.staging": {
+					MY_DEV_VAR_A: "my .env UPDATED staging variable A",
+					MY_DEV_VAR_B: "my .env UPDATED staging variable B",
+					MY_DEV_VAR_C: "my .env UPDATED variable C", // Note that unlike .dev.vars, we merge .env files
+				},
+			});
+		}, WAIT_FOR_OPTIONS);
 	}
 );

packages/vite-plugin-cloudflare/playground/node-compat/__tests__/worker-postgres/postgres.spec.ts

@@ -6,10 +6,12 @@ import {
 } from "../../../__test-utils__";
 
 test("should be able to create a pg Client", async () => {
-	await vi.waitFor(async () =>
-		expect(await getTextResponse()).toMatchInlineSnapshot(
-			`"hh-pgsql-public.ebi.ac.uk"`
-		)
+	await vi.waitFor(
+		async () =>
+			expect(await getTextResponse()).toMatchInlineSnapshot(
+				`"hh-pgsql-public.ebi.ac.uk"`
+			),
+		WAIT_FOR_OPTIONS
 	);
 });
 

packages/vite-plugin-cloudflare/playground/node-compat/__tests__/worker-warnings/warnings.spec.ts

@@ -1,6 +1,6 @@
 import dedent from "ts-dedent";
 import { expect, test, vi } from "vitest";
-import { isBuild, serverLogs } from "../../../__test-utils__";
+import { isBuild, serverLogs, WAIT_FOR_OPTIONS } from "../../../__test-utils__";
 
 test.skipIf(isBuild)(
 	"should display warnings if nodejs_compat is missing",
@@ -14,7 +14,7 @@ test.skipIf(isBuild)(
 				 - "perf_hooks" imported from "worker-warnings/index.ts"
 				`
 				),
-			{ timeout: 5_000, interval: 500 }
+			WAIT_FOR_OPTIONS
 		);
 	}
 );

packages/vite-plugin-cloudflare/playground/vitest.config.e2e.ts

@@ -13,7 +13,7 @@ export default defineConfig({
 		globalSetup: ["./vitest-global-setup.ts"],
 		reporters: "dot",
 		onConsoleLog: () => debuglog.enabled,
-		testTimeout: 100_000,
+		testTimeout: 10000,
 	},
 	publicDir: false,
 });

packages/vite-plugin-cloudflare/src/cloudflare-environment.ts

@@ -91,6 +91,7 @@ export class CloudflareDevEnvironment extends vite.DevEnvironment {
 	async initRunner(
 		worker: ReplaceWorkersTypes<Fetcher>,
 		workerConfig: WorkerConfig,
+		/** A unique identifier used for debugging errors when config updates. */
 		configId: string
 	) {
 		this.#worker = worker;
@@ -201,6 +202,7 @@ export function initRunners(
 	resolvedPluginConfig: WorkersResolvedConfig,
 	viteDevServer: vite.ViteDevServer,
 	miniflare: Miniflare,
+	/** A unique identifier used for debugging errors when config updates. */
 	configId: string
 ): Promise<void[]> | undefined {
 	return Promise.all(

packages/vite-plugin-cloudflare/src/index.ts

@@ -107,6 +107,7 @@ export function cloudflare(pluginConfig: PluginConfig = {}): vite.Plugin[] {
 	let containerImageTagsSeen: Set<string> | undefined;
 	let runningContainerIds: Array<string>;
 
+	/** Used to track whether hooks are being called because of a server restart or a server close event. */
 	let restartingServer = false;
 
 	return [
@@ -343,6 +344,7 @@ if (import.meta.hot) {
 			// Vite `configureServer` Hook
 			// see https://vite.dev/guide/api-plugin.html#configureserver
 			async configureServer(viteDevServer) {
+				// Patch the `server.restart` method to track whether the server is restarting or not.
 				const restartServer = viteDevServer.restart.bind(viteDevServer);
 				viteDevServer.restart = async () => {
 					try {
@@ -354,12 +356,14 @@ if (import.meta.hot) {
 						restartingServer = false;
 					}
 				};
+
 				assertIsNotPreview(resolvedPluginConfig);
 
 				// It is possible to get into a situation where the dev server is restarted by a config file change
 				// right in the middle of the Vite server and the supporting Workers being initialized.
 				// We use an abort controller to signal to the initialization code that it should stop if the config has changed.
 				const restartAbortController = new AbortController();
+
 				// We use a `configId` to help debug how the config changes are triggering the restarts.
 				const configId = randomUUID();
 
@@ -424,8 +428,8 @@ if (import.meta.hot) {
 						configId,
 						"Aborting setting up miniflare because config has changed."
 					);
-					// The config has changes while this was still trying to setup the server.
-					// So just abort and allow the new server to be set up.
+					// The config has changed while we were still trying to setup the server,
+					// so just abort and allow the new server to be set up instead.
 					return;
 				}
 

packages/vite-plugin-cloudflare/src/runner-worker/index.ts

@@ -132,6 +132,7 @@ export function createWorkerEntrypointWrapper(
 	entrypoint: string
 ): WorkerEntrypointConstructor<WrapperEnv> {
 	class Wrapper extends WorkerEntrypoint<WrapperEnv> {
+		/** A unique identifier used for debugging errors when config updates. */
 		configId?: string;
 		constructor(ctx: ExecutionContext, env: WrapperEnv) {
 			super(ctx, env);