Skip to content

Commit

Permalink
fix(@angular-devkit/build-angular): use babel default export helper i…
Browse files Browse the repository at this point in the history
…n build optimizer

Within the build optimizer's static member optimization pass, a class that is directly
default exported must be split into two statements: the class declaration and the
default export. This is because the pass can wrap classes in a pure annotated IIFE which
results in a variable declaration replacement and variable declarations can not be directly
default exported. Previously, the pass did this splitting manually but this was causing
later babel plugins to fail. In addition to updating the AST in this case, scoping information
also needed to be updated. To support this, a babel helper package is now used that handles
the details of the statement split operation.
  • Loading branch information
clydin authored and angular-robot[bot] committed Feb 10, 2023
1 parent c65b026 commit 8356240
Show file tree
Hide file tree
Showing 7 changed files with 64 additions and 29 deletions.
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@
"@babel/core": "7.20.12",
"@babel/generator": "7.20.14",
"@babel/helper-annotate-as-pure": "7.18.6",
"@babel/helper-split-export-declaration": "7.18.6",
"@babel/plugin-proposal-async-generator-functions": "7.20.7",
"@babel/plugin-transform-async-to-generator": "7.20.7",
"@babel/plugin-transform-runtime": "7.19.6",
Expand Down
1 change: 1 addition & 0 deletions packages/angular_devkit/build_angular/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,7 @@ ts_library(
"@npm//@babel/core",
"@npm//@babel/generator",
"@npm//@babel/helper-annotate-as-pure",
"@npm//@babel/helper-split-export-declaration",
"@npm//@babel/plugin-proposal-async-generator-functions",
"@npm//@babel/plugin-transform-async-to-generator",
"@npm//@babel/plugin-transform-runtime",
Expand Down
1 change: 1 addition & 0 deletions packages/angular_devkit/build_angular/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
"@babel/core": "7.20.12",
"@babel/generator": "7.20.14",
"@babel/helper-annotate-as-pure": "7.18.6",
"@babel/helper-split-export-declaration": "7.18.6",
"@babel/plugin-proposal-async-generator-functions": "7.20.7",
"@babel/plugin-transform-async-to-generator": "7.20.7",
"@babel/plugin-transform-runtime": "7.19.6",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

import { NodePath, PluginObj, PluginPass, types } from '@babel/core';
import annotateAsPure from '@babel/helper-annotate-as-pure';
import splitExportDeclaration from '@babel/helper-split-export-declaration';

/**
* The name of the Typescript decorator helper function created by the TypeScript compiler.
Expand Down Expand Up @@ -183,12 +184,18 @@ function analyzeClassSiblings(
}

/**
* The set of classed already visited and analyzed during the plugin's execution.
* The set of classes already visited and analyzed during the plugin's execution.
* This is used to prevent adjusted classes from being repeatedly analyzed which can lead
* to an infinite loop.
*/
const visitedClasses = new WeakSet<types.Class>();

/**
* A map of classes that have already been analyzed during the default export splitting step.
* This is used to avoid analyzing a class declaration twice if it is a direct default export.
*/
const exportDefaultAnalysis = new WeakMap<types.Class, ReturnType<typeof analyzeClassSiblings>>();

/**
* A babel plugin factory function for adjusting classes; primarily with Angular metadata.
* The adjustments include wrapping classes with known safe or no side effects with pure
Expand All @@ -201,6 +208,25 @@ const visitedClasses = new WeakSet<types.Class>();
export default function (): PluginObj {
return {
visitor: {
// When a class is converted to a variable declaration, the default export must be moved
// to a subsequent statement to prevent a JavaScript syntax error.
ExportDefaultDeclaration(path: NodePath<types.ExportDefaultDeclaration>, state: PluginPass) {
const declaration = path.get('declaration');
if (!declaration.isClassDeclaration()) {
return;
}

const { wrapDecorators } = state.opts as { wrapDecorators: boolean };
const analysis = analyzeClassSiblings(path, declaration.node.id, wrapDecorators);
exportDefaultAnalysis.set(declaration.node, analysis);

// Splitting the export declaration is not needed if the class will not be wrapped
if (analysis.hasPotentialSideEffects) {
return;
}

splitExportDeclaration(path);
},
ClassDeclaration(path: NodePath<types.ClassDeclaration>, state: PluginPass) {
const { node: classNode, parentPath } = path;
const { wrapDecorators } = state.opts as { wrapDecorators: boolean };
Expand All @@ -210,14 +236,10 @@ export default function (): PluginObj {
}

// Analyze sibling statements for elements of the class that were downleveled
const hasExport =
parentPath.isExportNamedDeclaration() || parentPath.isExportDefaultDeclaration();
const origin = hasExport ? parentPath : path;
const { wrapStatementPaths, hasPotentialSideEffects } = analyzeClassSiblings(
origin,
classNode.id,
wrapDecorators,
);
const origin = parentPath.isExportNamedDeclaration() ? parentPath : path;
const { wrapStatementPaths, hasPotentialSideEffects } =
exportDefaultAnalysis.get(classNode) ??
analyzeClassSiblings(origin, classNode.id, wrapDecorators);

visitedClasses.add(classNode);

Expand Down Expand Up @@ -288,18 +310,7 @@ export default function (): PluginObj {
const declaration = types.variableDeclaration('let', [
types.variableDeclarator(types.cloneNode(classNode.id), replacementInitializer),
]);
if (parentPath.isExportDefaultDeclaration()) {
// When converted to a variable declaration, the default export must be moved
// to a subsequent statement to prevent a JavaScript syntax error.
parentPath.replaceWithMultiple([
declaration,
types.exportNamedDeclaration(undefined, [
types.exportSpecifier(types.cloneNode(classNode.id), types.identifier('default')),
]),
]);
} else {
path.replaceWith(declaration);
}
path.replaceWith(declaration);
},
ClassExpression(path: NodePath<types.ClassExpression>, state: PluginPass) {
const { node: classNode, parentPath } = path;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -169,14 +169,27 @@ describe('adjust-static-class-members Babel plugin', () => {
});

it('does not wrap default exported class with no connected siblings', () => {
testCaseNoChange(`
export default class CustomComponentEffects {
constructor(_actions) {
this._actions = _actions;
this.doThis = this._actions;
// NOTE: This could technically have no changes but the default export splitting detection
// does not perform class property analysis currently.
testCase({
input: `
export default class CustomComponentEffects {
constructor(_actions) {
this._actions = _actions;
this.doThis = this._actions;
}
}
}
`);
`,
expected: `
class CustomComponentEffects {
constructor(_actions) {
this._actions = _actions;
this.doThis = this._actions;
}
}
export { CustomComponentEffects as default };
`,
});
});

it('does wrap not default exported class with only side effect fields', () => {
Expand Down
8 changes: 8 additions & 0 deletions packages/angular_devkit/build_angular/src/typings.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,3 +11,11 @@ declare module '@babel/helper-annotate-as-pure' {
pathOrNode: import('@babel/types').Node | { node: import('@babel/types').Node },
): void;
}

declare module '@babel/helper-split-export-declaration' {
export default function splitExportDeclaration(
exportDeclaration: import('@babel/traverse').NodePath<
import('@babel/types').ExportDefaultDeclaration
>,
): void;
}
2 changes: 1 addition & 1 deletion yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -581,7 +581,7 @@
dependencies:
"@babel/types" "^7.20.0"

"@babel/helper-split-export-declaration@^7.18.6":
"@babel/helper-split-export-declaration@7.18.6", "@babel/helper-split-export-declaration@^7.18.6":
version "7.18.6"
resolved "https://registry.yarnpkg.com/@babel/helper-split-export-declaration/-/helper-split-export-declaration-7.18.6.tgz#7367949bc75b20c6d5a5d4a97bba2824ae8ef075"
integrity sha512-bde1etTx6ZyTmobl9LLMMQsaizFVZrquTEHOqKeQESMKo4PlObf+8+JA25ZsIpZhT/WEd39+vOdLXAFG/nELpA==
Expand Down

0 comments on commit 8356240

Please sign in to comment.