Skip to content

Commit 4d7cd5e

Browse files
committed
fix(@angular/build): correctly wrap class expressions with static properties 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 #27662 (cherry picked from commit ac9fc72)
1 parent decca6e commit 4d7cd5e

File tree

2 files changed

+117
-84
lines changed

2 files changed

+117
-84
lines changed

packages/angular/build/src/tools/babel/plugins/adjust-static-class-members.ts

Lines changed: 90 additions & 84 deletions
Original file line numberDiff line numberDiff line change
@@ -245,87 +245,12 @@ export default function (): PluginObj {
245245

246246
visitedClasses.add(classNode);
247247

248-
if (hasPotentialSideEffects) {
249-
return;
250-
}
251-
252248
// If no statements to wrap, check for static class properties.
253-
// Static class properties may be downleveled at later stages in the build pipeline
254-
// which results in additional function calls outside the class body. These calls
255-
// then cause the class to be referenced and not eligible for removal. Since it is
256-
// not known at this stage whether the class needs to be downleveled, the transform
257-
// wraps classes preemptively to allow for potential removal within the optimization
258-
// stages.
259-
if (wrapStatementPaths.length === 0) {
260-
let shouldWrap = false;
261-
for (const element of path.get('body').get('body')) {
262-
if (element.isClassProperty()) {
263-
// Only need to analyze static properties
264-
if (!element.node.static) {
265-
continue;
266-
}
267-
268-
// Check for potential side effects.
269-
// These checks are conservative and could potentially be expanded in the future.
270-
const elementKey = element.get('key');
271-
const elementValue = element.get('value');
272-
if (
273-
elementKey.isIdentifier() &&
274-
(!elementValue.isExpression() ||
275-
canWrapProperty(elementKey.node.name, elementValue))
276-
) {
277-
shouldWrap = true;
278-
} else {
279-
// Not safe to wrap
280-
shouldWrap = false;
281-
break;
282-
}
283-
// eslint-disable-next-line @typescript-eslint/no-explicit-any
284-
} else if ((element as any).isStaticBlock()) {
285-
// Only need to analyze static blocks
286-
const body = element.get('body');
287-
288-
if (Array.isArray(body) && body.length > 1) {
289-
// Not safe to wrap
290-
shouldWrap = false;
291-
break;
292-
}
293-
294-
const expression = body.find((n: NodePath<types.Node>) =>
295-
n.isExpressionStatement(),
296-
) as NodePath<types.ExpressionStatement> | undefined;
297-
298-
const assignmentExpression = expression?.get('expression');
299-
if (assignmentExpression?.isAssignmentExpression()) {
300-
const left = assignmentExpression.get('left');
301-
if (!left.isMemberExpression()) {
302-
continue;
303-
}
304-
305-
if (!left.get('object').isThisExpression()) {
306-
// Not safe to wrap
307-
shouldWrap = false;
308-
break;
309-
}
310-
311-
const element = left.get('property');
312-
const right = assignmentExpression.get('right');
313-
if (
314-
element.isIdentifier() &&
315-
(!right.isExpression() || canWrapProperty(element.node.name, right))
316-
) {
317-
shouldWrap = true;
318-
} else {
319-
// Not safe to wrap
320-
shouldWrap = false;
321-
break;
322-
}
323-
}
324-
}
325-
}
326-
if (!shouldWrap) {
327-
return;
328-
}
249+
if (
250+
hasPotentialSideEffects ||
251+
(wrapStatementPaths.length === 0 && !analyzeClassStaticProperties(path).shouldWrap)
252+
) {
253+
return;
329254
}
330255

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

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

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

383306
visitedClasses.add(classNode);
384307

385-
if (hasPotentialSideEffects || wrapStatementPaths.length === 0) {
308+
// If no statements to wrap, check for static class properties.
309+
if (
310+
hasPotentialSideEffects ||
311+
(wrapStatementPaths.length === 0 && !analyzeClassStaticProperties(path).shouldWrap)
312+
) {
386313
return;
387314
}
388315

@@ -416,3 +343,82 @@ export default function (): PluginObj {
416343
},
417344
};
418345
}
346+
347+
/**
348+
* Static class properties may be downleveled at later stages in the build pipeline
349+
* which results in additional function calls outside the class body. These calls
350+
* then cause the class to be referenced and not eligible for removal. Since it is
351+
* not known at this stage whether the class needs to be downleveled, the transform
352+
* wraps classes preemptively to allow for potential removal within the optimization stages.
353+
*/
354+
function analyzeClassStaticProperties(
355+
path: NodePath<types.ClassDeclaration | types.ClassExpression>,
356+
): { shouldWrap: boolean } {
357+
let shouldWrap = false;
358+
for (const element of path.get('body').get('body')) {
359+
if (element.isClassProperty()) {
360+
// Only need to analyze static properties
361+
if (!element.node.static) {
362+
continue;
363+
}
364+
365+
// Check for potential side effects.
366+
// These checks are conservative and could potentially be expanded in the future.
367+
const elementKey = element.get('key');
368+
const elementValue = element.get('value');
369+
if (
370+
elementKey.isIdentifier() &&
371+
(!elementValue.isExpression() || canWrapProperty(elementKey.node.name, elementValue))
372+
) {
373+
shouldWrap = true;
374+
} else {
375+
// Not safe to wrap
376+
shouldWrap = false;
377+
break;
378+
}
379+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
380+
} else if ((element as any).isStaticBlock()) {
381+
// Only need to analyze static blocks
382+
const body = element.get('body');
383+
384+
if (Array.isArray(body) && body.length > 1) {
385+
// Not safe to wrap
386+
shouldWrap = false;
387+
break;
388+
}
389+
390+
const expression = body.find((n: NodePath<types.Node>) => n.isExpressionStatement()) as
391+
| NodePath<types.ExpressionStatement>
392+
| undefined;
393+
394+
const assignmentExpression = expression?.get('expression');
395+
if (assignmentExpression?.isAssignmentExpression()) {
396+
const left = assignmentExpression.get('left');
397+
if (!left.isMemberExpression()) {
398+
continue;
399+
}
400+
401+
if (!left.get('object').isThisExpression()) {
402+
// Not safe to wrap
403+
shouldWrap = false;
404+
break;
405+
}
406+
407+
const element = left.get('property');
408+
const right = assignmentExpression.get('right');
409+
if (
410+
element.isIdentifier() &&
411+
(!right.isExpression() || canWrapProperty(element.node.name, right))
412+
) {
413+
shouldWrap = true;
414+
} else {
415+
// Not safe to wrap
416+
shouldWrap = false;
417+
break;
418+
}
419+
}
420+
}
421+
}
422+
423+
return { shouldWrap };
424+
}

packages/angular/build/src/tools/babel/plugins/adjust-static-class-members_spec.ts

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -595,6 +595,33 @@ describe('adjust-static-class-members Babel plugin', () => {
595595
}),
596596
);
597597

598+
it(
599+
'wraps class with Angular ɵfac static field (esbuild)',
600+
testCase({
601+
input: `
602+
var Comp2Component = class _Comp2Component {
603+
static {
604+
this.ɵfac = function Comp2Component_Factory(t) {
605+
return new (t || _Comp2Component)();
606+
};
607+
}
608+
};
609+
`,
610+
expected: `
611+
var Comp2Component = /*#__PURE__*/ (() => {
612+
let Comp2Component = class _Comp2Component {
613+
static {
614+
this.ɵfac = function Comp2Component_Factory(t) {
615+
return new (t || _Comp2Component)();
616+
};
617+
}
618+
};
619+
return Comp2Component;
620+
})();
621+
`,
622+
}),
623+
);
624+
598625
it(
599626
'wraps class with class decorators when wrapDecorators is true (esbuild output)',
600627
testCase({

0 commit comments

Comments
 (0)