From 78f76485fe315ffd0262c1a3732092731235828b Mon Sep 17 00:00:00 2001 From: Jan Martin Date: Wed, 11 Sep 2024 14:46:49 -0700 Subject: [PATCH] feat(@angular-devkit/architect): merge object options from CLI We recently introduced the ability to pass object values from the command line (#28362). @clydin noticed that the initial behavior didn't work well for `--define`: It completely replaced all values even if just one of multiple defines is specified. This updates the architect to support merging of object options. If both the base option (e.g. from `angular.json`) and the override (e.g. from a CLI `--flag`) are objects, the objects are merged. See: https://github.com/angular/angular-cli/pull/28362 --- .../angular_devkit/architect/src/architect.ts | 6 +- .../angular_devkit/architect/src/options.ts | 39 +++++++++++ .../architect/src/options_spec.ts | 70 +++++++++++++++++++ 3 files changed, 111 insertions(+), 4 deletions(-) create mode 100644 packages/angular_devkit/architect/src/options.ts create mode 100644 packages/angular_devkit/architect/src/options_spec.ts diff --git a/packages/angular_devkit/architect/src/architect.ts b/packages/angular_devkit/architect/src/architect.ts index 56f2b26d4e7c..aecbeb06ea05 100644 --- a/packages/angular_devkit/architect/src/architect.ts +++ b/packages/angular_devkit/architect/src/architect.ts @@ -45,6 +45,7 @@ import { SimpleScheduler, createJobHandler, } from './jobs'; +import { mergeOptions } from './options'; import { scheduleByName, scheduleByTarget } from './schedule-by-name'; const inputSchema = require('./input-schema.json'); @@ -71,10 +72,7 @@ function _createJobHandlerFromBuilderInfo( concatMap(async (message) => { if (message.kind === JobInboundMessageKind.Input) { const v = message.value as BuilderInput; - const options = { - ...baseOptions, - ...v.options, - }; + const options = mergeOptions(baseOptions, v.options); // Validate v against the options schema. const validation = await registry.compile(info.optionSchema); diff --git a/packages/angular_devkit/architect/src/options.ts b/packages/angular_devkit/architect/src/options.ts new file mode 100644 index 000000000000..c3455d1f43a9 --- /dev/null +++ b/packages/angular_devkit/architect/src/options.ts @@ -0,0 +1,39 @@ +/** + * @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 } from '@angular-devkit/core'; + +import { BuilderInput } from './api'; + +type OverrideOptions = BuilderInput['options']; + +export function mergeOptions( + baseOptions: json.JsonObject, + overrideOptions: OverrideOptions, +): json.JsonObject { + if (!overrideOptions) { + return { ...baseOptions }; + } + + const options = { + ...baseOptions, + ...overrideOptions, + }; + + // For object-object overrides, we merge one layer deep. + for (const key of Object.keys(overrideOptions)) { + const override = overrideOptions[key]; + const base = baseOptions[key]; + + if (json.isJsonObject(base) && json.isJsonObject(override)) { + options[key] = { ...base, ...override }; + } + } + + return options; +} diff --git a/packages/angular_devkit/architect/src/options_spec.ts b/packages/angular_devkit/architect/src/options_spec.ts new file mode 100644 index 000000000000..ac84ed7ff747 --- /dev/null +++ b/packages/angular_devkit/architect/src/options_spec.ts @@ -0,0 +1,70 @@ +/** + * @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 { mergeOptions } from './options'; + +describe('mergeOptions', () => { + it('overwrites literal values', () => { + expect( + mergeOptions( + { + onlyBase: 'base', + a: 'foo', + b: 42, + c: true, + }, + { + onlyOverride: 'override', + a: 'bar', + b: 43, + c: false, + }, + ), + ).toEqual({ + onlyBase: 'base', + a: 'bar', + b: 43, + c: false, + onlyOverride: 'override', + }); + }); + + it('merges object values one layer deep', () => { + expect( + mergeOptions( + { + obj: { + nested: { + fromBase: true, + }, + fromBase: true, + overridden: false, + }, + }, + { + obj: { + nested: { + fromOverride: true, + }, + overridden: true, + fromOverride: true, + }, + }, + ), + ).toEqual({ + obj: { + nested: { + fromOverride: true, + }, + fromBase: true, + overridden: true, + fromOverride: true, + }, + }); + }); +});