chore: refactor wrangler login/logout/whoami to use defineCommand (#7150)

emily-shen · edmundhung · penalosa · web-flow · commit 6380d864d6c7 · 2024-11-05T13:03:00.000Z
* move whoami.ts into user/

* move wranger `login/logout/whoami` and use defineCommand

* Update packages/wrangler/src/index.ts

Co-authored-by: Somhairle MacLeòid &lt;smacleod@cloudflare.com&gt;

* make args optional

* disable warning for invalid config

* add changeset

* allow disabling readConfig warning with defineCommand

---------

Co-authored-by: Edmund Hung &lt;edmund@cloudflare.com&gt;
Co-authored-by: Somhairle MacLeòid &lt;smacleod@cloudflare.com&gt;
diff --git a/.changeset/good-eggs-lay.md b/.changeset/good-eggs-lay.md
@@ -0,0 +1,5 @@
+---
+"wrangler": patch
+---
+
+refactor: improve login/logout/whoami setup with the new internal registration utils
diff --git a/packages/wrangler/src/__tests__/sentry.test.ts b/packages/wrangler/src/__tests__/sentry.test.ts
@@ -259,18 +259,29 @@ describe("sentry", () => {
 				              Object {
 				                "colno": 0,
 				                "context_line": "",
-				                "filename": "/wrangler/packages/wrangler/src/index.ts",
+				                "filename": "/wrangler/packages/wrangler/src/core/register-commands.ts",
 				                "function": "",
 				                "in_app": false,
 				                "lineno": 0,
-				                "module": "index.ts",
+				                "module": "register-commands.ts",
+				                "post_context": Array [],
+				                "pre_context": Array [],
+				              },
+				              Object {
+				                "colno": 0,
+				                "context_line": "",
+				                "filename": "/wrangler/packages/wrangler/src/user/commands.ts",
+				                "function": "",
+				                "in_app": false,
+				                "lineno": 0,
+				                "module": "commands.ts",
 				                "post_context": Array [],
 				                "pre_context": Array [],
 				              },
 				              Object {
 				                "colno": 0,
 				                "context_line": "",
-				                "filename": "/wrangler/packages/wrangler/src/whoami.ts",
+				                "filename": "/wrangler/packages/wrangler/src/user/whoami.ts",
 				                "function": "",
 				                "in_app": false,
 				                "lineno": 0,
@@ -281,7 +292,7 @@ describe("sentry", () => {
 				              Object {
 				                "colno": 0,
 				                "context_line": "",
-				                "filename": "/wrangler/packages/wrangler/src/whoami.ts",
+				                "filename": "/wrangler/packages/wrangler/src/user/whoami.ts",
 				                "function": "",
 				                "in_app": false,
 				                "lineno": 0,
@@ -292,7 +303,7 @@ describe("sentry", () => {
 				              Object {
 				                "colno": 0,
 				                "context_line": "",
-				                "filename": "/wrangler/packages/wrangler/src/whoami.ts",
+				                "filename": "/wrangler/packages/wrangler/src/user/whoami.ts",
 				                "function": "",
 				                "in_app": false,
 				                "lineno": 0,
diff --git a/packages/wrangler/src/__tests__/whoami.test.ts b/packages/wrangler/src/__tests__/whoami.test.ts
@@ -1,6 +1,6 @@
 import { http, HttpResponse } from "msw";
 import { writeAuthConfigFile } from "../user";
-import { getUserInfo } from "../whoami";
+import { getUserInfo } from "../user/whoami";
 import { mockConsoleMethods } from "./helpers/mock-console";
 import { useMockIsTTY } from "./helpers/mock-istty";
 import {
diff --git a/packages/wrangler/src/core/define-command.ts b/packages/wrangler/src/core/define-command.ts
@@ -101,13 +101,20 @@ export type CommandDefinition<
 		 * @default true
 		 */
 		printBanner?: boolean;
+
+		/**
+		 * By default, wrangler will print warnings about wrangler.toml configuration.
+		 * Set this value to `false` to skip printing these warnings.
+		 */
+		printConfigWarnings?: boolean;
 	};
 
 	/**
 	 * A plain key-value object describing the CLI args for this command.
 	 * Shared args can be defined as another plain object and spread into this.
 	 */
-	args: NamedArgDefs;
+	args?: NamedArgDefs;
+
 	/**
 	 * Optionally declare some of the named args as positional args.
 	 * The order of this array is the order they are expected in the command.
diff --git a/packages/wrangler/src/core/register-commands.ts b/packages/wrangler/src/core/register-commands.ts
@@ -106,7 +106,7 @@ function walkTreeAndRegister(
 		// inference from positionalArgs
 		const commandPositionalArgsSuffix = def.positionalArgs
 			?.map((key) => {
-				const { demandOption, array } = def.args[key];
+				const { demandOption, array } = def.args?.[key] ?? {};
 				return demandOption
 					? `<${key}${array ? ".." : ""}>` // <key> or <key..>
 					: `[${key}${array ? ".." : ""}]`; // [key] or [key..]
@@ -124,21 +124,23 @@ function walkTreeAndRegister(
 		(def.metadata.hidden ? false : def.metadata.description) as string, // cast to satisfy typescript overload selection
 		function builder(subYargs) {
 			if (def.type === "command") {
-				yargs.options(def.args);
+				const args = def.args ?? {};
+
+				yargs.options(args);
 
 				// Yargs offers an `array: true` option that will always coerces the value to an array
 				// e.g. `--name foo` becomes `{ name: ["foo"] }` instead of `{ name: "foo" }`
 				// However, non-array arguments can still receives multiple values
 				// e.g. `--name foo --name bar` becomes `{ name: ["foo", "bar"] }` regardless of the `array` setting
 				// @see https://github.com/yargs/yargs/issues/1318
-				for (const [key, opt] of Object.entries(def.args)) {
+				for (const [key, opt] of Object.entries(args)) {
 					if (!opt.array) {
 						yargs.check(demandSingleValue(key));
 					}
 				}
 
 				for (const key of def.positionalArgs ?? []) {
-					yargs.positional(key, def.args[key]);
+					yargs.positional(key, args[key]);
 				}
 			} else if (def.type === "namespace") {
 				// this is our hacky way of printing --help text for incomplete commands
@@ -180,7 +182,12 @@ function createHandler(def: CommandDefinition) {
 			await def.validateArgs?.(args);
 
 			await def.handler(args, {
-				config: readConfig(args.config, args),
+				config: readConfig(
+					args.config,
+					args,
+					undefined,
+					!(def.behaviour?.printConfigWarnings ?? true)
+				),
 				errors: { UserError, FatalError },
 				logger,
 				fetchResult,
diff --git a/packages/wrangler/src/index.ts b/packages/wrangler/src/index.ts
@@ -7,7 +7,7 @@ import makeCLI from "yargs";
 import { version as wranglerVersion } from "../package.json";
 import { ai } from "./ai";
 import { cloudchamber } from "./cloudchamber";
-import { loadDotEnv, readConfig } from "./config";
+import { loadDotEnv } from "./config";
 import { createCommandRegister } from "./core/register-commands";
 import { d1 } from "./d1";
 import { deleteHandler, deleteOptions } from "./delete";
@@ -48,9 +48,9 @@ import { hyperdrive } from "./hyperdrive/index";
 import { initHandler, initOptions } from "./init";
 import "./kv";
 import "./workflows";
+import "./user/commands";
 import { demandSingleValue } from "./core";
 import { logBuildFailure, logger, LOGGER_LEVELS } from "./logger";
-import * as metrics from "./metrics";
 import { mTlsCertificateCommands } from "./mtls-certificate/cli";
 import { writeOutput } from "./output";
 import { pages } from "./pages";
@@ -70,19 +70,13 @@ import { tailHandler, tailOptions } from "./tail";
 import registerTriggersSubcommands from "./triggers";
 import { typesHandler, typesOptions } from "./type-generation";
 import { printWranglerBanner, updateCheck } from "./update-check";
-import {
-	getAuthFromEnv,
-	listScopes,
-	login,
-	logout,
-	validateScopeKeys,
-} from "./user";
+import { getAuthFromEnv } from "./user";
+import { whoami } from "./user/whoami";
 import { debugLogFilepath } from "./utils/log-file";
 import { vectorize } from "./vectorize/index";
 import registerVersionsSubcommands from "./versions";
 import registerVersionsDeploymentsSubcommands from "./versions/deployments";
 import registerVersionsRollbackCommand from "./versions/rollback";
-import { whoami } from "./whoami";
 import { asJson } from "./yargs-types";
 import type { Config } from "./config";
 import type { LoggerLevel } from "./logger";
@@ -596,99 +590,10 @@ export function createCLIParser(argv: string[]) {
 	});
 
 	/******************** CMD GROUP ***********************/
-	// login
-	wrangler.command(
-		// this needs scopes as an option?
-		"login",
-		"🔓 Login to Cloudflare",
-		(yargs) => {
-			// TODO: This needs some copy editing
-			// I mean, this entire app does, but this too.
-			return yargs
-				.option("scopes-list", {
-					describe: "List all the available OAuth scopes with descriptions",
-				})
-				.option("browser", {
-					default: true,
-					type: "boolean",
-					describe: "Automatically open the OAuth link in a browser",
-				})
-				.option("scopes", {
-					describe: "Pick the set of applicable OAuth scopes when logging in",
-					array: true,
-					type: "string",
-					requiresArg: true,
-				});
-			// TODO: scopes
-		},
-		async (args) => {
-			await printWranglerBanner();
-			if (args["scopes-list"]) {
-				listScopes();
-				return;
-			}
-			if (args.scopes) {
-				if (args.scopes.length === 0) {
-					// don't allow no scopes to be passed, that would be weird
-					listScopes();
-					return;
-				}
-				if (!validateScopeKeys(args.scopes)) {
-					throw new CommandLineArgsError(
-						`One of ${args.scopes} is not a valid authentication scope. Run "wrangler login --scopes-list" to see the valid scopes.`
-					);
-				}
-				await login({ scopes: args.scopes, browser: args.browser });
-				return;
-			}
-			await login({ browser: args.browser });
-			const config = readConfig(args.config, args, undefined, true);
-			await metrics.sendMetricsEvent("login user", {
-				sendMetrics: config.send_metrics,
-			});
-
-			// TODO: would be nice if it optionally saved login
-			// credentials inside node_modules/.cache or something
-			// this way you could have multiple users on a single machine
-		}
-	);
 
-	// logout
-	wrangler.command(
-		// this needs scopes as an option?
-		"logout",
-		"🚪 Logout from Cloudflare",
-		() => {},
-		async (args) => {
-			await printWranglerBanner();
-			await logout();
-			const config = readConfig(undefined, args, undefined, true);
-			await metrics.sendMetricsEvent("logout user", {
-				sendMetrics: config.send_metrics,
-			});
-		}
-	);
-
-	// whoami
-	wrangler.command(
-		"whoami",
-		"🕵️  Retrieve your user information",
-		(yargs) => {
-			return yargs.option("account", {
-				type: "string",
-				describe:
-					"Show membership information for the given account (id or name).",
-			});
-		},
-		async (args) => {
-			await printWranglerBanner();
-			await whoami(args.account);
-			const config = readConfig(undefined, args);
-			await metrics.sendMetricsEvent("view accounts", {
-				sendMetrics: config.send_metrics,
-			});
-		}
-	);
+	register.registerNamespace("login");
+	register.registerNamespace("logout");
+	register.registerNamespace("whoami");
 
 	/******************************************************/
 	/*               DEPRECATED COMMANDS                  */
diff --git a/packages/wrangler/src/user/commands.ts b/packages/wrangler/src/user/commands.ts
@@ -0,0 +1,104 @@
+import { defineCommand } from "../core/define-command";
+import { CommandLineArgsError } from "../errors";
+import * as metrics from "../metrics";
+import { listScopes, login, logout, validateScopeKeys } from "./user";
+import { whoami } from "./whoami";
+
+defineCommand({
+	command: "wrangler login",
+	metadata: {
+		description: "🔓 Login to Cloudflare",
+		owner: "Workers: Authoring and Testing",
+		status: "stable",
+	},
+	behaviour: {
+		printConfigWarnings: false,
+	},
+	args: {
+		"scopes-list": {
+			describe: "List all the available OAuth scopes with descriptions",
+		},
+		browser: {
+			default: true,
+			type: "boolean",
+			describe: "Automatically open the OAuth link in a browser",
+		},
+		scopes: {
+			describe: "Pick the set of applicable OAuth scopes when logging in",
+			array: true,
+			type: "string",
+			requiresArg: true,
+		},
+	},
+	async handler(args, { config }) {
+		if (args.scopesList) {
+			listScopes();
+			return;
+		}
+		if (args.scopes) {
+			if (args.scopes.length === 0) {
+				// don't allow no scopes to be passed, that would be weird
+				listScopes();
+				return;
+			}
+			if (!validateScopeKeys(args.scopes)) {
+				throw new CommandLineArgsError(
+					`One of ${args.scopes} is not a valid authentication scope. Run "wrangler login --scopes-list" to see the valid scopes.`
+				);
+			}
+			await login({ scopes: args.scopes, browser: args.browser });
+			return;
+		}
+		await login({ browser: args.browser });
+		await metrics.sendMetricsEvent("login user", {
+			sendMetrics: config.send_metrics,
+		});
+
+		// TODO: would be nice if it optionally saved login
+		// credentials inside node_modules/.cache or something
+		// this way you could have multiple users on a single machine
+	},
+});
+
+defineCommand({
+	command: "wrangler logout",
+	metadata: {
+		description: "🚪 Logout from Cloudflare",
+		owner: "Workers: Authoring and Testing",
+		status: "stable",
+	},
+	behaviour: {
+		printConfigWarnings: false,
+	},
+	async handler(_, { config }) {
+		await logout();
+		await metrics.sendMetricsEvent("logout user", {
+			sendMetrics: config.send_metrics,
+		});
+	},
+});
+
+defineCommand({
+	command: "wrangler whoami",
+	metadata: {
+		description: "🕵️  Retrieve your user information",
+		owner: "Workers: Authoring and Testing",
+		status: "stable",
+	},
+	behaviour: {
+		printConfigWarnings: false,
+	},
+	args: {
+		account: {
+			type: "string",
+			describe:
+				"Show membership information for the given account (id or name).",
+		},
+	},
+	async handler(args, { config }) {
+		await whoami(args.account);
+		await metrics.sendMetricsEvent("view accounts", {
+			sendMetrics: config.send_metrics,
+		});
+	},
+});
diff --git a/packages/wrangler/src/user/whoami.ts b/packages/wrangler/src/user/whoami.ts
