Skip to content

Commit e30e61b

Browse files
crisbetopkozlowski-opensource
authored andcommitted
fix(compiler-cli): avoid allocating an object for signals in production mode
Currently when the signal debug name transform sees something like `const foo = signal(0);`, it transforms the signal into `signal(0, {...(ngDevMode ? { debugName: 'foo' } : {})})`. After minification this becomes `signal(0, {})` which will allocate memory for the empty object literal. These changes rework the logic to produce `signal(0, ...(ngDevMode ? [{ debugName: 'foo' }] : []))` which will be fully tree shaken away to `signal(0)`.
1 parent d8ab83c commit e30e61b

File tree

8 files changed

+142
-154
lines changed

8 files changed

+142
-154
lines changed

packages/compiler-cli/src/ngtsc/transform/src/implicit_signal_debug_name_transform.ts

Lines changed: 74 additions & 80 deletions
Original file line numberDiff line numberDiff line change
@@ -9,100 +9,94 @@
99
import ts from 'typescript';
1010

1111
function insertDebugNameIntoCallExpression(
12-
callExpression: ts.CallExpression,
12+
node: ts.CallExpression,
1313
debugName: string,
1414
): ts.CallExpression {
15-
const signalExpressionIsRequired = isRequiredSignalFunction(callExpression.expression);
16-
let configPosition = signalExpressionIsRequired ? 0 : 1;
17-
18-
// 1. If the call expression has no arguments, we pretend that the config object is at position 0.
19-
// We do this so that we can insert a spread element at the start of the args list in a way where
20-
// undefined can be the first argument but still get tree-shaken out in production builds.
21-
// or
22-
// 2. If the signal has an object-only definition (e.g. `linkedSignal` or `resource`), we set
23-
// the argument position to 0, i.e. reusing the existing object.
24-
const signalExpressionHasNoArguments = callExpression.arguments.length === 0;
25-
const signalWithObjectOnlyDefinition = isSignalWithObjectOnlyDefinition(callExpression);
26-
if (signalExpressionHasNoArguments || signalWithObjectOnlyDefinition) {
27-
configPosition = 0;
28-
}
29-
30-
const nodeArgs = Array.from(callExpression.arguments);
31-
let existingArgument = nodeArgs[configPosition];
32-
33-
if (existingArgument === undefined) {
34-
existingArgument = ts.factory.createObjectLiteralExpression([]);
35-
}
36-
37-
// Do nothing if an identifier is used as the config object
38-
// Ex:
39-
// const defaultObject = { equals: () => false };
40-
// signal(123, defaultObject)
41-
if (ts.isIdentifier(existingArgument)) {
42-
return callExpression;
43-
}
15+
const isRequired = isRequiredSignalFunction(node.expression);
16+
const hasNoArgs = node.arguments.length === 0;
17+
const configPosition = hasNoArgs || isSignalWithObjectOnlyDefinition(node) || isRequired ? 0 : 1;
18+
const existingArg =
19+
configPosition >= node.arguments.length ? null : node.arguments[configPosition];
4420

45-
if (!ts.isObjectLiteralExpression(existingArgument)) {
46-
return callExpression;
47-
}
48-
49-
// Insert debugName into the existing config object
50-
const properties = Array.from(existingArgument.properties);
51-
const debugNameExists = properties.some(
52-
(prop) =>
53-
ts.isPropertyAssignment(prop) && ts.isIdentifier(prop.name) && prop.name.text === 'debugName',
54-
);
55-
56-
if (debugNameExists) {
57-
return callExpression;
21+
// Do nothing if the existing parameter isn't statically analyzable or already has a `debugName`.
22+
if (
23+
existingArg !== null &&
24+
(!ts.isObjectLiteralExpression(existingArg) ||
25+
existingArg.properties.some(
26+
(prop) =>
27+
ts.isPropertyAssignment(prop) &&
28+
ts.isIdentifier(prop.name) &&
29+
prop.name.text === 'debugName',
30+
))
31+
) {
32+
return node;
5833
}
5934

60-
const ngDevModeIdentifier = ts.factory.createIdentifier('ngDevMode');
6135
const debugNameProperty = ts.factory.createPropertyAssignment(
6236
'debugName',
6337
ts.factory.createStringLiteral(debugName),
6438
);
65-
const debugNameObject = ts.factory.createObjectLiteralExpression([debugNameProperty]);
66-
const emptyObject = ts.factory.createObjectLiteralExpression();
67-
68-
// Create the spread expression: `...(ngDevMode ? { debugName: 'myDebugName' } : {})`
69-
const spreadDebugNameExpression = ts.factory.createSpreadAssignment(
70-
ts.factory.createParenthesizedExpression(
71-
ts.factory.createConditionalExpression(
72-
ngDevModeIdentifier,
73-
undefined, // Question token
74-
debugNameObject,
75-
undefined, // Colon token
76-
emptyObject,
39+
40+
let newArgs: ts.Expression[];
41+
42+
if (existingArg !== null) {
43+
// If there's an existing object literal already, we transform it as follows:
44+
// `signal(0, {equal})` becomes `signal(0, { ...(ngDevMode ? {debugName: "n"} : {}), equal })`.
45+
// During minification the spread will be removed since it's pointing to an empty object.
46+
const transformedArg = ts.factory.createObjectLiteralExpression([
47+
ts.factory.createSpreadAssignment(
48+
createNgDevModeConditional(
49+
ts.factory.createObjectLiteralExpression([debugNameProperty]),
50+
ts.factory.createObjectLiteralExpression(),
51+
),
7752
),
78-
),
79-
);
53+
...existingArg.properties,
54+
]);
8055

81-
const transformedConfigProperties = ts.factory.createObjectLiteralExpression([
82-
spreadDebugNameExpression,
83-
...properties,
84-
]);
85-
86-
let transformedSignalArgs = [];
87-
88-
// The following expression handles 3 cases:
89-
// 1. Non-`required` signals without an argument that need to be prepended with `undefined` (e.g. `model()`).
90-
// 2. Signals with object-only definition like `resource` or `linkedSignal` with computation;
91-
// Or `required` signals.
92-
// 3. All remaining cases where we have a signal with an argument (e.g `computed(Fn)` or `signal('foo')`).
93-
if (signalExpressionHasNoArguments && !signalExpressionIsRequired) {
94-
transformedSignalArgs = [ts.factory.createIdentifier('undefined'), transformedConfigProperties];
95-
} else if (signalWithObjectOnlyDefinition || signalExpressionIsRequired) {
96-
transformedSignalArgs = [transformedConfigProperties];
56+
newArgs = node.arguments.map((arg) => (arg === existingArg ? transformedArg : arg));
9757
} else {
98-
transformedSignalArgs = [nodeArgs[0], transformedConfigProperties];
58+
// If there's no existing argument, we transform it as follows:
59+
// `input(0)` becomes `input(0, ...(ngDevMode ? [{debugName: "n"}] : []))`
60+
// Spreading into an empty literal allows for the array to be dropped during minification.
61+
const spreadArgs: ts.Expression[] = [];
62+
63+
// If we're adding an argument, but the function requires a first argument (e.g. `input()`),
64+
// we have to add `undefined` before the debug literal.
65+
if (hasNoArgs && !isRequired) {
66+
spreadArgs.push(ts.factory.createIdentifier('undefined'));
67+
}
68+
69+
spreadArgs.push(ts.factory.createObjectLiteralExpression([debugNameProperty]));
70+
71+
newArgs = [
72+
...node.arguments,
73+
ts.factory.createSpreadElement(
74+
createNgDevModeConditional(
75+
ts.factory.createArrayLiteralExpression(spreadArgs),
76+
ts.factory.createArrayLiteralExpression(),
77+
),
78+
),
79+
];
9980
}
10081

101-
return ts.factory.updateCallExpression(
102-
callExpression,
103-
callExpression.expression,
104-
callExpression.typeArguments,
105-
ts.factory.createNodeArray(transformedSignalArgs),
82+
return ts.factory.updateCallExpression(node, node.expression, node.typeArguments, newArgs);
83+
}
84+
85+
/**
86+
* Creates an expression in the form of `(ngDevMode ? <devModeExpression> : <prodModeExpression>)`.
87+
*/
88+
function createNgDevModeConditional(
89+
devModeExpression: ts.Expression,
90+
prodModeExpression: ts.Expression,
91+
): ts.ParenthesizedExpression {
92+
return ts.factory.createParenthesizedExpression(
93+
ts.factory.createConditionalExpression(
94+
ts.factory.createIdentifier('ngDevMode'),
95+
undefined,
96+
devModeExpression,
97+
undefined,
98+
prodModeExpression,
99+
),
106100
);
107101
}
108102

packages/compiler-cli/test/compliance/test_cases/model_inputs/GOLDEN_PARTIAL.js

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,8 @@ import { Directive, model } from '@angular/core';
55
import * as i0 from "@angular/core";
66
export class TestDir {
77
constructor() {
8-
this.counter = model(0, Object.assign({}, (ngDevMode ? { debugName: "counter" } : {})));
9-
this.name = model.required(Object.assign({}, (ngDevMode ? { debugName: "name" } : {})));
8+
this.counter = model(0, ...(ngDevMode ? [{ debugName: "counter" }] : []));
9+
this.name = model.required(...(ngDevMode ? [{ debugName: "name" }] : []));
1010
}
1111
}
1212
TestDir.ɵfac = i0.ɵɵngDeclareFactory({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: TestDir, deps: [], target: i0.ɵɵFactoryTarget.Directive });
@@ -34,8 +34,8 @@ import { Component, model } from '@angular/core';
3434
import * as i0 from "@angular/core";
3535
export class TestComp {
3636
constructor() {
37-
this.counter = model(0, Object.assign({}, (ngDevMode ? { debugName: "counter" } : {})));
38-
this.name = model.required(Object.assign({}, (ngDevMode ? { debugName: "name" } : {})));
37+
this.counter = model(0, ...(ngDevMode ? [{ debugName: "counter" }] : []));
38+
this.name = model.required(...(ngDevMode ? [{ debugName: "name" }] : []));
3939
}
4040
}
4141
TestComp.ɵfac = i0.ɵɵngDeclareFactory({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: TestComp, deps: [], target: i0.ɵɵFactoryTarget.Component });
@@ -65,7 +65,7 @@ import { Directive, EventEmitter, Input, model, Output } from '@angular/core';
6565
import * as i0 from "@angular/core";
6666
export class TestDir {
6767
constructor() {
68-
this.counter = model(0, Object.assign({}, (ngDevMode ? { debugName: "counter" } : {})));
68+
this.counter = model(0, ...(ngDevMode ? [{ debugName: "counter" }] : []));
6969
this.modelWithAlias = model(false, Object.assign(Object.assign({}, (ngDevMode ? { debugName: "modelWithAlias" } : {})), { alias: 'alias' }));
7070
this.decoratorInput = true;
7171
this.decoratorInputWithAlias = true;

packages/compiler-cli/test/compliance/test_cases/r3_view_compiler/animations/GOLDEN_PARTIAL.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -188,7 +188,7 @@ import { Component, signal } from '@angular/core';
188188
import * as i0 from "@angular/core";
189189
export class MyComponent {
190190
constructor() {
191-
this.enterClass = signal('slide', Object.assign({}, (ngDevMode ? { debugName: "enterClass" } : {})));
191+
this.enterClass = signal('slide', ...(ngDevMode ? [{ debugName: "enterClass" }] : []));
192192
}
193193
}
194194
MyComponent.ɵfac = i0.ɵɵngDeclareFactory({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: MyComponent, deps: [], target: i0.ɵɵFactoryTarget.Component });
@@ -299,7 +299,7 @@ import { Component, signal } from '@angular/core';
299299
import * as i0 from "@angular/core";
300300
export class MyComponent {
301301
constructor() {
302-
this.leaveClass = signal('fade', Object.assign({}, (ngDevMode ? { debugName: "leaveClass" } : {})));
302+
this.leaveClass = signal('fade', ...(ngDevMode ? [{ debugName: "leaveClass" }] : []));
303303
}
304304
}
305305
MyComponent.ɵfac = i0.ɵɵngDeclareFactory({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: MyComponent, deps: [], target: i0.ɵɵFactoryTarget.Component });

packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_bindings/control_bindings/GOLDEN_PARTIAL.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import { Component, Directive, input } from '@angular/core';
55
import * as i0 from "@angular/core";
66
export class Field {
77
constructor() {
8-
this.field = input(undefined, Object.assign({}, (ngDevMode ? { debugName: "field" } : {})));
8+
this.field = input(...(ngDevMode ? [undefined, { debugName: "field" }] : []));
99
}
1010
}
1111
Field.ɵfac = i0.ɵɵngDeclareFactory({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: Field, deps: [], target: i0.ɵɵFactoryTarget.Directive });

packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_listener/GOLDEN_PARTIAL.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -967,7 +967,7 @@ import { Component, Directive, model, signal } from '@angular/core';
967967
import * as i0 from "@angular/core";
968968
export class NgModelDirective {
969969
constructor() {
970-
this.ngModel = model.required(Object.assign({}, (ngDevMode ? { debugName: "ngModel" } : {})));
970+
this.ngModel = model.required(...(ngDevMode ? [{ debugName: "ngModel" }] : []));
971971
}
972972
}
973973
NgModelDirective.ɵfac = i0.ɵɵngDeclareFactory({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: NgModelDirective, deps: [], target: i0.ɵɵFactoryTarget.Directive });
@@ -1023,7 +1023,7 @@ import { Component, Directive, model } from '@angular/core';
10231023
import * as i0 from "@angular/core";
10241024
export class NgModelDirective {
10251025
constructor() {
1026-
this.ngModel = model('', Object.assign({}, (ngDevMode ? { debugName: "ngModel" } : {})));
1026+
this.ngModel = model('', ...(ngDevMode ? [{ debugName: "ngModel" }] : []));
10271027
}
10281028
}
10291029
NgModelDirective.ɵfac = i0.ɵɵngDeclareFactory({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: NgModelDirective, deps: [], target: i0.ɵɵFactoryTarget.Directive });

packages/compiler-cli/test/compliance/test_cases/signal_inputs/GOLDEN_PARTIAL.js

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,8 @@ import { Directive, input } from '@angular/core';
55
import * as i0 from "@angular/core";
66
export class TestDir {
77
constructor() {
8-
this.counter = input(0, Object.assign({}, (ngDevMode ? { debugName: "counter" } : {})));
9-
this.name = input.required(Object.assign({}, (ngDevMode ? { debugName: "name" } : {})));
8+
this.counter = input(0, ...(ngDevMode ? [{ debugName: "counter" }] : []));
9+
this.name = input.required(...(ngDevMode ? [{ debugName: "name" }] : []));
1010
}
1111
}
1212
TestDir.ɵfac = i0.ɵɵngDeclareFactory({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: TestDir, deps: [], target: i0.ɵɵFactoryTarget.Directive });
@@ -34,8 +34,8 @@ import { Component, input } from '@angular/core';
3434
import * as i0 from "@angular/core";
3535
export class TestComp {
3636
constructor() {
37-
this.counter = input(0, Object.assign({}, (ngDevMode ? { debugName: "counter" } : {})));
38-
this.name = input.required(Object.assign({}, (ngDevMode ? { debugName: "name" } : {})));
37+
this.counter = input(0, ...(ngDevMode ? [{ debugName: "counter" }] : []));
38+
this.name = input.required(...(ngDevMode ? [{ debugName: "name" }] : []));
3939
}
4040
}
4141
TestComp.ɵfac = i0.ɵɵngDeclareFactory({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: TestComp, deps: [], target: i0.ɵɵFactoryTarget.Component });
@@ -68,7 +68,7 @@ function convertToBoolean(value) {
6868
}
6969
export class TestDir {
7070
constructor() {
71-
this.counter = input(0, Object.assign({}, (ngDevMode ? { debugName: "counter" } : {})));
71+
this.counter = input(0, ...(ngDevMode ? [{ debugName: "counter" }] : []));
7272
this.signalWithTransform = input(false, Object.assign(Object.assign({}, (ngDevMode ? { debugName: "signalWithTransform" } : {})), { transform: convertToBoolean }));
7373
this.signalWithTransformAndAlias = input(false, Object.assign(Object.assign({}, (ngDevMode ? { debugName: "signalWithTransformAndAlias" } : {})), { alias: 'publicNameSignal', transform: convertToBoolean }));
7474
this.decoratorInput = true;

packages/compiler-cli/test/compliance/test_cases/signal_queries/GOLDEN_PARTIAL.js

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -8,12 +8,12 @@ export class SomeToken {
88
const nonAnalyzableRefersToString = 'a, b, c';
99
export class TestDir {
1010
constructor() {
11-
this.query1 = viewChild('locatorA', Object.assign({}, (ngDevMode ? { debugName: "query1" } : {})));
12-
this.query2 = viewChildren('locatorB', Object.assign({}, (ngDevMode ? { debugName: "query2" } : {})));
13-
this.query3 = contentChild('locatorC', Object.assign({}, (ngDevMode ? { debugName: "query3" } : {})));
14-
this.query4 = contentChildren('locatorD', Object.assign({}, (ngDevMode ? { debugName: "query4" } : {})));
15-
this.query5 = viewChild(forwardRef(() => SomeToken), Object.assign({}, (ngDevMode ? { debugName: "query5" } : {})));
16-
this.query6 = viewChildren(SomeToken, Object.assign({}, (ngDevMode ? { debugName: "query6" } : {})));
11+
this.query1 = viewChild('locatorA', ...(ngDevMode ? [{ debugName: "query1" }] : []));
12+
this.query2 = viewChildren('locatorB', ...(ngDevMode ? [{ debugName: "query2" }] : []));
13+
this.query3 = contentChild('locatorC', ...(ngDevMode ? [{ debugName: "query3" }] : []));
14+
this.query4 = contentChildren('locatorD', ...(ngDevMode ? [{ debugName: "query4" }] : []));
15+
this.query5 = viewChild(forwardRef(() => SomeToken), ...(ngDevMode ? [{ debugName: "query5" }] : []));
16+
this.query6 = viewChildren(SomeToken, ...(ngDevMode ? [{ debugName: "query6" }] : []));
1717
this.query7 = viewChild('locatorE', Object.assign(Object.assign({}, (ngDevMode ? { debugName: "query7" } : {})), { read: SomeToken }));
1818
this.query8 = contentChildren('locatorF, locatorG', Object.assign(Object.assign({}, (ngDevMode ? { debugName: "query8" } : {})), { descendants: true }));
1919
this.query9 = contentChildren(nonAnalyzableRefersToString, Object.assign(Object.assign({}, (ngDevMode ? { debugName: "query9" } : {})), { descendants: true }));
@@ -53,10 +53,10 @@ import { Component, contentChild, contentChildren, viewChild, viewChildren } fro
5353
import * as i0 from "@angular/core";
5454
export class TestComp {
5555
constructor() {
56-
this.query1 = viewChild('locatorA', Object.assign({}, (ngDevMode ? { debugName: "query1" } : {})));
57-
this.query2 = viewChildren('locatorB', Object.assign({}, (ngDevMode ? { debugName: "query2" } : {})));
58-
this.query3 = contentChild('locatorC', Object.assign({}, (ngDevMode ? { debugName: "query3" } : {})));
59-
this.query4 = contentChildren('locatorD', Object.assign({}, (ngDevMode ? { debugName: "query4" } : {})));
56+
this.query1 = viewChild('locatorA', ...(ngDevMode ? [{ debugName: "query1" }] : []));
57+
this.query2 = viewChildren('locatorB', ...(ngDevMode ? [{ debugName: "query2" }] : []));
58+
this.query3 = contentChild('locatorC', ...(ngDevMode ? [{ debugName: "query3" }] : []));
59+
this.query4 = contentChildren('locatorD', ...(ngDevMode ? [{ debugName: "query4" }] : []));
6060
}
6161
}
6262
TestComp.ɵfac = i0.ɵɵngDeclareFactory({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: TestComp, deps: [], target: i0.ɵɵFactoryTarget.Component });
@@ -88,8 +88,8 @@ import { ContentChild, contentChild, Directive, ViewChild, viewChild } from '@an
8888
import * as i0 from "@angular/core";
8989
export class TestDir {
9090
constructor() {
91-
this.signalViewChild = viewChild('locator1', Object.assign({}, (ngDevMode ? { debugName: "signalViewChild" } : {})));
92-
this.signalContentChild = contentChild('locator2', Object.assign({}, (ngDevMode ? { debugName: "signalContentChild" } : {})));
91+
this.signalViewChild = viewChild('locator1', ...(ngDevMode ? [{ debugName: "signalViewChild" }] : []));
92+
this.signalContentChild = contentChild('locator2', ...(ngDevMode ? [{ debugName: "signalContentChild" }] : []));
9393
}
9494
}
9595
TestDir.ɵfac = i0.ɵɵngDeclareFactory({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: TestDir, deps: [], target: i0.ɵɵFactoryTarget.Directive });

0 commit comments

Comments
 (0)