Skip to content

Commit

Permalink
fix(@angular/build): correctly wrap class expressions with static pro…
Browse files Browse the repository at this point in the history 
…perties or blocks emitted by esbuild

Prior to this commit, class expressions emitted by esbuild that have static blocks or properties were not being properly wrapped. This caused the class to become non-treeshakable.

Closes angular#27662
  • Loading branch information
@alan-agius4
alan-agius4 committed May 21, 2024
1 parent ebfd7b1 commit 5104fbc
Show file tree
Hide file tree
Showing 2 changed files with 117 additions and 84 deletions.
174 changes: 90 additions & 84 deletions packages/angular/build/src/tools/babel/plugins/adjust-static-class-members.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -245,87 +245,12 @@ export default function (): PluginObj {

visitedClasses.add(classNode);

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.node.name, elementValue))
) {
shouldWrap = true;
} else {
// Not safe to wrap
shouldWrap = false;
break;
}
// eslint-disable-next-line @typescript-eslint/no-explicit-any
} else if ((element as any).isStaticBlock()) {
// Only need to analyze static blocks
const body = element.get('body');

if (Array.isArray(body) && body.length > 1) {
// Not safe to wrap
shouldWrap = false;
break;
}

const expression = body.find((n: NodePath<types.Node>) =>
n.isExpressionStatement(),
) as NodePath<types.ExpressionStatement> | undefined;

const assignmentExpression = expression?.get('expression');
if (assignmentExpression?.isAssignmentExpression()) {
const left = assignmentExpression.get('left');
if (!left.isMemberExpression()) {
continue;
}

if (!left.get('object').isThisExpression()) {
// Not safe to wrap
shouldWrap = false;
break;
}

const element = left.get('property');
const right = assignmentExpression.get('right');
if (
element.isIdentifier() &&
(!right.isExpression() || canWrapProperty(element.node.name, right))
) {
shouldWrap = true;
} else {
// Not safe to wrap
shouldWrap = false;
break;
}
}
}
}
if (!shouldWrap) {
return;
}
if (
hasPotentialSideEffects ||
(wrapStatementPaths.length === 0 && !analyzeClassStaticProperties(path).shouldWrap)
) {
return;
}

const wrapStatementNodes: types.Statement[] = [];
Expand Down Expand Up @@ -359,9 +284,7 @@ export default function (): PluginObj {
const { node: classNode, parentPath } = path;
const { wrapDecorators } = state.opts as { wrapDecorators: boolean };

// Class expressions are used by TypeScript to represent downlevel class/constructor decorators.
// If not wrapping decorators, they do not need to be processed.
if (!wrapDecorators || visitedClasses.has(classNode)) {
if (visitedClasses.has(classNode)) {
return;
}

Expand All @@ -382,7 +305,11 @@ export default function (): PluginObj {

visitedClasses.add(classNode);

if (hasPotentialSideEffects || wrapStatementPaths.length === 0) {
// If no statements to wrap, check for static class properties.
if (
hasPotentialSideEffects ||
(wrapStatementPaths.length === 0 && !analyzeClassStaticProperties(path).shouldWrap)
) {
return;
}

Expand Down Expand Up @@ -416,3 +343,82 @@ export default function (): PluginObj {
},
};
}

/**
* 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.
*/
function analyzeClassStaticProperties(
path: NodePath<types.ClassDeclaration | types.ClassExpression>,
): { shouldWrap: boolean } {
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.node.name, elementValue))
) {
shouldWrap = true;
} else {
// Not safe to wrap
shouldWrap = false;
break;
}
// eslint-disable-next-line @typescript-eslint/no-explicit-any
} else if ((element as any).isStaticBlock()) {
// Only need to analyze static blocks
const body = element.get('body');

if (Array.isArray(body) && body.length > 1) {
// Not safe to wrap
shouldWrap = false;
break;
}

const expression = body.find((n: NodePath<types.Node>) => n.isExpressionStatement()) as
| NodePath<types.ExpressionStatement>
| undefined;

const assignmentExpression = expression?.get('expression');
if (assignmentExpression?.isAssignmentExpression()) {
const left = assignmentExpression.get('left');
if (!left.isMemberExpression()) {
continue;
}

if (!left.get('object').isThisExpression()) {
// Not safe to wrap
shouldWrap = false;
break;
}

const element = left.get('property');
const right = assignmentExpression.get('right');
if (
element.isIdentifier() &&
(!right.isExpression() || canWrapProperty(element.node.name, right))
) {
shouldWrap = true;
} else {
// Not safe to wrap
shouldWrap = false;
break;
}
}
}
}

return { shouldWrap };
}
27 changes: 27 additions & 0 deletions packages/angular/build/src/tools/babel/plugins/adjust-static-class-members_spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -595,6 +595,33 @@ describe('adjust-static-class-members Babel plugin', () => {
}),
);

it(
'wraps class with Angular ɵfac static field (esbuild)',
testCase({
input: `
var Comp2Component = class _Comp2Component {
static {
this.ɵfac = function Comp2Component_Factory(t) {
return new (t || _Comp2Component)();
};
}
};
`,
expected: `
var Comp2Component = /*#__PURE__*/ (() => {
let Comp2Component = class _Comp2Component {
static {
this.ɵfac = function Comp2Component_Factory(t) {
return new (t || _Comp2Component)();
};
}
};
return Comp2Component;
})();
`,
}),
);

it(
'wraps class with class decorators when wrapDecorators is true (esbuild output)',
testCase({
Expand Down

0 comments on commit 5104fbc

Please sign in to comment.