fix(@angular/cli): apply default to array types

alan-agius4 · alan-agius4 · commit ce212777543c · 2025-08-18T11:46:59.000Z
This commit fixes an issue where the `default` option was not being applied to `array` type options in yargs.

This seemingly minor change required refactoring in some tests, which revealed that a `.coerce` validation was incorrectly throwing an error on failure. The validation logic was moved to a `.check` to ensure proper error handling and prevent unexpected failures.
diff --git a/packages/angular/cli/src/command-builder/utilities/json-schema.ts b/packages/angular/cli/src/command-builder/utilities/json-schema.ts
@@ -51,10 +51,33 @@ export interface Option extends YargsOptions {
   itemValueType?: 'string';
 }
 
+function checkStringMap(keyValuePairOptions: Set<string>, args: Arguments): boolean {
+  for (const key of keyValuePairOptions) {
+    const value = args[key];
+    if (!Array.isArray(value)) {
+      // Value has been parsed.
+      continue;
+    }
+
+    for (const pair of value) {
+      if (pair === undefined) {
+        continue;
+      }
+
+      if (!pair.includes('=')) {
+        throw new Error(
+          `Invalid value for argument: ${key}, Given: '${pair}', Expected key=value pair`,
+        );
+      }
+    }
+  }
+
+  return true;
+}
+
 function coerceToStringMap(
-  dashedName: string,
   value: (string | undefined)[],
-): Record<string, string> | Promise<never> {
+): Record<string, string> | (string | undefined)[] {
   const stringMap: Record<string, string> = {};
   for (const pair of value) {
     // This happens when the flag isn't passed at all.
@@ -64,18 +87,12 @@ function coerceToStringMap(
 
     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`,
-        ),
-      );
+      // In the case it is not valid skip processing this option and handle the error in `checkStringMap`
+      return value;
     }
+
     const key = pair.slice(0, eqIdx);
-    const value = pair.slice(eqIdx + 1);
-    stringMap[key] = value;
+    stringMap[key] = pair.slice(eqIdx + 1);
   }
 
   return stringMap;
@@ -184,6 +201,7 @@ export async function parseJsonSchemaToOptions(
     if (current.default !== undefined) {
       switch (types[0]) {
         case 'string':
+        case 'array':
           if (typeof current.default == 'string') {
             defaultValue = current.default;
           }
@@ -308,7 +326,7 @@ export function addSchemaOptionsToCommand<T>(
     }
 
     if (itemValueType) {
-      keyValuePairOptions.add(name);
+      keyValuePairOptions.add(dashedName);
     }
 
     const sharedOptions: YargsOptions & PositionalOptions = {
@@ -317,7 +335,7 @@ export function addSchemaOptionsToCommand<T>(
       description,
       deprecated,
       choices,
-      coerce: itemValueType ? coerceToStringMap.bind(null, dashedName) : undefined,
+      coerce: itemValueType ? coerceToStringMap : undefined,
       // This should only be done when `--help` is used otherwise default will override options set in angular.json.
       ...(includeDefaultValues ? { default: defaultVal } : {}),
     };
@@ -341,6 +359,11 @@ export function addSchemaOptionsToCommand<T>(
     }
   }
 
+  // Valid key/value options
+  if (keyValuePairOptions.size) {
+    localYargs.check(checkStringMap.bind(null, keyValuePairOptions), false);
+  }
+
   // Handle options which have been defined in the schema with `no` prefix.
   if (booleanOptionsWithNoPrefix.size) {
     localYargs.middleware((options: Arguments) => {
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
@@ -11,90 +11,56 @@ import yargs 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);
-
+describe('parseJsonSchemaToOptions', () => {
+  describe('without required fields in schema', () => {
+    const parse = async (args: string[]) => {
       // 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'],
-        },
-        'arrayWithChoices': {
-          'type': 'array',
-          'items': {
-            'type': 'string',
-            'enum': ['always', 'never'],
+    let localYargs: yargs.Argv<unknown>;
+    beforeEach(async () => {
+      // Create a fresh yargs for each call. The yargs object is stateful and
+      // calling .parse multiple times on the same instance isn't safe.
+      localYargs = yargs().exitProcess(false).strict().fail(false).wrap(1_000);
+      const jsonSchema = {
+        'type': 'object',
+        'properties': {
+          'maxSize': {
+            'type': 'number',
           },
-        },
-        'extendable': {
-          'type': 'object',
-          'properties': {},
-          'additionalProperties': {
+          'ssr': {
             'type': 'string',
+            'enum': ['always', 'surprise-me', 'never'],
           },
-        },
-        'someDefine': {
-          'type': 'object',
-          'additionalProperties': {
-            'type': 'string',
+          'arrayWithChoices': {
+            'type': 'array',
+            'default': 'default-array',
+            'items': {
+              'type': 'string',
+              'enum': ['always', 'never', 'default-array'],
+            },
+          },
+          'extendable': {
+            'type': 'object',
+            'properties': {},
+            'additionalProperties': {
+              'type': 'string',
+            },
+          },
+          'someDefine': {
+            'type': 'object',
+            'additionalProperties': {
+              'type': 'string',
+            },
           },
         },
-      },
+      };
+      const registry = new schema.CoreSchemaRegistry();
+      const options = await parseJsonSchemaToOptions(registry, jsonSchema, false);
+      addSchemaOptionsToCommand(localYargs, options, true);
     });
 
     describe('type=number', () => {
@@ -123,6 +89,10 @@ describe('parseJsonSchemaToOptions', () => {
           /Argument: array-with-choices, Given: "yes", Choices:/,
         );
       });
+
+      it('should add default value to help', async () => {
+        expect(await localYargs.getHelp()).toContain('[default: "default-array"]');
+      });
     });
 
     describe('type=string, enum', () => {
@@ -150,11 +120,9 @@ describe('parseJsonSchemaToOptions', () => {
 
       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/,
         );
       });
