Skip to content

Commit 22e1aad

Browse files
committed
refactor(@angular/cli): avoid implicit undefined array defaults for @angular/build builders
Previously, all builder options that were of type array were set to a default empty array even if there was no explicit default defined within the schema. This can be problematic for options that have differing behavior based on their presence such as runtime calculated defaults. The implicit defaulting behavior was also not aligned with the generated schema types which resulted in additional type safety and initialization regardless. As a result, the implicit behavior was effectively redundant in most cases. Since this change could be breaking for third-party builders, the removal of this behavior is currently limited to the `@angular/build` package.
1 parent a5ace27 commit 22e1aad

File tree

5 files changed

+44
-9
lines changed

5 files changed

+44
-9
lines changed

goldens/public-api/angular_devkit/core/index.api.md

+4
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,9 @@ import { ValidateFunction } from 'ajv';
1818
// @public (undocumented)
1919
function addUndefinedDefaults(value: JsonValue, _pointer: JsonPointer, schema?: JsonSchema): JsonValue;
2020

21+
// @public (undocumented)
22+
function addUndefinedObjectDefaults(value: JsonValue, _pointer: JsonPointer, schema?: JsonSchema): JsonValue;
23+
2124
// @public
2225
class AliasHost<StatsT extends object = {}> extends ResolverHost<StatsT> {
2326
// (undocumented)
@@ -1297,6 +1300,7 @@ class TransformLogger extends Logger {
12971300

12981301
declare namespace transforms {
12991302
export {
1303+
addUndefinedObjectDefaults,
13001304
addUndefinedDefaults
13011305
}
13021306
}

modules/testing/builder/src/builder-harness.ts

+5-1
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,11 @@ export class BuilderHarness<T> {
104104
...builderInfo,
105105
};
106106

107-
this.schemaRegistry.addPostTransform(json.schema.transforms.addUndefinedDefaults);
107+
if (builderInfo?.builderName?.startsWith('@angular/build:')) {
108+
this.schemaRegistry.addPostTransform(json.schema.transforms.addUndefinedObjectDefaults);
109+
} else {
110+
this.schemaRegistry.addPostTransform(json.schema.transforms.addUndefinedDefaults);
111+
}
108112
}
109113

110114
private resolvePath(path: string): string {

packages/angular/build/src/builders/application/tests/setup.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ import { Schema } from '../schema';
1313
export * from '../../../../../../../modules/testing/builder/src';
1414

1515
export const APPLICATION_BUILDER_INFO = Object.freeze({
16-
name: '@angular-devkit/build-angular:application',
16+
name: '@angular/build:application',
1717
schemaPath: __dirname + '/../schema.json',
1818
});
1919

packages/angular/cli/src/command-builder/architect-base-command-module.ts

+15-5
Original file line numberDiff line numberDiff line change
@@ -52,10 +52,16 @@ export abstract class ArchitectBaseCommandModule<T extends object>
5252
return this.onMissingTarget(e.message);
5353
}
5454

55+
const isAngularBuild = builderName.startsWith('@angular/build:');
56+
5557
const { logger } = this.context;
56-
const run = await this.getArchitect().scheduleTarget(target, options as json.JsonObject, {
57-
logger,
58-
});
58+
const run = await this.getArchitect(isAngularBuild).scheduleTarget(
59+
target,
60+
options as json.JsonObject,
61+
{
62+
logger,
63+
},
64+
);
5965

6066
const analytics = isPackageNameSafeForAnalytics(builderName)
6167
? await this.getAnalytics()
@@ -150,13 +156,17 @@ export abstract class ArchitectBaseCommandModule<T extends object>
150156
}
151157

152158
private _architect: Architect | undefined;
153-
protected getArchitect(): Architect {
159+
protected getArchitect(skipUndefinedArrayTransform: boolean): Architect {
154160
if (this._architect) {
155161
return this._architect;
156162
}
157163

158164
const registry = new json.schema.CoreSchemaRegistry();
159-
registry.addPostTransform(json.schema.transforms.addUndefinedDefaults);
165+
if (skipUndefinedArrayTransform) {
166+
registry.addPostTransform(json.schema.transforms.addUndefinedObjectDefaults);
167+
} else {
168+
registry.addPostTransform(json.schema.transforms.addUndefinedDefaults);
169+
}
160170
registry.useXDeprecatedProvider((msg) => this.context.logger.warn(msg));
161171

162172
const architectHost = this.getArchitectHost();

packages/angular_devkit/core/src/json/schema/transforms.ts

+19-2
Original file line numberDiff line numberDiff line change
@@ -11,10 +11,27 @@ import { JsonPointer } from './interface';
1111
import { JsonSchema } from './schema';
1212
import { getTypesOfSchema } from './utility';
1313

14+
export function addUndefinedObjectDefaults(
15+
value: JsonValue,
16+
_pointer: JsonPointer,
17+
schema?: JsonSchema,
18+
): JsonValue {
19+
return transformUndefined(value, _pointer, schema, true);
20+
}
21+
1422
export function addUndefinedDefaults(
1523
value: JsonValue,
1624
_pointer: JsonPointer,
1725
schema?: JsonSchema,
26+
): JsonValue {
27+
return transformUndefined(value, _pointer, schema, false);
28+
}
29+
30+
function transformUndefined(
31+
value: JsonValue,
32+
_pointer: JsonPointer,
33+
schema?: JsonSchema,
34+
onlyObjects?: boolean,
1835
): JsonValue {
1936
if (typeof schema === 'boolean' || schema === undefined) {
2037
return value;
@@ -45,7 +62,7 @@ export function addUndefinedDefaults(
4562
return value;
4663
}
4764

48-
if (type === 'array') {
65+
if (!onlyObjects && type === 'array') {
4966
return value == undefined ? [] : value;
5067
}
5168

@@ -94,7 +111,7 @@ export function addUndefinedDefaults(
94111
});
95112

96113
if (adjustedSchema && isJsonObject(adjustedSchema)) {
97-
newValue[propName] = addUndefinedDefaults(value, _pointer, adjustedSchema);
114+
newValue[propName] = transformUndefined(value, _pointer, adjustedSchema, onlyObjects);
98115
}
99116
}
100117
}

0 commit comments

Comments
 (0)