refactor

packages/wrangler/src/__tests__/core/command-registration.test.ts

@@ -1,9 +1,10 @@
 import { normalizeOutput } from "../../../e2e/helpers/normalize";
 import {
-	COMMAND_DEFINITIONS,
 	defineAlias,
 	defineCommand,
 	defineNamespace,
+	DefinitionTreeNode,
+	DefinitionTreeRoot,
 } from "../../core/define-command";
 import { mockConsoleMethods } from "../helpers/mock-console";
 import { runInTempDir } from "../helpers/run-in-tmp";
@@ -13,18 +14,14 @@ describe("Command Registration", () => {
 	runInTempDir();
 	const std = mockConsoleMethods();
 
-	let originalDefinitions: typeof COMMAND_DEFINITIONS = [];
+	let originalDefinitions: [string, DefinitionTreeNode][];
 	beforeAll(() => {
-		originalDefinitions = COMMAND_DEFINITIONS.slice();
+		originalDefinitions = [...DefinitionTreeRoot.subtree.entries()];
 	});
 
 	beforeEach(() => {
 		// resets the commands definitions so the tests do not conflict with eachother
-		COMMAND_DEFINITIONS.splice(
-			0,
-			COMMAND_DEFINITIONS.length,
-			...originalDefinitions
-		);
+		DefinitionTreeRoot.subtree = new Map(originalDefinitions);
 
 		// To make these tests less verbose, we will define
 		// a bunch of commands that *use* all features
@@ -165,9 +162,6 @@ describe("Command Registration", () => {
 	test("displays commands in top-level --help", async () => {
 		await runWrangler("--help");
 
-		// TODO: fix ordering in top-level --help output
-		//     The current ordering is hackily built on top of yargs default output
-		//     This abstraction will enable us to completely customise the --help output
 		expect(std.out).toMatchInlineSnapshot(`
 			"wrangler
 
@@ -331,9 +325,6 @@ describe("Command Registration", () => {
 		defineAlias({
 			command: "wrangler my-test-alias",
 			aliasOf: "wrangler my-test-command",
-			metadata: {
-				hidden: false,
-			},
 		});
 
 		await runWrangler("my-test-alias --help");
@@ -342,7 +333,9 @@ describe("Command Registration", () => {
 		expect(std.out).toMatchInlineSnapshot(`
 			"wrangler my-test-alias [pos] [posNum]
 
-			Alias for \\"wrangler my-test-command\\". My test command
+			My test command
+
+			Alias for \\"wrangler my-test-command\\".
 
 			POSITIONALS
 			  pos  [string]
@@ -383,7 +376,7 @@ describe("Command Registration", () => {
 
 		expect(std.out).toMatchInlineSnapshot(`"Ran command"`);
 		expect(normalizeOutput(std.warn)).toMatchInlineSnapshot(
-			`"▲ [WARNING] 🚧 \`wrangler alpha-command\` is a alpha command. Please report any issues to https://github.com/cloudflare/workers-sdk/issues/new/choose"`
+			`"▲ [WARNING] 🚧 \`wrangler alpha-command\` is an alpha command. Please report any issues to https://github.com/cloudflare/workers-sdk/issues/new/choose"`
 		);
 	});
 	test("auto log deprecation message", async () => {
@@ -436,36 +429,32 @@ describe("Command Registration", () => {
 
 	describe("registration errors", () => {
 		test("throws upon duplicate command definition", async () => {
-			defineCommand({
-				command: "wrangler my-test-command",
-				metadata: {
-					description: "",
-					owner: "Workers: Authoring and Testing",
-					status: "stable",
-				},
-				args: {},
-				handler() {},
-			});
-
-			await expect(
-				runWrangler("my-test-command")
-			).rejects.toMatchInlineSnapshot(
+			await expect(() => {
+				defineCommand({
+					command: "wrangler my-test-command",
+					metadata: {
+						description: "",
+						owner: "Workers: Authoring and Testing",
+						status: "stable",
+					},
+					args: {},
+					handler() {},
+				});
+			}).toThrowErrorMatchingInlineSnapshot(
 				`[Error: Duplicate definition for "wrangler my-test-command"]`
 			);
 		});
 		test("throws upon duplicate namespace definition", async () => {
-			defineNamespace({
-				command: "wrangler one two",
-				metadata: {
-					description: "",
-					owner: "Workers: Authoring and Testing",
-					status: "stable",
-				},
-			});
-
-			await expect(
-				runWrangler("my-test-command")
-			).rejects.toMatchInlineSnapshot(
+			await expect(() => {
+				defineNamespace({
+					command: "wrangler one two",
+					metadata: {
+						description: "",
+						owner: "Workers: Authoring and Testing",
+						status: "stable",
+					},
+				});
+			}).toThrowErrorMatchingInlineSnapshot(
 				`[Error: Duplicate definition for "wrangler one two"]`
 			);
 		});
@@ -505,7 +494,7 @@ describe("Command Registration", () => {
 			await expect(
 				runWrangler("my-test-command")
 			).rejects.toMatchInlineSnapshot(
-				`[Error: Alias of alias encountered greater than 5 hops]`
+				`[Error: Missing definition for "wrangler undefined-command" (resolving from "wrangler my-alias-command")]`
 			);
 		});
 	});

packages/wrangler/src/core/define-command.ts

@@ -1,3 +1,4 @@
+import type { fetchResult } from "../cfetch";
 import type { Config } from "../config";
 import type { OnlyCamelCase } from "../config/config";
 import type { FatalError, UserError } from "../errors";
@@ -12,6 +13,13 @@ import type {
 	PositionalOptions,
 } from "yargs";
 
+export class CommandRegistrationError extends Error {}
+
+type StringKeyOf<T> = Extract<keyof T, string>;
+export type DeepFlatten<T> = T extends object
+	? { [K in keyof T]: DeepFlatten<T[K]> }
+	: T;
+
 export type Command = `wrangler${string}`;
 export type Metadata = {
 	description: string;
@@ -23,12 +31,10 @@ export type Metadata = {
 	owner: Teams;
 };
 
-export type ArgDefinition = PositionalOptions & Pick<Options, "hidden">;
-export type BaseNamedArgDefinitions = {
-	[key: string]: ArgDefinition;
-};
-type StringKeyOf<T> = Extract<keyof T, string>;
-export type HandlerArgs<Args extends BaseNamedArgDefinitions> = DeepFlatten<
+export type ArgDefinition = PositionalOptions &
+	Pick<Options, "hidden" | "requiresArg">;
+export type NamedArgDefinitions = { [key: string]: ArgDefinition };
+export type HandlerArgs<Args extends NamedArgDefinitions> = DeepFlatten<
 	OnlyCamelCase<
 		RemoveIndex<
 			ArgumentsCamelCase<
@@ -38,10 +44,6 @@ export type HandlerArgs<Args extends BaseNamedArgDefinitions> = DeepFlatten<
 	>
 >;
 
-type DeepFlatten<T> = T extends object
-	? { [K in keyof T]: DeepFlatten<T[K]> }
-	: T;
-
 export type HandlerContext = {
 	/**
 	 * The wrangler config file read from disk and parsed.
@@ -51,6 +53,10 @@ export type HandlerContext = {
 	 * The logger instance provided to the command implementor as a convenience.
 	 */
 	logger: Logger;
+	/**
+	 * Use fetchResult to make *auth'd* requests to the Cloudflare API.
+	 */
+	fetchResult: typeof fetchResult;
 	/**
 	 * Error classes provided to the command implementor as a convenience
 	 * to aid discoverability and to encourage their usage.
@@ -70,7 +76,7 @@ export type HandlerContext = {
 };
 
 export type CommandDefinition<
-	NamedArgs extends BaseNamedArgDefinitions = BaseNamedArgDefinitions,
+	NamedArgDefs extends NamedArgDefinitions = NamedArgDefinitions,
 > = {
 	/**
 	 * The full command as it would be written by the user.
@@ -101,47 +107,43 @@ export type CommandDefinition<
 	 * 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: NamedArgs;
+	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.
 	 * Use args[key].demandOption and args[key].array to declare required and variadic
 	 * positional args, respectively.
 	 */
-	positionalArgs?: Array<StringKeyOf<NamedArgs>>;
+	positionalArgs?: Array<StringKeyOf<NamedArgDefs>>;
 
 	/**
 	 * A hook to implement custom validation of the args before the handler is called.
 	 * Throw `CommandLineArgsError` with actionable error message if args are invalid.
 	 * The return value is ignored.
 	 */
-	validateArgs?: (args: HandlerArgs<NamedArgs>) => void | Promise<void>;
+	validateArgs?: (args: HandlerArgs<NamedArgDefs>) => void | Promise<void>;
 
 	/**
 	 * The implementation of the command which is given camelCase'd args
 	 * and a ctx object of convenience properties
 	 */
 	handler: (
-		args: HandlerArgs<NamedArgs>,
+		args: HandlerArgs<NamedArgDefs>,
 		ctx: HandlerContext
 	) => void | Promise<void>;
 };
 
-export const COMMAND_DEFINITIONS: Array<
-	CommandDefinition | NamespaceDefinition | AliasDefinition
-> = [];
-
-type DefineCommandResult<NamedArgs extends BaseNamedArgDefinitions> =
+type DefineCommandResult<NamedArgDefs extends NamedArgDefinitions> =
 	DeepFlatten<{
-		args: HandlerArgs<NamedArgs>; // used for type inference only
+		args: HandlerArgs<NamedArgDefs>; // used for type inference only
 	}>;
-export function defineCommand<NamedArgs extends BaseNamedArgDefinitions>(
-	definition: CommandDefinition<NamedArgs>
-): DefineCommandResult<NamedArgs>;
+export function defineCommand<NamedArgDefs extends NamedArgDefinitions>(
+	definition: CommandDefinition<NamedArgDefs>
+): DefineCommandResult<NamedArgDefs>;
 export function defineCommand(
 	definition: CommandDefinition
-): DefineCommandResult<BaseNamedArgDefinitions> {
-	COMMAND_DEFINITIONS.push(definition as unknown as CommandDefinition);
+): DefineCommandResult<NamedArgDefinitions> {
+	upsertDefinition({ type: "command", ...definition });
 
 	// @ts-expect-error return type is used for type inference only
 	return {};
@@ -152,7 +154,7 @@ export type NamespaceDefinition = {
 	metadata: Metadata;
 };
 export function defineNamespace(definition: NamespaceDefinition) {
-	COMMAND_DEFINITIONS.push(definition);
+	upsertDefinition({ type: "namespace", ...definition });
 }
 
 export type AliasDefinition = {
@@ -161,5 +163,45 @@ export type AliasDefinition = {
 	metadata?: Partial<Metadata>;
 };
 export function defineAlias(definition: AliasDefinition) {
-	COMMAND_DEFINITIONS.push(definition);
+	upsertDefinition({ type: "alias", ...definition });
+}
+
+export type InternalDefinition =
+	| ({ type: "command" } & CommandDefinition)
+	| ({ type: "namespace" } & NamespaceDefinition)
+	| ({ type: "alias" } & AliasDefinition);
+export type DefinitionTreeNode = {
+	definition?: InternalDefinition;
+	subtree: DefinitionTree;
+};
+export type DefinitionTree = Map<string, DefinitionTreeNode>;
+
+export const DefinitionTreeRoot: DefinitionTreeNode = { subtree: new Map() };
+function upsertDefinition(def: InternalDefinition, root = DefinitionTreeRoot) {
+	const segments = def.command.split(" ").slice(1); // eg. ["versions", "secret", "put"]
+
+	let node = root;
+	for (const segment of segments) {
+		const subtree = node.subtree;
+		let child = subtree.get(segment);
+		if (!child) {
+			child = {
+				definition: undefined,
+				subtree: new Map(),
+			};
+			subtree.set(segment, child);
+		}
+
+		node = child;
+	}
+
+	if (node.definition) {
+		throw new CommandRegistrationError(
+			`Duplicate definition for "${def.command}"`
+		);
+	}
+
+	node.definition = def;
+
+	return node;
 }

packages/wrangler/src/core/index.ts

@@ -0,0 +1 @@
+export { defineAlias, defineCommand, defineNamespace } from "./define-command";