Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Aug 18, 2025
+113 −123
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 37 additions & 14 deletions packages/angular/cli/src/command-builder/utilities/json-schema.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand 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;
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -308,7 +326,7 @@ export function addSchemaOptionsToCommand<T>(
}

if (itemValueType) {
keyValuePairOptions.add(name);
keyValuePairOptions.add(dashedName);
}

const sharedOptions: YargsOptions & PositionalOptions = {
Expand All @@ -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 } : {}),
};
Expand All @@ -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) => {
Expand Down
185 changes: 76 additions & 109 deletions packages/angular/cli/src/command-builder/utilities/json-schema_spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,95 +6,61 @@
* found in the LICENSE file at https://angular.dev/license
*/

import { json, schema } from '@angular-devkit/core';
import { schema } from '@angular-devkit/core';
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', () => {
Expand Down Expand Up @@ -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', () => {
Expand Down Expand Up @@ -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/,
);
});
Expand Down Expand Up @@ -187,43 +155,42 @@ describe('parseJsonSchemaToOptions', () => {

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
}
const jsonSchema = {
'$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
}
'a': {
'type': 'string',
'description': 'a.',
'$default': {
'$source': 'argv',
'index': 0,
},
},
"optC": {
"type": "string",
"description": "optC"
'optC': {
'type': 'string',
'description': 'optC',
},
"optA": {
"type": "string",
"description": "optA"
'optA': {
'type': 'string',
'description': 'optA',
},
'optB': {
'type': 'string',
'description': 'optB',
},
"optB": {
"type": "string",
"description": "optB"
}
}
}`) as json.JsonObject;
},
};
const registry = new schema.CoreSchemaRegistry();
const options = await parseJsonSchemaToOptions(registry, jsonSchema, /* interactive= */ true);

Expand Down
Loading