diff --git a/.changeset/good-eggs-lay.md b/.changeset/good-eggs-lay.md new file mode 100644 index 000000000000..14b95bea3128 --- /dev/null +++ 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 index b866a6ad1eb5..a902d393e9b4 100644 --- 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 index 8fd2dde791cd..966b378139e4 100644 --- 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 index 16ccd1b1d1ea..6ad2a472bcda 100644 --- 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 index 76921508da49..1dfdf191e926 100644 --- 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 ? ".." : ""}>` // or : `[${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 index b81d82dafb8e..e71445acde35 100644 --- 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 new file mode 100644 index 000000000000..c3546b01983c --- /dev/null +++ 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/whoami.ts b/packages/wrangler/src/user/whoami.ts similarity index 94% rename from packages/wrangler/src/whoami.ts rename to packages/wrangler/src/user/whoami.ts index e9e5d9c0cac1..d9fcf533beaa 100644 --- a/packages/wrangler/src/whoami.ts +++ b/packages/wrangler/src/user/whoami.ts @@ -1,9 +1,9 @@ import chalk from "chalk"; -import { fetchPagedListResult, fetchResult } from "./cfetch"; -import { isAuthenticationError } from "./deploy/deploy"; -import { logger } from "./logger"; -import { getAPIToken, getAuthFromEnv, getScopes } from "./user"; -import { fetchMembershipRoles } from "./user/membership"; +import { fetchPagedListResult, fetchResult } from "../cfetch"; +import { isAuthenticationError } from "../deploy/deploy"; +import { logger } from "../logger"; +import { fetchMembershipRoles } from "./membership"; +import { getAPIToken, getAuthFromEnv, getScopes } from "."; export async function whoami(accountFilter?: string) { logger.log("Getting User settings...");