diff --git a/.changeset/salty-plums-worry.md b/.changeset/salty-plums-worry.md new file mode 100644 index 000000000000..54044ecfcc69 --- /dev/null +++ b/.changeset/salty-plums-worry.md @@ -0,0 +1,7 @@ +--- +"@cloudflare/vite-plugin": patch +--- + +fix: track server restart in module scope + +When using `@cloudflare/vite-plugin` with React Router, miniflare might be disposed during restart. This change makes sure to track when the dev server restart in module scope to avoid unexpected behavior. diff --git a/packages/create-cloudflare/e2e/helpers/framework-helpers.ts b/packages/create-cloudflare/e2e/helpers/framework-helpers.ts index 740f8bb7a4c7..db0dad554b7d 100644 --- a/packages/create-cloudflare/e2e/helpers/framework-helpers.ts +++ b/packages/create-cloudflare/e2e/helpers/framework-helpers.ts @@ -30,7 +30,6 @@ import { runC3 } from "./run-c3"; import { kill, spawnWithLogging } from "./spawn"; import type { TemplateConfig } from "../../src/templates"; import type { RunnerConfig } from "./run-c3"; -import type { JsonMap } from "@iarna/toml"; import type { Writable } from "stream"; export type FrameworkTestConfig = RunnerConfig & { @@ -87,33 +86,59 @@ export async function runC3ForFrameworkTest( return match[1]; } -/** - * Either update or create a wrangler configuration file to include a `TEST` var. - * - * This is rather than having a wrangler configuration file in the e2e test's fixture folder, - * which overwrites any that comes from the framework's template. - */ -export async function addTestVarsToWranglerToml(projectPath: string) { +export function updateWranglerConfig( + projectPath: string, + // eslint-disable-next-line @typescript-eslint/no-explicit-any + handleUpdate: >(config: T) => T, +) { const wranglerTomlPath = join(projectPath, "wrangler.toml"); const wranglerJsoncPath = join(projectPath, "wrangler.jsonc"); if (existsSync(wranglerTomlPath)) { const wranglerToml = readToml(wranglerTomlPath); - wranglerToml.vars ??= {}; - (wranglerToml.vars as JsonMap).TEST = "C3_TEST"; - writeToml(wranglerTomlPath, wranglerToml); + writeToml( + wranglerTomlPath, + handleUpdate(JSON.parse(JSON.stringify(wranglerToml))), + ); + + return () => { + writeToml(wranglerTomlPath, handleUpdate(wranglerToml)); + }; } else if (existsSync(wranglerJsoncPath)) { const wranglerJsonc = readJSON(wranglerJsoncPath) as { vars: Record; }; - wranglerJsonc.vars ??= {}; - wranglerJsonc.vars.TEST = "C3_TEST"; - writeJSON(wranglerJsoncPath, wranglerJsonc); + writeJSON( + wranglerJsoncPath, + handleUpdate(JSON.parse(JSON.stringify(wranglerJsonc))), + ); + + return () => { + writeJSON(wranglerJsoncPath, wranglerJsonc); + }; } } +/** + * Either update or create a wrangler configuration file to include a `TEST` var. + * + * This is rather than having a wrangler configuration file in the e2e test's fixture folder, + * which overwrites any that comes from the framework's template. + */ +export async function addTestVarsToWranglerToml(projectPath: string) { + updateWranglerConfig(projectPath, (config) => { + return { + ...config, + vars: { + ...config.vars, + TEST: "C3_TEST", + }, + }; + }); +} + export async function verifyDeployment( { testCommitMessage }: FrameworkTestConfig, frameworkId: string, @@ -141,6 +166,83 @@ export async function verifyDeployment( }); } +export async function verifyDevScript( + { verifyDev }: FrameworkTestConfig, + { devScript }: TemplateConfig, + projectPath: string, + logStream: Writable, +) { + if (!verifyDev) { + return; + } + + assert( + devScript, + "Expected a dev script as we are verifying the dev session in " + + projectPath, + ); + + // Run the dev-server on random ports to avoid colliding with other tests + const port = await getPort(); + const proc = spawnWithLogging( + [ + packageManager.name, + "run", + devScript, + ...(packageManager.name === "npm" ? ["--"] : []), + ...(verifyDev.devArgs ?? []), + "--port", + `${port}`, + ], + { + cwd: projectPath, + env: { + VITEST: undefined, + }, + }, + logStream, + ); + + let restoreConfig: (() => void) | undefined; + + try { + await retry( + { times: 300, sleepMs: 5000 }, + async () => await fetch(`http://127.0.0.1:${port}${verifyDev.route}`), + ); + + // Make a request to the specified test route + const res = await fetch(`http://127.0.0.1:${port}${verifyDev.route}`); + expect(await res.text()).toContain(verifyDev.expectedText); + + if (verifyDev.configChanges) { + const { configChanges } = verifyDev; + const updatedVars = configChanges.vars; + + restoreConfig = updateWranglerConfig(projectPath, (config) => ({ + ...config, + vars: { + ...config.vars, + ...updatedVars, + }, + })); + + await retry({ times: 10, sleepMs: 500 }, async () => { + const res2 = await fetch(`http://127.0.0.1:${port}${verifyDev.route}`); + expect(await res2.text()).toContain(configChanges.expectedText); + }); + } + } finally { + // Kill the process gracefully so ports can be cleaned up + await kill(proc); + // Restore the wrangler config if we modified it + restoreConfig?.(); + // Wait for a second to allow process to exit cleanly. Otherwise, the port might + // end up camped and cause future runs to fail + await sleep(1000); + } +} + export async function verifyPreviewScript( { verifyPreview }: FrameworkTestConfig, { previewScript }: TemplateConfig, diff --git a/packages/create-cloudflare/e2e/helpers/run-c3.ts b/packages/create-cloudflare/e2e/helpers/run-c3.ts index 988f212051d7..1992df0f91f3 100644 --- a/packages/create-cloudflare/e2e/helpers/run-c3.ts +++ b/packages/create-cloudflare/e2e/helpers/run-c3.ts @@ -46,6 +46,18 @@ export type RunnerConfig = { * Specifies whether to run the test script for the project and verify the exit code. */ verifyTest?: boolean; + /** + * Specifies whether to run the dev script for the project and verify the response from the specified route. + */ + verifyDev?: { + devArgs?: string[]; + route: string; + expectedText: string; + configChanges?: { + vars: Record; + expectedText: string; + }; + }; }; export const runC3 = async ( diff --git a/packages/create-cloudflare/e2e/tests/frameworks/frameworks.test.ts b/packages/create-cloudflare/e2e/tests/frameworks/frameworks.test.ts index 4ebcd80462fe..88f295b68bb5 100644 --- a/packages/create-cloudflare/e2e/tests/frameworks/frameworks.test.ts +++ b/packages/create-cloudflare/e2e/tests/frameworks/frameworks.test.ts @@ -17,6 +17,7 @@ import { shouldRunTest, testGitCommitMessage, verifyDeployment, + verifyDevScript, verifyPreviewScript, verifyTypes, } from "../../helpers/framework-helpers"; @@ -134,6 +135,13 @@ describe } } + await verifyDevScript( + testConfig, + frameworkConfig, + project.path, + logStream, + ); + await verifyPreviewScript( testConfig, frameworkConfig, diff --git a/packages/create-cloudflare/e2e/tests/frameworks/test-config.ts b/packages/create-cloudflare/e2e/tests/frameworks/test-config.ts index 046c8f9be67f..467b28a54a1e 100644 --- a/packages/create-cloudflare/e2e/tests/frameworks/test-config.ts +++ b/packages/create-cloudflare/e2e/tests/frameworks/test-config.ts @@ -30,6 +30,17 @@ function getFrameworkTestConfig(pm: string): NamedFrameworkTestConfig[] { expectedText: "Hello from Cloudflare", previewArgs: ["--host=127.0.0.1"], }, + verifyDev: { + route: "/", + expectedText: "Hello from Cloudflare", + devArgs: ["--host=127.0.0.1"], + configChanges: { + vars: { + VALUE_FROM_CLOUDFLARE: "Hello React Router", + }, + expectedText: "Hello React Router", + }, + }, nodeCompat: false, flags: ["--no-install", "--no-git-init"], }, diff --git a/packages/mock-npm-registry/src/index.ts b/packages/mock-npm-registry/src/index.ts index 49906ee35eac..5b00922cf42b 100644 --- a/packages/mock-npm-registry/src/index.ts +++ b/packages/mock-npm-registry/src/index.ts @@ -41,9 +41,13 @@ export async function startMockNpmRegistry(...targetPackages: string[]) { ); console.log( - `Starting up local npm registry on http://localhost:${registryPort} at ${registryPath}` + `Starting up local npm registry on http://localhost:${registryPort} at ${registryPath} with ${pkgs.size} packages published:` ); + for (const [pkg, pkgPath] of pkgs.entries()) { + console.log(` - ${pkg} (${pkgPath})`); + } + let stopServer = await startVerdaccioServer(configPath); const tempConfig = path.join(registryPath, ".npmrc"); diff --git a/packages/vite-plugin-cloudflare/src/index.ts b/packages/vite-plugin-cloudflare/src/index.ts index 8f7b248495a8..d76c118ff002 100644 --- a/packages/vite-plugin-cloudflare/src/index.ts +++ b/packages/vite-plugin-cloudflare/src/index.ts @@ -64,6 +64,8 @@ const debuglog = util.debuglog("@cloudflare:vite-plugin"); // this flag is used to show the workers configs warning only once let workersConfigsWarningShown = false; +/** Used to track whether hooks are being called because of a server restart or a server close event. */ +let restartingServer = false; let miniflare: Miniflare | undefined; /** @@ -76,8 +78,6 @@ let miniflare: Miniflare | undefined; export function cloudflare(pluginConfig: PluginConfig = {}): vite.Plugin[] { const ctx = new PluginContext(); let containerImageTagsSeen = new Set(); - /** Used to track whether hooks are being called because of a server restart or a server close event. */ - let restartingServer = false; return [ {