diff --git a/.changeset/dry-run-project-setup.md b/.changeset/dry-run-project-setup.md new file mode 100644 index 000000000..1bc7e5981 --- /dev/null +++ b/.changeset/dry-run-project-setup.md @@ -0,0 +1,7 @@ +--- +"cf": patch +--- + +Avoid project setup changes during deploy dry runs + +Pass `--dry-run` to framework setup and Wrangler config conversion for `cf deploy`, `cf workers versions create`, and `cf workers triggers deploy`. If setup is needed, show the planned changes and skip the build and upload. diff --git a/packages/cli/src/__tests__/commands/deploy/index.test.ts b/packages/cli/src/__tests__/commands/deploy/index.test.ts index df4bdbd99..31d72ff14 100644 --- a/packages/cli/src/__tests__/commands/deploy/index.test.ts +++ b/packages/cli/src/__tests__/commands/deploy/index.test.ts @@ -1,3 +1,4 @@ +import { existsSync, readFileSync } from "node:fs"; import { mockConsoleMethods, runInTempDir, @@ -1137,6 +1138,42 @@ describe("cf deploy", () => { }); describe("--dry-run", () => { + it("keeps existing project files unchanged and skips the build when setup is needed", async () => { + const packageJson = JSON.stringify({ + name: "astro-project", + scripts: { deploy: "astro build && wrangler deploy" }, + dependencies: { + astro: "7.3.5", + "@astrojs/cloudflare": "14.3.3", + }, + devDependencies: { cf: "1.0.0-beta.6", wrangler: "^4.142.0" }, + }); + const tsconfig = '{"include":["src/**/*"]}\n'; + const lockfile = "existing lockfile\n"; + const requests = recordRequests(); + await seed({ + "package.json": packageJson, + "package-lock.json": lockfile, + "tsconfig.json": tsconfig, + "wrangler.jsonc": '{"name":"astro-project"}', + "node_modules/astro/package.json": JSON.stringify({ + name: "astro", + version: "7.3.5", + }), + }); + + const { exitCode } = await runCf(["deploy", "--dry-run"]); + + expect(exitCode).toBe(0); + expect(readFileSync("package.json", "utf8")).toBe(packageJson); + expect(readFileSync("package-lock.json", "utf8")).toBe(lockfile); + expect(readFileSync("tsconfig.json", "utf8")).toBe(tsconfig); + expect(existsSync("public/.assetsignore")).toBe(false); + expect(buildDelegateWasCalled()).toBe(false); + expect(requests).toEqual([]); + expect(std.out).toContain("Autoconfig process run in dry-run mode"); + }); + it("does not upload the worker", async () => { const upload = mockWorkerUpload(); await seed({ diff --git a/packages/cli/src/__tests__/commands/migrate.test.ts b/packages/cli/src/__tests__/commands/migrate.test.ts index 566b9c459..aca73ae74 100644 --- a/packages/cli/src/__tests__/commands/migrate.test.ts +++ b/packages/cli/src/__tests__/commands/migrate.test.ts @@ -165,7 +165,7 @@ describe("cf migrate", () => { expect(migrateWranglerToCf).not.toHaveBeenCalled(); }); - it("offers to run the same migration for project workflows", async () => { + it("offers to convert a Wrangler config before project setup", async () => { await seed({ "wrangler.jsonc": "{}" }); const confirmMigration = vi.fn().mockResolvedValue(true); @@ -191,6 +191,31 @@ describe("cf migrate", () => { ); }); + it("dry-runs an accepted Wrangler config conversion before project setup", async () => { + await seed({ "wrangler.jsonc": "{}" }); + const confirmMigration = vi.fn().mockResolvedValue(true); + + await expect( + maybeMigrateWranglerProject( + process.cwd(), + confirmMigration, + "stdout", + true + ) + ).resolves.toBe(true); + + expect(migrateWranglerToCf).toHaveBeenCalledWith( + path.join(process.cwd(), "wrangler.jsonc"), + { + bundler: "wrangler", + dryRun: true, + force: false, + installDependencies: true, + } + ); + expect(migrateWranglerToCf).toHaveBeenCalledOnce(); + }); + it("uses Vite for automatic migration when the plugin is declared", async () => { await seed({ "config/package.json": JSON.stringify({ diff --git a/packages/cli/src/__tests__/commands/triggers-deploy.test.ts b/packages/cli/src/__tests__/commands/triggers-deploy.test.ts index 47b6bae48..f0f3b8ea2 100644 --- a/packages/cli/src/__tests__/commands/triggers-deploy.test.ts +++ b/packages/cli/src/__tests__/commands/triggers-deploy.test.ts @@ -83,6 +83,27 @@ describe("cf workers triggers deploy", () => { expect(requests).toEqual([]); }); + it("skips the build and trigger deployment when setup is needed during a dry run", async () => { + const requests = recordRequests(); + await seed({ + "package.json": JSON.stringify({ + name: "astro-project", + dependencies: { astro: "7.3.5" }, + }), + "node_modules/astro/package.json": JSON.stringify({ + name: "astro", + version: "7.3.5", + }), + }); + + const { exitCode } = await runCf([...TRIGGERS_DEPLOY_COMMAND, "--dry-run"]); + + expect(exitCode).toBe(0); + expect(buildDelegateWasCalled()).toBe(false); + expect(requests).toEqual([]); + expect(std.out).toContain("Autoconfig process run in dry-run mode"); + }); + it("builds and deploys scheduled triggers with --local=false", async () => { let schedulesBody: unknown; msw.use( diff --git a/packages/cli/src/__tests__/lib/autoconfig-migration.test.ts b/packages/cli/src/__tests__/lib/autoconfig-migration.test.ts index 1180b7eaa..03dfcca01 100644 --- a/packages/cli/src/__tests__/lib/autoconfig-migration.test.ts +++ b/packages/cli/src/__tests__/lib/autoconfig-migration.test.ts @@ -42,6 +42,19 @@ describe("project preparation", () => { expect(mocks.maybeMigrateWranglerProject).not.toHaveBeenCalled(); }); + it("does not run setup for a configured project during a dry run", async () => { + const configuredDetails = { ...unconfiguredDetails, configured: true }; + mocks.getDetailsForAutoConfig.mockResolvedValue(configuredDetails); + + await expect(prepareProject("/project", { dryRun: true })).resolves.toEqual( + { + details: configuredDetails, + } + ); + expect(mocks.maybeMigrateWranglerProject).not.toHaveBeenCalled(); + expect(mocks.runAutoConfig).not.toHaveBeenCalled(); + }); + it("offers migration even when autoconfig cannot analyze the legacy project", async () => { const configuredDetails = { ...unconfiguredDetails, @@ -67,7 +80,7 @@ describe("project preparation", () => { expect(mocks.runAutoConfig).not.toHaveBeenCalled(); }); - it("runs autoconfig when migration is unavailable or declined", async () => { + it("runs framework setup when Wrangler config conversion did not run", async () => { const configuration = { scripts: {}, outputDir: "dist", @@ -83,4 +96,47 @@ describe("project preparation", () => { }); expect(mocks.runAutoConfig).toHaveBeenCalledOnce(); }); + + it("dry-runs framework setup when Wrangler config conversion did not run", async () => { + mocks.getDetailsForAutoConfig.mockResolvedValue(unconfiguredDetails); + mocks.maybeMigrateWranglerProject.mockResolvedValue(false); + mocks.runAutoConfig.mockResolvedValue({ buildCommand: "npm run build" }); + + await expect( + prepareProject("/project", { dryRun: true }) + ).resolves.toMatchObject({ + details: unconfiguredDetails, + setupNeeded: true, + }); + expect(mocks.runAutoConfig).toHaveBeenCalledWith( + unconfiguredDetails, + expect.objectContaining({ dryRun: true, runBuild: false }) + ); + expect(mocks.maybeMigrateWranglerProject).toHaveBeenCalledWith( + "/project", + expect.any(Function), + undefined, + true + ); + }); + + it("stops after an accepted Wrangler config conversion dry run", async () => { + mocks.getDetailsForAutoConfig.mockResolvedValue(unconfiguredDetails); + mocks.maybeMigrateWranglerProject.mockResolvedValue(true); + + await expect(prepareProject("/project", { dryRun: true })).resolves.toEqual( + { + details: unconfiguredDetails, + setupNeeded: true, + } + ); + expect(mocks.maybeMigrateWranglerProject).toHaveBeenCalledWith( + "/project", + expect.any(Function), + undefined, + true + ); + expect(mocks.getDetailsForAutoConfig).toHaveBeenCalledOnce(); + expect(mocks.runAutoConfig).not.toHaveBeenCalled(); + }); }); diff --git a/packages/cli/src/commands/build/index.ts b/packages/cli/src/commands/build/index.ts index 85c7cf1c6..13a1f8607 100644 --- a/packages/cli/src/commands/build/index.ts +++ b/packages/cli/src/commands/build/index.ts @@ -24,16 +24,25 @@ interface RunBuildOptions extends CommandOutputOptions { // Validate the Worker the caller will consume, so an invalid default // Worker cannot block a workflow that selected another one. worker?: string; + dryRun?: boolean; } export async function runBuild( mode?: string, - { worker: selectedWorker, ...options }: RunBuildOptions = {}, + { worker: selectedWorker, dryRun = false, ...options }: RunBuildOptions = {}, ctx: { isPreview?: boolean } = {} -): Promise { +): Promise<"built" | "setup-needed"> { const output = options.output ?? "stdout"; const cwd = process.cwd(); - const { details, configuration } = await prepareProject(cwd, options); + const { details, configuration, setupNeeded } = await prepareProject(cwd, { + ...options, + dryRun, + }); + if (setupNeeded) { + // A Wrangler config conversion or framework setup was dry-run, so the + // configuration the build needs has not been written yet. + return "setup-needed"; + } const buildCommand = configuration?.buildCommand ?? details?.buildCommand; const env: Record = { ...details?.env, @@ -91,6 +100,7 @@ export async function runBuild( output: output === "stderr" ? process.stderr : undefined, }); } + return "built"; } function formatImplName(discovered: DiscoveredImpl): string { diff --git a/packages/cli/src/commands/deploy/shared.ts b/packages/cli/src/commands/deploy/shared.ts index cb796e21f..3922897cf 100644 --- a/packages/cli/src/commands/deploy/shared.ts +++ b/packages/cli/src/commands/deploy/shared.ts @@ -93,7 +93,14 @@ type UploadArgs = SharedUploadArgs & { export async function runUpload(argv: UploadArgs, ctx: UploadCommand) { // Delegate the build before applying cf's dotenv values. if (!argv.prebuilt) { - await runBuild(argv.mode, { worker: argv.worker }); + const build = await runBuild(argv.mode, { + worker: argv.worker, + dryRun: argv["dry-run"], + }); + if (build === "setup-needed") { + clack.log.success("--dry-run: exiting now."); + return; + } clack.log.message("", { spacing: 0 }); } @@ -213,7 +220,7 @@ async function uploadBuildOutput(argv: UploadArgs, ctx: UploadCommand) { ); } - clack.log.success( - argv["dry-run"] ? "Dry run complete" : `${ctx.command} complete` - ); + if (!argv["dry-run"]) { + clack.log.success(`${ctx.command} complete`); + } } diff --git a/packages/cli/src/commands/workers/triggers/deploy.ts b/packages/cli/src/commands/workers/triggers/deploy.ts index d8ddb15de..bb5f66e91 100644 --- a/packages/cli/src/commands/workers/triggers/deploy.ts +++ b/packages/cli/src/commands/workers/triggers/deploy.ts @@ -56,7 +56,14 @@ const triggersDeployCommand: CommandModule< } if (!argv.prebuilt) { - await runBuild(argv.mode, { worker: argv.worker }); + const build = await runBuild(argv.mode, { + worker: argv.worker, + dryRun: argv["dry-run"], + }); + if (build === "setup-needed") { + clack.log.success("--dry-run: exiting now."); + return; + } clack.log.message("", { spacing: 0 }); } @@ -100,9 +107,9 @@ async function deployTriggers(argv: TriggersDeployArgs): Promise { await triggersDeploy( createTriggerProps(worker, wranglerConfig, accountId, argv) ); - clack.log.success( - argv["dry-run"] ? "Dry run complete" : "Trigger deploy complete" - ); + if (!argv["dry-run"]) { + clack.log.success("Trigger deploy complete"); + } } export default triggersDeployCommand; diff --git a/packages/cli/src/lib/autoconfig.ts b/packages/cli/src/lib/autoconfig.ts index 1059262e2..aacd1b107 100644 --- a/packages/cli/src/lib/autoconfig.ts +++ b/packages/cli/src/lib/autoconfig.ts @@ -24,6 +24,10 @@ export interface CommandOutputOptions { output?: CommandOutput; } +interface ProjectPreparationOptions extends CommandOutputOptions { + dryRun?: boolean; +} + export interface RunProjectCommandOptions extends CommandOutputOptions { env?: Readonly>; args?: readonly string[]; @@ -77,23 +81,34 @@ export async function analyzeProject( } } +/** + * Run framework autoconfig after detection, applying or dry-running setup + * without building. + */ export async function configureProject( details: AutoConfigDetails, - options: CommandOutputOptions = {} + options: ProjectPreparationOptions = {} ): Promise { return runAutoConfig(details, { target: "cf", context: createAutoConfigContext(options), + dryRun: options.dryRun, runBuild: false, }); } +/** + * Choose project setup before a build: an accepted Wrangler config conversion + * takes precedence over framework setup. In a dry run, `setupNeeded` means + * a setup route was selected but not applied, so callers must skip the build. + */ export async function prepareProject( cwd: string, - options: CommandOutputOptions = {} + options: ProjectPreparationOptions = {} ): Promise<{ details: AutoConfigDetails | undefined; configuration?: AutoConfigSummary; + setupNeeded?: true; }> { let details = await analyzeProject(cwd, options); if (details?.configured) { @@ -101,13 +116,16 @@ export async function prepareProject( } const context = createAutoConfigContext(options); - if ( - await maybeMigrateWranglerProject( - cwd, - (text, confirmOptions) => context.dialogs.confirm(text, confirmOptions), - options.output - ) - ) { + const migrationRan = await maybeMigrateWranglerProject( + cwd, + (text, confirmOptions) => context.dialogs.confirm(text, confirmOptions), + options.output, + options.dryRun + ); + if (migrationRan) { + if (options.dryRun) { + return { details, setupNeeded: true }; + } details = await analyzeProject(cwd, options); return { details }; } @@ -115,7 +133,10 @@ export async function prepareProject( return { details, ...(details - ? { configuration: await configureProject(details, options) } + ? { + configuration: await configureProject(details, options), + ...(options.dryRun ? { setupNeeded: true as const } : {}), + } : {}), }; } diff --git a/packages/cli/src/lib/wrangler-migration.ts b/packages/cli/src/lib/wrangler-migration.ts index c132e8638..cc2d29665 100644 --- a/packages/cli/src/lib/wrangler-migration.ts +++ b/packages/cli/src/lib/wrangler-migration.ts @@ -192,10 +192,16 @@ export async function runWranglerMigration( } } +/** + * Offer to convert a Wrangler config before framework setup. Returns true + * only after the user accepts and the conversion command completes, including + * when it runs in dry-run mode. + */ export async function maybeMigrateWranglerProject( projectPath: string, confirmMigration: ConfirmMigration, - output: MigrationOutput = "stdout" + output: MigrationOutput = "stdout", + dryRun = false ): Promise { const configPath = await findWranglerConfig(projectPath, { failOnMultiple: false, @@ -218,6 +224,6 @@ export async function maybeMigrateWranglerProject( } const bundler = await detectWranglerMigrationBundler(configPath); - await runWranglerMigration(configPath, { bundler, output }); + await runWranglerMigration(configPath, { bundler, output, dryRun }); return true; }