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 72b282905e42..90c619dc024e 100644 --- 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, 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 | Promise { +): Record | (string | undefined)[] { const stringMap: Record = {}; 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( } if (itemValueType) { - keyValuePairOptions.add(name); + keyValuePairOptions.add(dashedName); } const sharedOptions: YargsOptions & PositionalOptions = { @@ -317,7 +335,7 @@ export function addSchemaOptionsToCommand( 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( } } + // 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 index fe24104cc611..cc86cc99dddc 100644 --- a/packages/angular/cli/src/command-builder/utilities/json-schema_spec.ts +++ b/packages/angular/cli/src/command-builder/utilities/json-schema_spec.ts @@ -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; + 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/, ); }); @@ -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);