[vite-plugin] make sure that .dev.vars files trigger a full reload (#8685)

* [vite-plugin] make sure that `.dev.vars` files trigger a full reload

* Apply suggestions from code review

Co-authored-by: James Opstad <[email protected]>

* use `.toContains` instead of `.toMatch`

---------

Co-authored-by: James Opstad <[email protected]>

Author: dario-piotrowicz

.changeset/blue-hats-accept.md

@@ -0,0 +1,5 @@
+---
+"@cloudflare/vite-plugin": patch
+---
+
+make sure that `.dev.vars` files trigger a full reload

packages/vite-plugin-cloudflare/playground/config-changes/.dev.vars

@@ -0,0 +1 @@
+MY_SECRET = "secret A"

packages/vite-plugin-cloudflare/playground/config-changes/.gitignore

@@ -0,0 +1 @@
+!.dev.vars

packages/vite-plugin-cloudflare/playground/config-changes/__tests__/config-changes.spec.ts

@@ -15,14 +15,14 @@ test.runIf(!isBuild)(
 			await vi.waitFor(
 				async () => {
 					const revertedResponse = await getTextResponse();
-					expect(revertedResponse).toBe('The value of MY_VAR is "one"');
+					expect(revertedResponse).toContain('The value of MY_VAR is "one"');
 				},
 				{ timeout: 5000 }
 			);
 		});
 
 		const originalResponse = await getTextResponse();
-		expect(originalResponse).toBe('The value of MY_VAR is "one"');
+		expect(originalResponse).toContain('The value of MY_VAR is "one"');
 
 		const updatedWorkerConfig = JSON.stringify({
 			...JSON.parse(originalWorkerConfig),
@@ -34,7 +34,7 @@ test.runIf(!isBuild)(
 		await vi.waitFor(
 			async () => {
 				const updatedResponse = await getTextResponse();
-				expect(updatedResponse).toBe('The value of MY_VAR is "two"');
+				expect(updatedResponse).toContain('The value of MY_VAR is "two"');
 			},
 			{ timeout: 5000 }
 		);
@@ -53,14 +53,14 @@ test.runIf(!isBuild)(
 			await vi.waitFor(
 				async () => {
 					const revertedResponse = await getTextResponse();
-					expect(revertedResponse).toBe('The value of MY_VAR is "one"');
+					expect(revertedResponse).toContain('The value of MY_VAR is "one"');
 				},
 				{ timeout: 5000 }
 			);
 		});
 
 		const originalResponse = await getTextResponse();
-		expect(originalResponse).toBe('The value of MY_VAR is "one"');
+		expect(originalResponse).toContain('The value of MY_VAR is "one"');
 
 		const updatedWorkerConfig = JSON.stringify({
 			...JSON.parse(originalWorkerConfig),
@@ -76,7 +76,7 @@ test.runIf(!isBuild)(
 				expect(serverLogs.errors.join()).toMatch(
 					/.*The provided Wrangler config main field .+? doesn't point to an existing file.*/
 				);
-				expect(newResponse).toBe('The value of MY_VAR is "one"');
+				expect(newResponse).toContain('The value of MY_VAR is "one"');
 			},
 			{ timeout: 5000 }
 		);

packages/vite-plugin-cloudflare/playground/config-changes/src/index.ts

@@ -1,9 +1,12 @@
 interface Env {
 	MY_VAR: string;
+	MY_SECRET: string;
 }
 
 export default {
 	async fetch(_, env) {
-		return new Response(`The value of MY_VAR is "${env.MY_VAR}"`);
+		return new Response(
+			`The value of MY_VAR is "${env.MY_VAR}" and the value of MY_SECRET is "${env.MY_SECRET}"`
+		);
 	},
 } satisfies ExportedHandler<Env>;

packages/vite-plugin-cloudflare/playground/config-changes/vite.config.with-cloudflare-env.ts

@@ -0,0 +1,7 @@
+import { cloudflare } from "@cloudflare/vite-plugin";
+import { defineConfig } from "vite";
+
+export default defineConfig({
+	mode: "with-specified-env",
+	plugins: [cloudflare({ inspectorPort: false, persistState: false })],
+});

packages/vite-plugin-cloudflare/playground/dev-vars/__tests__/vars-changes.spec.ts

@@ -0,0 +1,61 @@
+import * as fs from "node:fs";
+import * as path from "node:path";
+import { expect, test, vi } from "vitest";
+import { getJsonResponse, isBuild } from "../../__test-utils__";
+
+test.runIf(!isBuild)(
+	"successfully updates when a var is updated in a .dev.vars file",
+	async ({ onTestFinished }) => {
+		const dotDevDotVarsFilePath = path.join(__dirname, "../.dev.vars");
+		const originalDotDevDotVars = fs.readFileSync(
+			dotDevDotVarsFilePath,
+			"utf-8"
+		);
+
+		onTestFinished(async () => {
+			fs.writeFileSync(dotDevDotVarsFilePath, originalDotDevDotVars);
+			// We need to ensure that the original config is restored before the next test runs
+			await vi.waitFor(
+				async () => {
+					expect(await getJsonResponse()).toEqual({
+						"variables present in .dev.vars": {
+							MY_DEV_VAR_A: "my .dev.vars variable A",
+							MY_DEV_VAR_B: "my .dev.vars variable B",
+							MY_DEV_VAR_C: "my .dev.vars variable C",
+						},
+					});
+				},
+				{ timeout: 5000 }
+			);
+		});
+
+		const originalResponse = await getJsonResponse();
+		expect(originalResponse).toEqual({
+			"variables present in .dev.vars": {
+				MY_DEV_VAR_A: "my .dev.vars variable A",
+				MY_DEV_VAR_B: "my .dev.vars variable B",
+				MY_DEV_VAR_C: "my .dev.vars variable C",
+			},
+		});
+
+		const updatedDotDevDotVars = originalDotDevDotVars.replace(
+			/my \.dev\.vars variable/g,
+			"my .dev.vars UPDATED variable"
+		);
+
+		fs.writeFileSync(dotDevDotVarsFilePath, updatedDotDevDotVars);
+		await vi.waitFor(
+			async () => {
+				const updatedResponse = await getJsonResponse();
+				expect(updatedResponse).toEqual({
+					"variables present in .dev.vars": {
+						MY_DEV_VAR_A: "my .dev.vars UPDATED variable A",
+						MY_DEV_VAR_B: "my .dev.vars UPDATED variable B",
+						MY_DEV_VAR_C: "my .dev.vars UPDATED variable C",
+					},
+				});
+			},
+			{ timeout: 5000 }
+		);
+	}
+);

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

@@ -0,0 +1,61 @@
+import * as fs from "node:fs";
+import * as path from "node:path";
+import { expect, test, vi } from "vitest";
+import { getJsonResponse, isBuild } from "../../../__test-utils__";
+
+test.runIf(!isBuild)(
+	"successfully updates when a var is updated in a .dev.vars.staging file",
+	async ({ onTestFinished }) => {
+		const dotDevDotVarsFilePath = path.join(
+			__dirname,
+			"../../.dev.vars.staging"
+		);
+		const originalDotDevDotVars = fs.readFileSync(
+			dotDevDotVarsFilePath,
+			"utf-8"
+		);
+
+		onTestFinished(async () => {
+			fs.writeFileSync(dotDevDotVarsFilePath, originalDotDevDotVars);
+			// We need to ensure that the original config is restored before the next test runs
+			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",
+						},
+					});
+				},
+				{ timeout: 5000 }
+			);
+		});
+
+		const originalResponse = await getJsonResponse();
+		expect(originalResponse).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",
+			},
+		});
+
+		const updatedDotDevDotVars = originalDotDevDotVars.replace(
+			/my \.dev\.vars staging variable/g,
+			"my .dev.vars UPDATED staging variable"
+		);
+
+		fs.writeFileSync(dotDevDotVarsFilePath, updatedDotDevDotVars);
+		await vi.waitFor(
+			async () => {
+				const updatedResponse = await getJsonResponse();
+				expect(updatedResponse).toEqual({
+					"variables present in .dev.vars.staging": {
+						MY_DEV_VAR_A: "my .dev.vars UPDATED staging variable A",
+						MY_DEV_VAR_B: "my .dev.vars UPDATED staging variable B",
+					},
+				});
+			},
+			{ timeout: 5000 }
+		);
+	}
+);

packages/vite-plugin-cloudflare/src/asset-config.ts

@@ -26,16 +26,15 @@ import type { Unstable_Config } from "wrangler";
 export function hasAssetsConfigChanged(
 	resolvedPluginConfig: ResolvedPluginConfig,
 	resolvedViteConfig: ResolvedConfig,
-	changedFile: string
+	changedFilePath: string
 ) {
 	if (!resolvedPluginConfig.experimental?.headersAndRedirectsDevModeSupport) {
 		return false;
 	}
-	// Note that we must "resolve" the changed file since the path from Vite will not match Windows backslashes.
 	return [
 		getRedirectsConfigPath(resolvedViteConfig),
 		getHeadersConfigPath(resolvedViteConfig),
-	].includes(path.resolve(changedFile));
+	].includes(changedFilePath);
 }
 
 /**

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

@@ -309,13 +309,16 @@ export function cloudflare(pluginConfig: PluginConfig = {}): vite.Plugin[] {
 				}
 			},
 			hotUpdate(options) {
+				// Note that we must "resolve" the changed file since the path from Vite will not match Windows backslashes.
+				const changedFilePath = path.resolve(options.file);
+
 				if (
-					// Vite normalizes `options.file` so we use `path.resolve` for Windows compatibility
-					resolvedPluginConfig.configPaths.has(path.resolve(options.file)) ||
+					resolvedPluginConfig.configPaths.has(changedFilePath) ||
+					hasDotDevDotVarsFileChanged(resolvedPluginConfig, changedFilePath) ||
 					hasAssetsConfigChanged(
 						resolvedPluginConfig,
 						resolvedViteConfig,
-						options.file
+						changedFilePath
 					)
 				) {
 					// It's OK for this to be called multiple times as Vite prevents concurrent execution
@@ -868,3 +871,25 @@ function getDotDevDotVarsContent(
 
 	return null;
 }
+
+/**
+ * Returns true if the `changedFile` matches one of a potential .dev.vars file.
+ */
+function hasDotDevDotVarsFileChanged(
+	resolvedPluginConfig: ResolvedPluginConfig,
+	changedFilePath: string
+) {
+	return [...resolvedPluginConfig.configPaths].some((configPath) => {
+		const dotDevDotVars = path.join(path.dirname(configPath), ".dev.vars");
+		if (dotDevDotVars === changedFilePath) {
+			return true;
+		}
+
+		if (resolvedPluginConfig.cloudflareEnv) {
+			const dotDevDotVarsForEnv = `${dotDevDotVars}.${resolvedPluginConfig.cloudflareEnv}`;
+			return dotDevDotVarsForEnv === changedFilePath;
+		}
+
+		return false;
+	});
+}