feat(@angular/cli): handle string key/value pairs, e.g. --define

Author: jkrems

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",
     ],
 )
 

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<T> = { [key in keyof T as CamelCaseKey<key>]: T[key] };
 
@@ -188,68 +188,16 @@ export abstract class CommandModule<T extends {} = {}> implements CommandModuleI
    * **Note:** This method should be called from the command bundler method.
    */
   protected addSchemaOptionsToCommand<T>(localYargs: Argv<T>, options: Option[]): Argv<T> {
-    const booleanOptionsWithNoPrefix = new Set<string>();
-
-    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;

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<string, string> | Promise<never> {
+  const stringMap: Record<string, string> = {};
+  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<T>(
+  localYargs: Argv<T>,
+  options: Option[],
+  includeDefaultValues: boolean,
+): Map<string, string> {
+  const booleanOptionsWithNoPrefix = new Set<string>();
+  const keyValuePairOptions = new Set<string>();
+  const optionsWithAnalytics = new Map<string, string>();
+
+  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;
+}

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,
+        }),
+      );
+    });
+  });
+});