Skip to content

fix(@angular-devkit/build-angular): build optimizer support for spec-compliant downlevel class properties #24658

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
angular-robot merged 1 commit into from
Feb 7, 2023
Merged

fix(@angular-devkit/build-angular): build optimizer support for spec-compliant downlevel class properties #24658

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 39 additions & 1 deletion packages/angular_devkit/build_angular/src/babel/plugins/adjust-static-class-members.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
141 changes: 139 additions & 2 deletions packages/angular_devkit/build_angular/src/babel/plugins/adjust-static-class-members_spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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: `
Expand All @@ -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: `
Expand Down Expand Up @@ -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: `
Expand Down Expand Up @@ -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(
`
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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: `
Expand Down