fix: setting triggers.crons:[] in Wrangler config should delete deployed cron schedules (#9266)

* test: tighten up metrics module mocking

Previously there were occasionally flakes where the module appeared to be unmocked when it was still supposed to be mocked.

* fix: setting triggers.crons:[] in Wrangler config should delete deployed cron schedules

Author: petebacondarwin

.changeset/shaky-planets-feel.md

@@ -0,0 +1,5 @@
+---
+"wrangler": patch
+---
+
+fix: setting triggers.crons:[] in Wrangler config should delete deployed cron schedules

packages/wrangler/src/__tests__/config/configuration.test.ts

@@ -114,7 +114,7 @@ describe("normalizeAndValidateConfig()", () => {
 			ai: undefined,
 			version_metadata: undefined,
 			triggers: {
-				crons: [],
+				crons: undefined,
 			},
 			unsafe: {
 				bindings: undefined,

packages/wrangler/src/__tests__/deploy.test.ts

@@ -2071,6 +2071,43 @@ Update them to point to this script instead?`,
 		it.todo("should error if it's a workers.dev route");
 	});
 
+	describe("triggers", () => {
+		it("should deploy the worker with a scheduled trigger", async () => {
+			const crons = ["*/5 * * * *", "0 18 * * 6L"];
+			writeWranglerConfig({
+				triggers: { crons },
+			});
+			writeWorkerSource();
+			mockSubDomainRequest();
+			mockUploadWorkerRequest({ expectedType: "esm" });
+			mockPublishSchedulesRequest({ crons });
+			await runWrangler("deploy ./index");
+		});
+
+		it("should deploy the worker with an empty array of scheduled triggers", async () => {
+			const crons: string[] = [];
+			writeWranglerConfig({
+				triggers: { crons },
+			});
+			writeWorkerSource();
+			mockSubDomainRequest();
+			mockUploadWorkerRequest({ expectedType: "esm" });
+			mockPublishSchedulesRequest({ crons });
+			await runWrangler("deploy ./index");
+		});
+
+		it.each([{ triggers: { crons: undefined } }, { triggers: undefined }, {}])(
+			"should deploy the worker without updating the scheduled triggers",
+			async (config) => {
+				writeWranglerConfig(config);
+				writeWorkerSource();
+				mockSubDomainRequest();
+				mockUploadWorkerRequest({ expectedType: "esm" });
+				await runWrangler("deploy ./index");
+			}
+		);
+	});
+
 	describe("entry-points", () => {
 		it("should be able to use `index` with no extension as the entry-point (esm)", async () => {
 			writeWranglerConfig();
@@ -12945,6 +12982,38 @@ function mockLastDeploymentRequest() {
 	msw.use(...mswSuccessDeploymentScriptMetadata);
 }
 
+function mockPublishSchedulesRequest({
+	crons = [],
+	env = undefined,
+	legacyEnv = false,
+}: {
+	crons: Config["triggers"]["crons"];
+	env?: string | undefined;
+	legacyEnv?: boolean | undefined;
+}) {
+	const servicesOrScripts = env && !legacyEnv ? "services" : "scripts";
+	const environment = env && !legacyEnv ? "/environments/:envName" : "";
+
+	msw.use(
+		http.put(
+			`*/accounts/:accountId/workers/${servicesOrScripts}/:scriptName${environment}/schedules`,
+			async ({ request, params }) => {
+				expect(params.accountId).toEqual("some-account-id");
+				expect(params.scriptName).toEqual(
+					legacyEnv && env ? `test-name-${env}` : "test-name"
+				);
+				if (!legacyEnv) {
+					expect(params.envName).toEqual(env);
+				}
+				const body = (await request.json()) as [{ cron: string }];
+				expect(body).toEqual(crons.map((cron) => ({ cron })));
+				return HttpResponse.json(createFetchResult(null));
+			},
+			{ once: true }
+		)
+	);
+}
+
 function mockPublishRoutesRequest({
 	routes = [],
 	env = undefined,

packages/wrangler/src/__tests__/metrics.test.ts

@@ -30,9 +30,9 @@ import { writeWranglerConfig } from "./helpers/write-wrangler-config";
 import type { MockInstance } from "vitest";
 
 vi.mock("../metrics/helpers");
-vi.unmock("../metrics/metrics-config");
 vi.mock("../metrics/send-event");
 vi.mock("../package-manager");
+vi.mocked(getMetricsConfig).mockReset();
 
 // eslint-disable-next-line @typescript-eslint/no-namespace
 declare module globalThis {
@@ -44,7 +44,7 @@ describe("metrics", () => {
 	let isCISpy: MockInstance;
 	const std = mockConsoleMethods();
 	const { setIsTTY } = useMockIsTTY();
-	runInTempDir();
+	runInTempDir({ homedir: "foo" });
 
 	beforeEach(async () => {
 		isCISpy = vi.spyOn(CI, "isCI").mockReturnValue(false);

packages/wrangler/src/__tests__/vitest.setup.ts

@@ -162,18 +162,16 @@ vi.mock("xdg-app-paths", () => {
 vi.mock("../metrics/metrics-config", async (importOriginal) => {
 	const realModule =
 		await importOriginal<typeof import("../metrics/metrics-config")>();
-	const fakeModule = {
-		...realModule,
-		getMetricsConfig: () => async () => {
-			return {
-				enabled: false,
-				deviceId: "mock-device",
-				userId: undefined,
-			};
-		},
-	};
-	return fakeModule;
+	vi.spyOn(realModule, "getMetricsConfig").mockImplementation(() => {
+		return {
+			enabled: false,
+			deviceId: "mock-device",
+			userId: undefined,
+		};
+	});
+	return realModule;
 });
+
 vi.mock("prompts", () => {
 	return {
 		__esModule: true,

packages/wrangler/src/config/config.ts

@@ -332,7 +332,7 @@ export const defaultWranglerConfig: Config = {
 	jsx_fragment: "React.Fragment",
 	migrations: [],
 	triggers: {
-		crons: [],
+		crons: undefined,
 	},
 	rules: [],
 	build: { command: undefined, watch_dir: "./src", cwd: undefined },

packages/wrangler/src/config/environment.ts

@@ -285,10 +285,10 @@ interface EnvironmentInheritable {
 	 *
 	 * For reference, see https://developers.cloudflare.com/workers/wrangler/configuration/#triggers
 	 *
-	 * @default {crons:[]}
+	 * @default {crons: undefined}
 	 * @inheritable
 	 */
-	triggers: { crons: string[] };
+	triggers: { crons: string[] | undefined };
 
 	/**
 	 * Specify limits for runtime behavior.

packages/wrangler/src/config/validation.ts

@@ -17,7 +17,6 @@ import {
 	inheritableInLegacyEnvironments,
 	isBoolean,
 	isMutuallyExclusiveWith,
-	isObjectWith,
 	isOptionalProperty,
 	isRequiredProperty,
 	isString,
@@ -1122,8 +1121,8 @@ function normalizeAndValidateEnvironment(
 			topLevelEnv,
 			rawEnv,
 			"triggers",
-			isObjectWith("crons"),
-			{ crons: [] }
+			validateTriggers,
+			{ crons: undefined }
 		),
 		assets: normalizeAndValidateAssets(diagnostics, topLevelEnv, rawEnv),
 		limits: normalizeAndValidateLimits(diagnostics, topLevelEnv, rawEnv),
@@ -1509,6 +1508,44 @@ const validateAndNormalizeRules = (
 	);
 };
 
+const validateTriggers: ValidatorFn = (
+	diagnostics,
+	triggersFieldName,
+	triggersValue
+) => {
+	if (triggersValue === undefined || triggersValue === null) {
+		return true;
+	}
+
+	if (typeof triggersValue !== "object") {
+		diagnostics.errors.push(
+			`Expected "${triggersFieldName}" to be of type object but got ${JSON.stringify(
+				triggersValue
+			)}.`
+		);
+		return false;
+	}
+
+	let isValid = true;
+
+	if ("crons" in triggersValue && !Array.isArray(triggersValue.crons)) {
+		diagnostics.errors.push(
+			`Expected "${triggersFieldName}.crons" to be of type array, but got ${JSON.stringify(triggersValue)}.`
+		);
+		isValid = false;
+	}
+
+	isValid =
+		validateAdditionalProperties(
+			diagnostics,
+			triggersFieldName,
+			Object.keys(triggersValue),
+			["crons"]
+		) && isValid;
+
+	return isValid;
+};
+
 const validateRules =
 	(envName: string): ValidatorFn =>
 	(diagnostics, field, envValue, config) => {

packages/wrangler/src/dev/miniflare.ts

@@ -1138,7 +1138,7 @@ export async function buildMiniflareOptions(
 	internalObjects: CfDurableObject[];
 	entrypointNames: string[];
 }> {
-	if (config.crons.length > 0 && !config.testScheduled) {
+	if (config.crons?.length && !config.testScheduled) {
 		if (!didWarnMiniflareCronSupport) {
 			didWarnMiniflareCronSupport = true;
 			logger.warn(

packages/wrangler/src/triggers/deploy.ts

@@ -38,7 +38,7 @@ export default async function triggersDeploy(
 ): Promise<string[] | void> {
 	const { config, accountId, name: scriptName } = props;
 
-	const triggers = props.triggers || config.triggers?.crons;
+	const schedules = props.triggers || config.triggers?.crons;
 	const routes =
 		props.routes ?? config.routes ?? (config.route ? [config.route] : []) ?? [];
 	const routesOnly: Array<Route> = [];
@@ -258,17 +258,18 @@ export default async function triggersDeploy(
 	}
 
 	// Configure any schedules for the script.
-	// TODO: rename this to `schedules`?
-	if (triggers && triggers.length) {
+	// If schedules is not defined then we just leave whatever is previously deployed alone.
+	// If it is an empty array we will remove all schedules.
+	if (schedules) {
 		deployments.push(
 			fetchResult(`${workerUrl}/schedules`, {
 				// Note: PUT will override previous schedules on this script.
 				method: "PUT",
-				body: JSON.stringify(triggers.map((cron) => ({ cron }))),
+				body: JSON.stringify(schedules.map((cron) => ({ cron }))),
 				headers: {
 					"Content-Type": "application/json",
 				},
-			}).then(() => triggers.map((trigger) => `schedule: ${trigger}`))
+			}).then(() => schedules.map((trigger) => `schedule: ${trigger}`))
 		);
 	}