Skip to content

Commit

Permalink
fix(@angular-devkit/build-angular): build optimizer support for spec-…
Browse files Browse the repository at this point in the history
…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)
  • Loading branch information
clydin authored and dgp1130 committed Feb 7, 2023
1 parent b8bbe96 commit e87134f
Show file tree
Hide file tree
Showing 2 changed files with 178 additions and 3 deletions.
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
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

0 comments on commit e87134f

Please sign in to comment.