Skip to content

Commit e87134f

Browse files
clydindgp1130
authored andcommittedFeb 7, 2023
fix(@angular-devkit/build-angular): build optimizer support for spec-compliant downlevel class properties
The build optimizer's static class field pass will now additionally wrap classes that contain side effect free class properties. This allows optimizers and bundlers further along in the build pipeline to downlevel that class properties while still retaining the ability to tree-shake the class if unused. The class properties may need to be downleveled for a variety of reasons such as lack of browser support, browser bugs with certain code structures, or to ensure spec-compliant runtime behavior. (cherry picked from commit bfc0fac)
1 parent b8bbe96 commit e87134f

File tree

2 files changed

+178
-3
lines changed

2 files changed

+178
-3
lines changed
 

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

+39-1
Original file line numberDiff line numberDiff line change
@@ -221,10 +221,48 @@ export default function (): PluginObj {
221221

222222
visitedClasses.add(classNode);
223223

224-
if (hasPotentialSideEffects || wrapStatementPaths.length === 0) {
224+
if (hasPotentialSideEffects) {
225225
return;
226226
}
227227

228+
// If no statements to wrap, check for static class properties.
229+
// Static class properties may be downleveled at later stages in the build pipeline
230+
// which results in additional function calls outside the class body. These calls
231+
// then cause the class to be referenced and not eligible for removal. Since it is
232+
// not known at this stage whether the class needs to be downleveled, the transform
233+
// wraps classes preemptively to allow for potential removal within the optimization
234+
// stages.
235+
if (wrapStatementPaths.length === 0) {
236+
let shouldWrap = false;
237+
for (const element of path.get('body').get('body')) {
238+
if (element.isClassProperty()) {
239+
// Only need to analyze static properties
240+
if (!element.node.static) {
241+
continue;
242+
}
243+
244+
// Check for potential side effects.
245+
// These checks are conservative and could potentially be expanded in the future.
246+
const elementKey = element.get('key');
247+
const elementValue = element.get('value');
248+
if (
249+
elementKey.isIdentifier() &&
250+
(!elementValue.isExpression() ||
251+
canWrapProperty(elementKey.get('name'), elementValue))
252+
) {
253+
shouldWrap = true;
254+
} else {
255+
// Not safe to wrap
256+
shouldWrap = false;
257+
break;
258+
}
259+
}
260+
}
261+
if (!shouldWrap) {
262+
return;
263+
}
264+
}
265+
228266
const wrapStatementNodes: types.Statement[] = [];
229267
for (const statementPath of wrapStatementPaths) {
230268
wrapStatementNodes.push(statementPath.node);

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

+139-2
Original file line numberDiff line numberDiff line change
@@ -191,7 +191,7 @@ describe('adjust-static-class-members Babel plugin', () => {
191191
`);
192192
});
193193

194-
it('does wrap not class with only side effect fields', () => {
194+
it('does not wrap class with only side effect fields', () => {
195195
testCaseNoChange(`
196196
class CustomComponentEffects {
197197
constructor(_actions) {
@@ -203,6 +203,30 @@ describe('adjust-static-class-members Babel plugin', () => {
203203
`);
204204
});
205205

206+
it('does not wrap class with only side effect native fields', () => {
207+
testCaseNoChange(`
208+
class CustomComponentEffects {
209+
static someFieldWithSideEffects = console.log('foo');
210+
constructor(_actions) {
211+
this._actions = _actions;
212+
this.doThis = this._actions;
213+
}
214+
}
215+
`);
216+
});
217+
218+
it('does not wrap class with only instance native fields', () => {
219+
testCaseNoChange(`
220+
class CustomComponentEffects {
221+
someFieldWithSideEffects = console.log('foo');
222+
constructor(_actions) {
223+
this._actions = _actions;
224+
this.doThis = this._actions;
225+
}
226+
}
227+
`);
228+
});
229+
206230
it('wraps class with pure annotated side effect fields (#__PURE__)', () => {
207231
testCase({
208232
input: `
@@ -229,6 +253,32 @@ describe('adjust-static-class-members Babel plugin', () => {
229253
});
230254
});
231255

256+
it('wraps class with pure annotated side effect native fields (#__PURE__)', () => {
257+
testCase({
258+
input: `
259+
class CustomComponentEffects {
260+
static someFieldWithSideEffects = /*#__PURE__*/ console.log('foo');
261+
constructor(_actions) {
262+
this._actions = _actions;
263+
this.doThis = this._actions;
264+
}
265+
}
266+
`,
267+
expected: `
268+
let CustomComponentEffects = /*#__PURE__*/ (() => {
269+
class CustomComponentEffects {
270+
static someFieldWithSideEffects = /*#__PURE__*/ console.log('foo');
271+
constructor(_actions) {
272+
this._actions = _actions;
273+
this.doThis = this._actions;
274+
}
275+
}
276+
return CustomComponentEffects;
277+
})();
278+
`,
279+
});
280+
});
281+
232282
it('wraps class with pure annotated side effect fields (@__PURE__)', () => {
233283
testCase({
234284
input: `
@@ -335,6 +385,32 @@ describe('adjust-static-class-members Babel plugin', () => {
335385
});
336386
});
337387

388+
it('wraps exported class with a pure native static field', () => {
389+
testCase({
390+
input: `
391+
export class CustomComponentEffects {
392+
static someField = 42;
393+
constructor(_actions) {
394+
this._actions = _actions;
395+
this.doThis = this._actions;
396+
}
397+
}
398+
`,
399+
expected: `
400+
export let CustomComponentEffects = /*#__PURE__*/ (() => {
401+
class CustomComponentEffects {
402+
static someField = 42;
403+
constructor(_actions) {
404+
this._actions = _actions;
405+
this.doThis = this._actions;
406+
}
407+
}
408+
return CustomComponentEffects;
409+
})();
410+
`,
411+
});
412+
});
413+
338414
it('wraps class with a basic literal static field', () => {
339415
testCase({
340416
input: `
@@ -416,6 +492,32 @@ describe('adjust-static-class-members Babel plugin', () => {
416492
`);
417493
});
418494

495+
it('does not wrap class with only pure native static fields and some side effect static fields', () => {
496+
testCaseNoChange(`
497+
class CustomComponentEffects {
498+
static someField = 42;
499+
constructor(_actions) {
500+
this._actions = _actions;
501+
this.doThis = this._actions;
502+
}
503+
}
504+
CustomComponentEffects.someFieldWithSideEffects = console.log('foo');
505+
`);
506+
});
507+
508+
it('does not wrap class with only some pure native static fields', () => {
509+
testCaseNoChange(`
510+
class CustomComponentEffects {
511+
static someField = 42;
512+
static someFieldWithSideEffects = console.log('foo');
513+
constructor(_actions) {
514+
this._actions = _actions;
515+
this.doThis = this._actions;
516+
}
517+
}
518+
`);
519+
});
520+
419521
it('does not wrap class with class decorators when wrapDecorators is false', () => {
420522
testCaseNoChange(
421523
`
@@ -597,7 +699,7 @@ describe('adjust-static-class-members Babel plugin', () => {
597699
});
598700
});
599701

600-
it('wraps class with multiple Angular static field', () => {
702+
it('wraps class with multiple Angular static fields', () => {
601703
testCase({
602704
input: `
603705
class CommonModule {
@@ -626,6 +728,41 @@ describe('adjust-static-class-members Babel plugin', () => {
626728
});
627729
});
628730

731+
it('wraps class with multiple Angular native static fields', () => {
732+
testCase({
733+
input: `
734+
class CommonModule {
735+
static ɵfac = function CommonModule_Factory(t) { return new (t || CommonModule)(); };
736+
static ɵmod = /*@__PURE__*/ ɵngcc0.ɵɵdefineNgModule({ type: CommonModule });
737+
static ɵinj = /*@__PURE__*/ ɵngcc0.ɵɵdefineInjector({ providers: [
738+
{ provide: NgLocalization, useClass: NgLocaleLocalization },
739+
] });
740+
}
741+
`,
742+
expected: `
743+
let CommonModule = /*#__PURE__*/ (() => {
744+
class CommonModule {
745+
static ɵfac = function CommonModule_Factory(t) {
746+
return new (t || CommonModule)();
747+
};
748+
static ɵmod = /*@__PURE__*/ ɵngcc0.ɵɵdefineNgModule({
749+
type: CommonModule,
750+
});
751+
static ɵinj = /*@__PURE__*/ ɵngcc0.ɵɵdefineInjector({
752+
providers: [
753+
{
754+
provide: NgLocalization,
755+
useClass: NgLocaleLocalization,
756+
},
757+
],
758+
});
759+
}
760+
return CommonModule;
761+
})();
762+
`,
763+
});
764+
});
765+
629766
it('wraps default exported class with pure static fields', () => {
630767
testCase({
631768
input: `

0 commit comments

Comments
 (0)
Please sign in to comment.