diff --git a/.changeset/shaky-planets-feel.md b/.changeset/shaky-planets-feel.md new file mode 100644 index 000000000000..8a4d84c74895 --- /dev/null +++ b/.changeset/shaky-planets-feel.md @@ -0,0 +1,5 @@ +--- +"wrangler": patch +--- + +fix: setting triggers.crons:[] in Wrangler config should delete deployed cron schedules diff --git a/packages/wrangler/src/__tests__/config/configuration.test.ts b/packages/wrangler/src/__tests__/config/configuration.test.ts index a81925536905..a2a5314e27c2 100644 --- a/packages/wrangler/src/__tests__/config/configuration.test.ts +++ b/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, diff --git a/packages/wrangler/src/__tests__/deploy.test.ts b/packages/wrangler/src/__tests__/deploy.test.ts index 1951018ef270..29daccfdaf94 100644 --- a/packages/wrangler/src/__tests__/deploy.test.ts +++ b/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, diff --git a/packages/wrangler/src/__tests__/metrics.test.ts b/packages/wrangler/src/__tests__/metrics.test.ts index cd33ff8001f6..c9e77b05db98 100644 --- a/packages/wrangler/src/__tests__/metrics.test.ts +++ b/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); diff --git a/packages/wrangler/src/__tests__/vitest.setup.ts b/packages/wrangler/src/__tests__/vitest.setup.ts index 276d7a8ea266..f41bf4db8b1b 100644 --- a/packages/wrangler/src/__tests__/vitest.setup.ts +++ b/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(); - 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, diff --git a/packages/wrangler/src/config/config.ts b/packages/wrangler/src/config/config.ts index f9452866e27e..9220d633154a 100644 --- a/packages/wrangler/src/config/config.ts +++ b/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 }, diff --git a/packages/wrangler/src/config/environment.ts b/packages/wrangler/src/config/environment.ts index 9a78dd9cc58f..6dd2b45fb69a 100644 --- a/packages/wrangler/src/config/environment.ts +++ b/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. diff --git a/packages/wrangler/src/config/validation.ts b/packages/wrangler/src/config/validation.ts index da890672252a..7b54b334fd75 100644 --- a/packages/wrangler/src/config/validation.ts +++ b/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) => { diff --git a/packages/wrangler/src/dev/miniflare.ts b/packages/wrangler/src/dev/miniflare.ts index 4ec155aa6834..cf8cd1b3c4c2 100644 --- a/packages/wrangler/src/dev/miniflare.ts +++ b/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( diff --git a/packages/wrangler/src/triggers/deploy.ts b/packages/wrangler/src/triggers/deploy.ts index 02c51e20ef94..93e920aea4a0 100644 --- a/packages/wrangler/src/triggers/deploy.ts +++ b/packages/wrangler/src/triggers/deploy.ts @@ -38,7 +38,7 @@ export default async function triggersDeploy( ): Promise { 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 = []; @@ -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}`)) ); }