Skip to content

Commit 4bcd1dc

Browse files
clydinjosephperrott
authored andcommitted
fix(@angular-devkit/build-angular): allow classes with pure annotated static properties to be optimized
When script optimizations are enabled, classes containing downlevelled static properties are wrapped in a pure annotated IIFE to allow the class to be removed if it is otherwise unused. Only properties with initializer values that do not have the potential for side effects can be safely wrapped. Previously, pure annotations were not considered when analyzing the values and this caused classes to be retained that could be considered safe for removal. To resolve this issue, pure annotations are now considered when analyzing a property's initializer value for potential side effects.
1 parent 3772836 commit 4bcd1dc

File tree

2 files changed

+98
-2
lines changed

2 files changed

+98
-2
lines changed

packages/angular_devkit/build_angular/src/babel/plugins/adjust-static-class-members.ts

+28-1
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,33 @@ export function getKeywords(): Iterable<string> {
5656
return ['class'];
5757
}
5858

59+
/**
60+
* Determines whether a property and its initializer value can be safely wrapped in a pure
61+
* annotated IIFE. Values that may cause side effects are not considered safe to wrap.
62+
* Wrapping such values may cause runtime errors and/or incorrect runtime behavior.
63+
*
64+
* @param propertyName The name of the property to analyze.
65+
* @param assignmentValue The initializer value that will be assigned to the property.
66+
* @returns If the property can be safely wrapped, then true; otherwise, false.
67+
*/
68+
function canWrapProperty(propertyName: string, assignmentValue: NodePath): boolean {
69+
if (angularStaticsToWrap.has(propertyName)) {
70+
return true;
71+
}
72+
73+
const { leadingComments } = assignmentValue.node as { leadingComments?: { value: string }[] };
74+
if (
75+
leadingComments?.some(
76+
// `@pureOrBreakMyCode` is used by closure and is present in Angular code
77+
({ value }) => value.includes('#__PURE__') || value.includes('@pureOrBreakMyCode'),
78+
)
79+
) {
80+
return true;
81+
}
82+
83+
return assignmentValue.isPure();
84+
}
85+
5986
/**
6087
* Analyze the sibling nodes of a class to determine if any downlevel elements should be
6188
* wrapped in a pure annotated IIFE. Also determines if any elements have potential side
@@ -140,7 +167,7 @@ function analyzeClassSiblings(
140167
if (angularStaticsToElide[propertyName]?.(assignmentValue)) {
141168
nextStatement.remove();
142169
--i;
143-
} else if (angularStaticsToWrap.has(propertyName) || assignmentValue.isPure()) {
170+
} else if (canWrapProperty(propertyName, assignmentValue)) {
144171
wrapStatementPaths.push(nextStatement);
145172
} else {
146173
// Statement cannot be safely wrapped which makes wrapping the class unneeded.

packages/angular_devkit/build_angular/src/babel/plugins/adjust-static-class-members_spec.ts

+70-1
Original file line numberDiff line numberDiff line change
@@ -182,7 +182,7 @@ describe('adjust-static-class-members Babel plugin', () => {
182182
`);
183183
});
184184

185-
it('does wrap not class with only side effect fields', () => {
185+
it('does wrap not default exported class with only side effect fields', () => {
186186
testCaseNoChange(`
187187
export default class CustomComponentEffects {
188188
constructor(_actions) {
@@ -194,6 +194,75 @@ describe('adjust-static-class-members Babel plugin', () => {
194194
`);
195195
});
196196

197+
it('does wrap not class with only side effect fields', () => {
198+
testCaseNoChange(`
199+
class CustomComponentEffects {
200+
constructor(_actions) {
201+
this._actions = _actions;
202+
this.doThis = this._actions;
203+
}
204+
}
205+
206+
CustomComponentEffects.someFieldWithSideEffects = console.log('foo');
207+
`);
208+
});
209+
210+
it('wraps class with pure annotated side effect fields', () => {
211+
testCase({
212+
input: `
213+
class CustomComponentEffects {
214+
constructor(_actions) {
215+
this._actions = _actions;
216+
this.doThis = this._actions;
217+
}
218+
}
219+
CustomComponentEffects.someFieldWithSideEffects = /*#__PURE__*/ console.log('foo');
220+
`,
221+
expected: `
222+
let CustomComponentEffects = /*#__PURE__*/ (() => {
223+
class CustomComponentEffects {
224+
constructor(_actions) {
225+
this._actions = _actions;
226+
this.doThis = this._actions;
227+
}
228+
}
229+
230+
CustomComponentEffects.someFieldWithSideEffects = /*#__PURE__*/ console.log('foo');
231+
return CustomComponentEffects;
232+
})();
233+
`,
234+
});
235+
});
236+
237+
it('wraps class with closure pure annotated side effect fields', () => {
238+
testCase({
239+
input: `
240+
class CustomComponentEffects {
241+
constructor(_actions) {
242+
this._actions = _actions;
243+
this.doThis = this._actions;
244+
}
245+
}
246+
CustomComponentEffects.someFieldWithSideEffects = /* @pureOrBreakMyCode */ console.log('foo');
247+
`,
248+
expected: `
249+
let CustomComponentEffects = /*#__PURE__*/ (() => {
250+
class CustomComponentEffects {
251+
constructor(_actions) {
252+
this._actions = _actions;
253+
this.doThis = this._actions;
254+
}
255+
}
256+
257+
CustomComponentEffects.someFieldWithSideEffects =
258+
/* @pureOrBreakMyCode */
259+
console.log('foo');
260+
return CustomComponentEffects;
261+
})();
262+
`,
263+
});
264+
});
265+
197266
it('wraps exported class with a pure static field', () => {
198267
testCase({
199268
input: `

0 commit comments

Comments
 (0)