Skip to content

Commit

Permalink
fix(@angular/cli): temporarily handle boolean options in schema prefi…
Browse files Browse the repository at this point in the history
…xed with `no`

With this commit we introduce an interim solution for options prefixed with `no` in `schema.json`

Previously, such options were handled as normal boolean option, but yargs handles options prefixed with `no` as negatations of the original option. Example with yargs, an option `noWatch` is will registered as `watch`.

Closes #23397

(cherry picked from commit ba3f671)
  • Loading branch information
alan-agius4 authored and dgp1130 committed Jun 21, 2022
1 parent 3dc7427 commit 5521648
Show file tree
Hide file tree
Showing 6 changed files with 81 additions and 2 deletions.
34 changes: 32 additions & 2 deletions packages/angular/cli/src/command-builder/command-module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import { analytics, logging, schema, strings } from '@angular-devkit/core';
import { readFileSync } from 'fs';
import * as path from 'path';
import {
Arguments,
ArgumentsCamelCase,
Argv,
CamelCaseKey,
Expand Down Expand Up @@ -199,6 +200,8 @@ 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,
Expand All @@ -223,13 +226,27 @@ export abstract class CommandModule<T extends {} = {}> implements CommandModuleI
...(this.context.args.options.help ? { default: defaultVal } : {}),
};

// TODO(alanagius4): remove in a major version.
// the below is an interim workaround to handle options which have been defined in the schema with `no` prefix.
let dashedName = strings.dasherize(name);
if (type === 'boolean' && dashedName.startsWith('no-')) {
dashedName = dashedName.slice(3);
booleanOptionsWithNoPrefix.add(dashedName);

// eslint-disable-next-line no-console
console.warn(
`Warning: '${name}' option has been declared with a 'no' prefix in the schema.` +
'Please file an issue with the author of this package.',
);
}

if (positional === undefined) {
localYargs = localYargs.option(strings.dasherize(name), {
localYargs = localYargs.option(dashedName, {
type,
...sharedOptions,
});
} else {
localYargs = localYargs.positional(strings.dasherize(name), {
localYargs = localYargs.positional(dashedName, {
type: type === 'array' || type === 'count' ? 'string' : type,
...sharedOptions,
});
Expand All @@ -241,6 +258,19 @@ export abstract class CommandModule<T extends {} = {}> implements CommandModuleI
}
}

// TODO(alanagius4): remove in a major version.
// the below is an interim workaround to 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 localYargs;
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
{
"schematics": {
"test": {
"factory": "./index.js",
"schema": "./schema.json",
"description": "test schematic that logs the options in the console."
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
exports.default = (options) => console.log(options);
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"name": "schematic-boolean-option",
"version": "0.0.1",
"schematics": "./collection.json"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{
"$schema": "http://json-schema.org/draft-07/schema",
"type": "object",
"description": "test schematic that logs the options in the console.",
"properties": {
"noWatch": {
"type": "boolean"
}
}
}
24 changes: 24 additions & 0 deletions tests/legacy-cli/e2e/tests/misc/negated-boolean-options.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
import { copyAssets } from '../../utils/assets';
import { execAndWaitForOutputToMatch } from '../../utils/process';

export default async function () {
await copyAssets('schematic-boolean-option-negated', 'schematic-boolean-option-negated');

await execAndWaitForOutputToMatch(
'ng',
['generate', './schematic-boolean-option-negated:test', '--no-watch'],
/noWatch: true/,
);

await execAndWaitForOutputToMatch(
'ng',
['generate', './schematic-boolean-option-negated:test', '--watch'],
/noWatch: false/,
);

await execAndWaitForOutputToMatch(
'ng',
['generate', './schematic-boolean-option-negated:test'],
/'noWatch' option has been declared with a 'no' prefix in the schema/,
);
}

0 comments on commit 5521648

Please sign in to comment.