From 201b60e1dd25b4abb7670e21d103b67d4eda0e14 Mon Sep 17 00:00:00 2001 From: Jan Krems Date: Thu, 5 Sep 2024 14:57:19 -0700 Subject: [PATCH] feat(@angular/cli): handle string key/value pairs, e.g. --define --- packages/angular/cli/BUILD.bazel | 1 + .../cli/src/command-builder/command-module.ts | 72 +----- .../command-builder/utilities/json-schema.ts | 155 +++++++++++- .../utilities/json-schema_spec.ts | 221 ++++++++++++++++++ 4 files changed, 382 insertions(+), 67 deletions(-) create mode 100644 packages/angular/cli/src/command-builder/utilities/json-schema_spec.ts diff --git a/packages/angular/cli/BUILD.bazel b/packages/angular/cli/BUILD.bazel index bfdcaca10e98..3eafc989623f 100644 --- a/packages/angular/cli/BUILD.bazel +++ b/packages/angular/cli/BUILD.bazel @@ -147,6 +147,7 @@ ts_library( "//packages/angular_devkit/schematics", "//packages/angular_devkit/schematics/testing", "@npm//@types/semver", + "@npm//@types/yargs", ], ) diff --git a/packages/angular/cli/src/command-builder/command-module.ts b/packages/angular/cli/src/command-builder/command-module.ts index e608c4b1d089..d80da2948ca4 100644 --- a/packages/angular/cli/src/command-builder/command-module.ts +++ b/packages/angular/cli/src/command-builder/command-module.ts @@ -26,7 +26,7 @@ import { considerSettingUpAutocompletion } from '../utilities/completion'; import { AngularWorkspace } from '../utilities/config'; import { memoize } from '../utilities/memoize'; import { PackageManagerUtils } from '../utilities/package-manager'; -import { Option } from './utilities/json-schema'; +import { Option, addSchemaOptionsToCommand } from './utilities/json-schema'; export type Options = { [key in keyof T as CamelCaseKey]: T[key] }; @@ -188,68 +188,16 @@ export abstract class CommandModule implements CommandModuleI * **Note:** This method should be called from the command bundler method. */ protected addSchemaOptionsToCommand(localYargs: Argv, options: Option[]): Argv { - const booleanOptionsWithNoPrefix = new Set(); - - for (const option of options) { - const { - default: defaultVal, - positional, - deprecated, - description, - alias, - userAnalytics, - type, - hidden, - name, - choices, - } = option; - - const sharedOptions: YargsOptions & PositionalOptions = { - alias, - hidden, - description, - deprecated, - choices, - // This should only be done when `--help` is used otherwise default will override options set in angular.json. - ...(this.context.args.options.help ? { default: defaultVal } : {}), - }; - - let dashedName = strings.dasherize(name); - - // Handle options which have been defined in the schema with `no` prefix. - if (type === 'boolean' && dashedName.startsWith('no-')) { - dashedName = dashedName.slice(3); - booleanOptionsWithNoPrefix.add(dashedName); - } - - if (positional === undefined) { - localYargs = localYargs.option(dashedName, { - type, - ...sharedOptions, - }); - } else { - localYargs = localYargs.positional(dashedName, { - type: type === 'array' || type === 'count' ? 'string' : type, - ...sharedOptions, - }); - } - - // Record option of analytics. - if (userAnalytics !== undefined) { - this.optionsWithAnalytics.set(name, userAnalytics); - } - } + const optionsWithAnalytics = addSchemaOptionsToCommand( + localYargs, + options, + // This should only be done when `--help` is used otherwise default will override options set in angular.json. + /* includeDefaultValues= */ this.context.args.options.help, + ); - // Handle options which have been defined in the schema with `no` prefix. - if (booleanOptionsWithNoPrefix.size) { - localYargs.middleware((options: Arguments) => { - for (const key of booleanOptionsWithNoPrefix) { - if (key in options) { - options[`no-${key}`] = !options[key]; - delete options[key]; - } - } - }, false); + // Record option of analytics. + for (const [name, userAnalytics] of optionsWithAnalytics) { + this.optionsWithAnalytics.set(name, userAnalytics); } return localYargs; diff --git a/packages/angular/cli/src/command-builder/utilities/json-schema.ts b/packages/angular/cli/src/command-builder/utilities/json-schema.ts index 2b17c1eb0226..a46b06646197 100644 --- a/packages/angular/cli/src/command-builder/utilities/json-schema.ts +++ b/packages/angular/cli/src/command-builder/utilities/json-schema.ts @@ -6,8 +6,8 @@ * found in the LICENSE file at https://angular.dev/license */ -import { json } from '@angular-devkit/core'; -import yargs from 'yargs'; +import { json, strings } from '@angular-devkit/core'; +import yargs, { Arguments, Argv, PositionalOptions, Options as YargsOptions } from 'yargs'; /** * An option description. @@ -43,6 +43,55 @@ export interface Option extends yargs.Options { * If this is falsey, do not report this option. */ userAnalytics?: string; + + /** + * Type of the values in a key/value pair field. + */ + itemValueType?: 'string'; +} + +function coerceToStringMap( + dashedName: string, + value: (string | undefined)[], +): Record | Promise { + const stringMap: Record = {}; + for (const pair of value) { + // This happens when the flag isn't passed at all. + if (pair === undefined) { + continue; + } + + const eqIdx = pair.indexOf('='); + if (eqIdx === -1) { + // TODO: Remove workaround once yargs properly handles thrown errors from coerce. + // Right now these sometimes end up as uncaught exceptions instead of proper validation + // errors with usage output. + return Promise.reject( + new Error( + `Invalid value for argument: ${dashedName}, Given: '${pair}', Expected key=value pair`, + ), + ); + } + const key = pair.slice(0, eqIdx); + const value = pair.slice(eqIdx + 1); + stringMap[key] = value; + } + + return stringMap; +} + +function isStringMap(node: json.JsonObject): boolean { + // Exclude fields with more specific kinds of properties. + if (node.properties || node.patternProperties) { + return false; + } + + // Restrict to additionalProperties with string values. + return ( + json.isJsonObject(node.additionalProperties) && + !node.additionalProperties.enum && + node.additionalProperties.type === 'string' + ); } export async function parseJsonSchemaToOptions( @@ -106,10 +155,13 @@ export async function parseJsonSchemaToOptions( return false; + case 'object': + return isStringMap(current); + default: return false; } - }) as ('string' | 'number' | 'boolean' | 'array')[]; + }) as ('string' | 'number' | 'boolean' | 'array' | 'object')[]; if (types.length == 0) { // This means it's not usable on the command line. e.g. an Object. @@ -150,7 +202,6 @@ export async function parseJsonSchemaToOptions( } } - const type = types[0]; const $default = current.$default; const $defaultIndex = json.isJsonObject($default) && $default['$source'] == 'argv' ? $default['index'] : undefined; @@ -182,7 +233,6 @@ export async function parseJsonSchemaToOptions( const option: Option = { name, description: '' + (current.description === undefined ? '' : current.description), - type, default: defaultValue, choices: enumValues.length ? enumValues : undefined, required, @@ -192,6 +242,14 @@ export async function parseJsonSchemaToOptions( userAnalytics, deprecated, positional, + ...(types[0] === 'object' + ? { + type: 'array', + itemValueType: 'string', + } + : { + type: types[0], + }), }; options.push(option); @@ -211,3 +269,90 @@ export async function parseJsonSchemaToOptions( return a.name.localeCompare(b.name); }); } + +/** + * Adds schema options to a command also this keeps track of options that are required for analytics. + * **Note:** This method should be called from the command bundler method. + * + * @returns A map from option name to analytics configuration. + */ +export function addSchemaOptionsToCommand( + localYargs: Argv, + options: Option[], + includeDefaultValues: boolean, +): Map { + const booleanOptionsWithNoPrefix = new Set(); + const keyValuePairOptions = new Set(); + const optionsWithAnalytics = new Map(); + + for (const option of options) { + const { + default: defaultVal, + positional, + deprecated, + description, + alias, + userAnalytics, + type, + itemValueType, + hidden, + name, + choices, + } = option; + + let dashedName = strings.dasherize(name); + + // Handle options which have been defined in the schema with `no` prefix. + if (type === 'boolean' && dashedName.startsWith('no-')) { + dashedName = dashedName.slice(3); + booleanOptionsWithNoPrefix.add(dashedName); + } + + if (itemValueType) { + keyValuePairOptions.add(name); + } + + const sharedOptions: YargsOptions & PositionalOptions = { + alias, + hidden, + description, + deprecated, + choices, + coerce: itemValueType ? coerceToStringMap.bind(null, dashedName) : undefined, + // This should only be done when `--help` is used otherwise default will override options set in angular.json. + ...(includeDefaultValues ? { default: defaultVal } : {}), + }; + + if (positional === undefined) { + localYargs = localYargs.option(dashedName, { + array: itemValueType ? true : undefined, + type: itemValueType ?? type, + ...sharedOptions, + }); + } else { + localYargs = localYargs.positional(dashedName, { + type: type === 'array' || type === 'count' ? 'string' : type, + ...sharedOptions, + }); + } + + // Record option of analytics. + if (userAnalytics !== undefined) { + optionsWithAnalytics.set(name, userAnalytics); + } + } + + // Handle options which have been defined in the schema with `no` prefix. + if (booleanOptionsWithNoPrefix.size) { + localYargs.middleware((options: Arguments) => { + for (const key of booleanOptionsWithNoPrefix) { + if (key in options) { + options[`no-${key}`] = !options[key]; + delete options[key]; + } + } + }, false); + } + + return optionsWithAnalytics; +} diff --git a/packages/angular/cli/src/command-builder/utilities/json-schema_spec.ts b/packages/angular/cli/src/command-builder/utilities/json-schema_spec.ts new file mode 100644 index 000000000000..5ec5db644bef --- /dev/null +++ b/packages/angular/cli/src/command-builder/utilities/json-schema_spec.ts @@ -0,0 +1,221 @@ +/** + * @license + * Copyright Google LLC All Rights Reserved. + * + * Use of this source code is governed by an MIT-style license that can be + * found in the LICENSE file at https://angular.dev/license + */ + +import { json, schema } from '@angular-devkit/core'; +import yargs, { positional } from 'yargs'; + +import { addSchemaOptionsToCommand, parseJsonSchemaToOptions } from './json-schema'; + +const YError = (() => { + try { + const y = yargs().strict().fail(false).exitProcess(false).parse(['--forced-failure']); + } catch (e) { + if (!(e instanceof Error)) { + throw new Error('Unexpected non-Error thrown'); + } + + return e.constructor as typeof Error; + } + throw new Error('Expected parse to fail'); +})(); + +interface ParseFunction { + (argv: string[]): unknown; +} + +function withParseForSchema( + jsonSchema: json.JsonObject, + { + interactive = true, + includeDefaultValues = true, + }: { interactive?: boolean; includeDefaultValues?: boolean } = {}, +): ParseFunction { + let actualParse: ParseFunction = () => { + throw new Error('Called before init'); + }; + const parse: ParseFunction = (args) => { + return actualParse(args); + }; + + beforeEach(async () => { + const registry = new schema.CoreSchemaRegistry(); + const options = await parseJsonSchemaToOptions(registry, jsonSchema, interactive); + + actualParse = async (args: string[]) => { + // Create a fresh yargs for each call. The yargs object is stateful and + // calling .parse multiple times on the same instance isn't safe. + const localYargs = yargs().exitProcess(false).strict().fail(false); + addSchemaOptionsToCommand(localYargs, options, includeDefaultValues); + + // Yargs only exposes the parse errors as proper errors when using the + // callback syntax. This unwraps that ugly workaround so tests can just + // use simple .toThrow/.toEqual assertions. + return localYargs.parseAsync(args); + }; + }); + + return parse; +} + +describe('parseJsonSchemaToOptions', () => { + describe('without required fields in schema', () => { + const parse = withParseForSchema({ + 'type': 'object', + 'properties': { + 'maxSize': { + 'type': 'number', + }, + 'ssr': { + 'type': 'string', + 'enum': ['always', 'surprise-me', 'never'], + }, + 'extendable': { + 'type': 'object', + 'properties': {}, + 'additionalProperties': { + 'type': 'string', + }, + }, + 'someDefine': { + 'type': 'object', + 'additionalProperties': { + 'type': 'string', + }, + }, + }, + }); + + describe('type=number', () => { + it('parses valid option value', async () => { + expect(await parse(['--max-size', '42'])).toEqual( + jasmine.objectContaining({ + 'maxSize': 42, + }), + ); + }); + }); + + describe('type=string, enum', () => { + it('parses valid option value', async () => { + expect(await parse(['--ssr', 'never'])).toEqual( + jasmine.objectContaining({ + 'ssr': 'never', + }), + ); + }); + + it('rejects non-enum values', async () => { + await expectAsync(parse(['--ssr', 'yes'])).toBeRejectedWithError( + /Argument: ssr, Given: "yes", Choices:/, + ); + }); + }); + + describe('type=object', () => { + it('ignores fields that define specific properties', async () => { + await expectAsync(parse(['--extendable', 'a=b'])).toBeRejectedWithError( + /Unknown argument: extendable/, + ); + }); + + it('rejects invalid values for string maps', async () => { + await expectAsync(parse(['--some-define', 'foo'])).toBeRejectedWithError( + YError, + /Invalid value for argument: some-define, Given: 'foo', Expected key=value pair/, + ); + await expectAsync(parse(['--some-define', '42'])).toBeRejectedWithError( + YError, + /Invalid value for argument: some-define, Given: '42', Expected key=value pair/, + ); + }); + + it('aggregates an object value', async () => { + expect( + await parse([ + '--some-define', + 'A_BOOLEAN=true', + '--some-define', + 'AN_INTEGER=42', + // Ensure we can handle '=' inside of string values. + '--some-define=A_STRING="❤️=❤️"', + '--some-define', + 'AN_UNQUOTED_STRING=❤️=❤️', + ]), + ).toEqual( + jasmine.objectContaining({ + 'someDefine': { + 'A_BOOLEAN': 'true', + 'AN_INTEGER': '42', + 'A_STRING': '"❤️=❤️"', + 'AN_UNQUOTED_STRING': '❤️=❤️', + }, + }), + ); + }); + }); + }); + + describe('with required positional argument', () => { + it('marks the required argument as required', async () => { + const jsonSchema = JSON.parse(` + { + "$id": "FakeSchema", + "title": "Fake Schema", + "type": "object", + "required": ["a"], + "properties": { + "b": { + "type": "string", + "description": "b.", + "$default": { + "$source": "argv", + "index": 1 + } + }, + "a": { + "type": "string", + "description": "a.", + "$default": { + "$source": "argv", + "index": 0 + } + }, + "optC": { + "type": "string", + "description": "optC" + }, + "optA": { + "type": "string", + "description": "optA" + }, + "optB": { + "type": "string", + "description": "optB" + } + } + }`) as json.JsonObject; + const registry = new schema.CoreSchemaRegistry(); + const options = await parseJsonSchemaToOptions(registry, jsonSchema, /* interactive= */ true); + + expect(options.find((opt) => opt.name === 'a')).toEqual( + jasmine.objectContaining({ + name: 'a', + positional: 0, + required: true, + }), + ); + expect(options.find((opt) => opt.name === 'b')).toEqual( + jasmine.objectContaining({ + name: 'b', + positional: 1, + required: false, + }), + ); + }); + }); +});