Skip to content

Commit

Permalink
fix(@angular-devkit/build-angular): allow classes with pure annotated…
Browse files Browse the repository at this point in the history
… 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.

(cherry picked from commit 4bcd1dc)
  • Loading branch information
clydin authored and josephperrott committed Jul 21, 2021
1 parent 6df1f79 commit 04e9ffe
Show file tree
Hide file tree
Showing 2 changed files with 98 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,33 @@ export function getKeywords(): Iterable<string> {
return ['class'];
}

/**
* Determines whether a property and its initializer value can be safely wrapped in a pure
* annotated IIFE. Values that may cause side effects are not considered safe to wrap.
* Wrapping such values may cause runtime errors and/or incorrect runtime behavior.
*
* @param propertyName The name of the property to analyze.
* @param assignmentValue The initializer value that will be assigned to the property.
* @returns If the property can be safely wrapped, then true; otherwise, false.
*/
function canWrapProperty(propertyName: string, assignmentValue: NodePath): boolean {
if (angularStaticsToWrap.has(propertyName)) {
return true;
}

const { leadingComments } = assignmentValue.node as { leadingComments?: { value: string }[] };
if (
leadingComments?.some(
// `@pureOrBreakMyCode` is used by closure and is present in Angular code
({ value }) => value.includes('#__PURE__') || value.includes('@pureOrBreakMyCode'),
)
) {
return true;
}

return assignmentValue.isPure();
}

/**
* Analyze the sibling nodes of a class to determine if any downlevel elements should be
* wrapped in a pure annotated IIFE. Also determines if any elements have potential side
Expand Down Expand Up @@ -140,7 +167,7 @@ function analyzeClassSiblings(
if (angularStaticsToElide[propertyName]?.(assignmentValue)) {
nextStatement.remove();
--i;
} else if (angularStaticsToWrap.has(propertyName) || assignmentValue.isPure()) {
} else if (canWrapProperty(propertyName, assignmentValue)) {
wrapStatementPaths.push(nextStatement);
} else {
// Statement cannot be safely wrapped which makes wrapping the class unneeded.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@ describe('adjust-static-class-members Babel plugin', () => {
`);
});

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

it('does wrap not class with only side effect fields', () => {
testCaseNoChange(`
class CustomComponentEffects {
constructor(_actions) {
this._actions = _actions;
this.doThis = this._actions;
}
}
CustomComponentEffects.someFieldWithSideEffects = console.log('foo');
`);
});

it('wraps class with pure annotated side effect fields', () => {
testCase({
input: `
class CustomComponentEffects {
constructor(_actions) {
this._actions = _actions;
this.doThis = this._actions;
}
}
CustomComponentEffects.someFieldWithSideEffects = /*#__PURE__*/ console.log('foo');
`,
expected: `
let CustomComponentEffects = /*#__PURE__*/ (() => {
class CustomComponentEffects {
constructor(_actions) {
this._actions = _actions;
this.doThis = this._actions;
}
}
CustomComponentEffects.someFieldWithSideEffects = /*#__PURE__*/ console.log('foo');
return CustomComponentEffects;
})();
`,
});
});

it('wraps class with closure pure annotated side effect fields', () => {
testCase({
input: `
class CustomComponentEffects {
constructor(_actions) {
this._actions = _actions;
this.doThis = this._actions;
}
}
CustomComponentEffects.someFieldWithSideEffects = /* @pureOrBreakMyCode */ console.log('foo');
`,
expected: `
let CustomComponentEffects = /*#__PURE__*/ (() => {
class CustomComponentEffects {
constructor(_actions) {
this._actions = _actions;
this.doThis = this._actions;
}
}
CustomComponentEffects.someFieldWithSideEffects =
/* @pureOrBreakMyCode */
console.log('foo');
return CustomComponentEffects;
})();
`,
});
});

it('wraps exported class with a pure static field', () => {
testCase({
input: `
Expand Down

0 comments on commit 04e9ffe

Please sign in to comment.