From 4dfc70a601aa44bbf8d544a4a91ac5acddea9e3b Mon Sep 17 00:00:00 2001 From: Charles Lyding <19598772+clydin@users.noreply.github.com> Date: Fri, 3 Feb 2023 13:51:38 -0500 Subject: [PATCH] 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. --- .../plugins/adjust-static-class-members.ts | 40 ++++- .../adjust-static-class-members_spec.ts | 141 +++++++++++++++++- 2 files changed, 178 insertions(+), 3 deletions(-) diff --git a/packages/angular_devkit/build_angular/src/babel/plugins/adjust-static-class-members.ts b/packages/angular_devkit/build_angular/src/babel/plugins/adjust-static-class-members.ts index 5c451970c655..35943ea9e5e7 100644 --- a/packages/angular_devkit/build_angular/src/babel/plugins/adjust-static-class-members.ts +++ b/packages/angular_devkit/build_angular/src/babel/plugins/adjust-static-class-members.ts @@ -221,10 +221,48 @@ export default function (): PluginObj { visitedClasses.add(classNode); - if (hasPotentialSideEffects || wrapStatementPaths.length === 0) { + if (hasPotentialSideEffects) { return; } + // If no statements to wrap, check for static class properties. + // Static class properties may be downleveled at later stages in the build pipeline + // which results in additional function calls outside the class body. These calls + // then cause the class to be referenced and not eligible for removal. Since it is + // not known at this stage whether the class needs to be downleveled, the transform + // wraps classes preemptively to allow for potential removal within the optimization + // stages. + if (wrapStatementPaths.length === 0) { + let shouldWrap = false; + for (const element of path.get('body').get('body')) { + if (element.isClassProperty()) { + // Only need to analyze static properties + if (!element.node.static) { + continue; + } + + // Check for potential side effects. + // These checks are conservative and could potentially be expanded in the future. + const elementKey = element.get('key'); + const elementValue = element.get('value'); + if ( + elementKey.isIdentifier() && + (!elementValue.isExpression() || + canWrapProperty(elementKey.get('name'), elementValue)) + ) { + shouldWrap = true; + } else { + // Not safe to wrap + shouldWrap = false; + break; + } + } + } + if (!shouldWrap) { + return; + } + } + const wrapStatementNodes: types.Statement[] = []; for (const statementPath of wrapStatementPaths) { wrapStatementNodes.push(statementPath.node); diff --git a/packages/angular_devkit/build_angular/src/babel/plugins/adjust-static-class-members_spec.ts b/packages/angular_devkit/build_angular/src/babel/plugins/adjust-static-class-members_spec.ts index 228bf57ea2a6..a62f87d38504 100644 --- a/packages/angular_devkit/build_angular/src/babel/plugins/adjust-static-class-members_spec.ts +++ b/packages/angular_devkit/build_angular/src/babel/plugins/adjust-static-class-members_spec.ts @@ -191,7 +191,7 @@ describe('adjust-static-class-members Babel plugin', () => { `); }); - it('does wrap not class with only side effect fields', () => { + it('does not wrap class with only side effect fields', () => { testCaseNoChange(` class CustomComponentEffects { constructor(_actions) { @@ -203,6 +203,30 @@ describe('adjust-static-class-members Babel plugin', () => { `); }); + it('does not wrap class with only side effect native fields', () => { + testCaseNoChange(` + class CustomComponentEffects { + static someFieldWithSideEffects = console.log('foo'); + constructor(_actions) { + this._actions = _actions; + this.doThis = this._actions; + } + } + `); + }); + + it('does not wrap class with only instance native fields', () => { + testCaseNoChange(` + class CustomComponentEffects { + someFieldWithSideEffects = console.log('foo'); + constructor(_actions) { + this._actions = _actions; + this.doThis = this._actions; + } + } + `); + }); + it('wraps class with pure annotated side effect fields (#__PURE__)', () => { testCase({ input: ` @@ -229,6 +253,32 @@ describe('adjust-static-class-members Babel plugin', () => { }); }); + it('wraps class with pure annotated side effect native fields (#__PURE__)', () => { + testCase({ + input: ` + class CustomComponentEffects { + static someFieldWithSideEffects = /*#__PURE__*/ console.log('foo'); + constructor(_actions) { + this._actions = _actions; + this.doThis = this._actions; + } + } + `, + expected: ` + let CustomComponentEffects = /*#__PURE__*/ (() => { + class CustomComponentEffects { + static someFieldWithSideEffects = /*#__PURE__*/ console.log('foo'); + constructor(_actions) { + this._actions = _actions; + this.doThis = this._actions; + } + } + return CustomComponentEffects; + })(); + `, + }); + }); + it('wraps class with pure annotated side effect fields (@__PURE__)', () => { testCase({ input: ` @@ -335,6 +385,32 @@ describe('adjust-static-class-members Babel plugin', () => { }); }); + it('wraps exported class with a pure native static field', () => { + testCase({ + input: ` + export class CustomComponentEffects { + static someField = 42; + constructor(_actions) { + this._actions = _actions; + this.doThis = this._actions; + } + } + `, + expected: ` + export let CustomComponentEffects = /*#__PURE__*/ (() => { + class CustomComponentEffects { + static someField = 42; + constructor(_actions) { + this._actions = _actions; + this.doThis = this._actions; + } + } + return CustomComponentEffects; + })(); + `, + }); + }); + it('wraps class with a basic literal static field', () => { testCase({ input: ` @@ -416,6 +492,32 @@ describe('adjust-static-class-members Babel plugin', () => { `); }); + it('does not wrap class with only pure native static fields and some side effect static fields', () => { + testCaseNoChange(` + class CustomComponentEffects { + static someField = 42; + constructor(_actions) { + this._actions = _actions; + this.doThis = this._actions; + } + } + CustomComponentEffects.someFieldWithSideEffects = console.log('foo'); + `); + }); + + it('does not wrap class with only some pure native static fields', () => { + testCaseNoChange(` + class CustomComponentEffects { + static someField = 42; + static someFieldWithSideEffects = console.log('foo'); + constructor(_actions) { + this._actions = _actions; + this.doThis = this._actions; + } + } + `); + }); + it('does not wrap class with class decorators when wrapDecorators is false', () => { testCaseNoChange( ` @@ -597,7 +699,7 @@ describe('adjust-static-class-members Babel plugin', () => { }); }); - it('wraps class with multiple Angular static field', () => { + it('wraps class with multiple Angular static fields', () => { testCase({ input: ` class CommonModule { @@ -626,6 +728,41 @@ describe('adjust-static-class-members Babel plugin', () => { }); }); + it('wraps class with multiple Angular native static fields', () => { + testCase({ + input: ` + class CommonModule { + static ɵfac = function CommonModule_Factory(t) { return new (t || CommonModule)(); }; + static ɵmod = /*@__PURE__*/ ɵngcc0.ɵɵdefineNgModule({ type: CommonModule }); + static ɵinj = /*@__PURE__*/ ɵngcc0.ɵɵdefineInjector({ providers: [ + { provide: NgLocalization, useClass: NgLocaleLocalization }, + ] }); + } + `, + expected: ` + let CommonModule = /*#__PURE__*/ (() => { + class CommonModule { + static ɵfac = function CommonModule_Factory(t) { + return new (t || CommonModule)(); + }; + static ɵmod = /*@__PURE__*/ ɵngcc0.ɵɵdefineNgModule({ + type: CommonModule, + }); + static ɵinj = /*@__PURE__*/ ɵngcc0.ɵɵdefineInjector({ + providers: [ + { + provide: NgLocalization, + useClass: NgLocaleLocalization, + }, + ], + }); + } + return CommonModule; + })(); + `, + }); + }); + it('wraps default exported class with pure static fields', () => { testCase({ input: `