From cc0fd7c36a3eecc38851ec4095f02242c2187054 Mon Sep 17 00:00:00 2001 From: Daniel Rivas Date: Wed, 12 Jun 2024 15:32:05 -0500 Subject: [PATCH] fix: wrangler deploy prompts warning with deployment styling Styling of the warning about multiple deployments during `wrangler deploy` should be consistent with `wrangler versions` styling. --- packages/cli/index.ts | 3 +- .../wrangler/src/__tests__/deploy.test.ts | 33 +------- .../versions/versions.deploy.test.ts | 41 ++++++++++ packages/wrangler/src/versions/deploy.ts | 78 +++++++++---------- 4 files changed, 83 insertions(+), 72 deletions(-) diff --git a/packages/cli/index.ts b/packages/cli/index.ts index 35be0d3dfcfa..a941bbebd8f4 100644 --- a/packages/cli/index.ts +++ b/packages/cli/index.ts @@ -163,13 +163,14 @@ export const warn = ( shape = shapes.corners.bl, // current default for backcompat -- TODO: change default to true once all callees have been updated multiline = false, + newlineBefore = true, } = {} ) => { logRaw( format(msg, { firstLinePrefix: gray(shape) + space() + status.warning, linePrefix: gray(shapes.bar), - newlineBefore: true, + newlineBefore: newlineBefore, formatLine: (line) => dim(line), // for backcompat but it's not ideal for this to be "dim" multiline, }) diff --git a/packages/wrangler/src/__tests__/deploy.test.ts b/packages/wrangler/src/__tests__/deploy.test.ts index a9c731a14ba2..a1112e0c1eed 100644 --- a/packages/wrangler/src/__tests__/deploy.test.ts +++ b/packages/wrangler/src/__tests__/deploy.test.ts @@ -43,10 +43,7 @@ import { mswSuccessOauthHandlers, mswSuccessUserHandlers, } from "./helpers/msw"; -import { - mswListNewDeploymentsLatestFiftyFifty, - mswListNewDeploymentsLatestFull, -} from "./helpers/msw/handlers/versions"; +import { mswListNewDeploymentsLatestFull } from "./helpers/msw/handlers/versions"; import { runInTempDir } from "./helpers/run-in-tmp"; import { runWrangler } from "./helpers/run-wrangler"; import { writeWorkerSource } from "./helpers/write-worker-source"; @@ -428,34 +425,6 @@ describe("deploy", () => { `); }); - it("should warn user when worker has deployment with multiple versions", async () => { - msw.use(...mswListNewDeploymentsLatestFiftyFifty); - writeWranglerToml(); - writeWorkerSource(); - mockSubDomainRequest(); - mockUploadWorkerRequest(); - mockConfirm({ - text: `"wrangler deploy" will upload a new version and deploy it globally immediately.\nAre you sure you want to continue?`, - result: false, - }); - - await runWrangler("deploy ./index"); - - expect(std.warn).toMatchInlineSnapshot(` - "▲ [WARNING] Your last deployment has multiple versions. To progress this deployment use \\"wrangler versions deploy\\" instead. - - Currently deployed versions: - - Version(s): (50%) test-name:version:0 - Created: 2021-01-01T00:00:00.000Z - - (50%) test-name:version:1 - Created: 2021-01-01T00:00:00.000Z - - " - `); - }); - it("should warn user when additional properties are passed to a services config", async () => { writeWranglerToml({ d1_databases: [ diff --git a/packages/wrangler/src/__tests__/versions/versions.deploy.test.ts b/packages/wrangler/src/__tests__/versions/versions.deploy.test.ts index 1891bc04f1de..cb337ddf9b50 100644 --- a/packages/wrangler/src/__tests__/versions/versions.deploy.test.ts +++ b/packages/wrangler/src/__tests__/versions/versions.deploy.test.ts @@ -11,6 +11,8 @@ import { collectCLIOutput } from "../helpers/collect-cli-output"; import { mockAccountId, mockApiToken } from "../helpers/mock-account-id"; import { mockConsoleMethods } from "../helpers/mock-console"; import { useMockIsTTY } from "../helpers/mock-istty"; +import { mockUploadWorkerRequest } from "../helpers/mock-upload-worker"; +import { mockSubDomainRequest } from "../helpers/mock-workers-subdomain"; import { msw, mswGetVersion, @@ -18,9 +20,12 @@ import { mswListVersions, mswPatchNonVersionedScriptSettings, mswPostNewDeployment, + mswSuccessDeploymentScriptMetadata, } from "../helpers/msw"; +import { mswListNewDeploymentsLatestFiftyFifty } from "../helpers/msw/handlers/versions"; import { runInTempDir } from "../helpers/run-in-tmp"; import { runWrangler } from "../helpers/run-wrangler"; +import { writeWorkerSource } from "../helpers/write-worker-source"; import writeWranglerToml from "../helpers/write-wrangler-toml"; import type { VersionsDeployArgs } from "../../versions/deploy"; @@ -43,6 +48,42 @@ describe("versions deploy", () => { ); }); + describe("legacy deploy", () => { + test("should warn user when worker has deployment with multiple versions", async () => { + msw.use( + ...mswSuccessDeploymentScriptMetadata, + ...mswListNewDeploymentsLatestFiftyFifty + ); + writeWranglerToml(); + writeWorkerSource(); + mockSubDomainRequest(); + mockUploadWorkerRequest(); + + await runWrangler("deploy ./index"); + + expect(normalizeOutput(std.out)).toMatchInlineSnapshot(` + "╭ WARNING Your last deployment has multiple versions. To progress this deployment use \\"wrangler versions deploy\\" instead. + │ + ├ Your last deployment has 2 version(s): + │ + │ (50%) test-name:version:0 + │ Created: TIMESTAMP + │ Tag: - + │ Message: - + │ + │ (50%) test-name:version:1 + │ Created: TIMESTAMP + │ Tag: - + │ Message: - + │ + ├ \\"wrangler deploy\\" will upload a new version and deploy it globally immediately. + Are you sure you want to continue? + │ yes + │" + `); + }); + }); + describe("without wrangler.toml", () => { test("succeeds with --name arg", async () => { const result = runWrangler( diff --git a/packages/wrangler/src/versions/deploy.ts b/packages/wrangler/src/versions/deploy.ts index 722d6083473d..e2aa903b97c6 100644 --- a/packages/wrangler/src/versions/deploy.ts +++ b/packages/wrangler/src/versions/deploy.ts @@ -11,6 +11,8 @@ import { import { findWranglerToml, readConfig } from "../config"; import { confirm } from "../dialogs"; import { UserError } from "../errors"; +import { CI } from "../is-ci"; +import isInteractive from "../is-interactive"; import { logger } from "../logger"; import * as metrics from "../metrics"; import { APIError } from "../parse"; @@ -225,42 +227,35 @@ export async function confirmLatestDeploymentOverwrite( const versionIds = latest.versions.map((v) => v.version_id); await fetchVersions(accountId, scriptName, versionCache, ...versionIds); - // Format each version. - - const formattedVersions = latest.versions.map((traffic) => { - const version = versionCache.get(traffic.version_id); + const versions = latest.versions.map((v) => { + const version = versionCache.get(v.version_id); assert(version); - const percentage = brandColor(`(${traffic.percentage}%)`); - const details = formatLabelledValues( - { Created: new Date(version.metadata["created_on"]).toISOString() }, - { - indentationCount: 4, - labelJustification: "right", - formatLabel: (label) => gray(label + ":"), - formatValue: (value) => gray(value), - } - ); - return `${percentage} ${version.id}\n${details}`; + return version; }); - // Format deployment. - - const formattedDeployment = formatLabelledValues({ - "Version(s)": formattedVersions.join("\n\n"), - }); + const traffic = new Map( + latest.versions.map((v) => [v.version_id, v.percentage]) + ); // Print message and confirmation. - logger.warn( - `Your last deployment has multiple versions. To progress this deployment use "wrangler versions deploy" instead. -Currently deployed versions: - -${formattedDeployment}` + cli.warn( + `Your last deployment has multiple versions. To progress this deployment use "wrangler versions deploy" instead.`, + { shape: cli.shapes.corners.tl, newlineBefore: false } ); - - return await confirm( - `"wrangler deploy" will upload a new version and deploy it globally immediately.\nAre you sure you want to continue?` + cli.newline(); + cli.logRaw( + `${leftT} Your last deployment has ${versions.length} version(s):` ); + printVersions(versions, traffic); + + return inputPrompt({ + type: "confirm", + question: `"wrangler deploy" will upload a new version and deploy it globally immediately.\nAre you sure you want to continue?`, + label: "", + defaultValue: true, // defaults to true to keep current behavior. + acceptDefault: !isInteractive() || CI.isCI(), + }); } } catch (e) { const isNotFound = e instanceof APIError && e.code == 10007; @@ -294,20 +289,25 @@ export function printVersions( versions: ApiVersion[], traffic: Map ) { - for (const version of versions) { - const trafficString = brandColor(`(${traffic.get(version.id)}%)`); - const versionIdString = white(version.id); + cli.newline(); + cli.log(formatVersions(versions, traffic)); + cli.newline(); +} - cli.log( - gray(` -${trafficString} ${versionIdString} +export function formatVersions( + versions: ApiVersion[], + traffic: Map +) { + return versions + .map((version) => { + const trafficString = brandColor(`(${traffic.get(version.id)}%)`); + const versionIdString = white(version.id); + return gray(`${trafficString} ${versionIdString} Created: ${version.metadata.created_on} Tag: ${version.annotations?.["workers/tag"] ?? BLANK_INPUT} - Message: ${version.annotations?.["workers/message"] ?? BLANK_INPUT}`) - ); - } - - cli.newline(); + Message: ${version.annotations?.["workers/message"] ?? BLANK_INPUT}`); + }) + .join("\n\n"); } /**