From 384ad29c9a66d78e545ed7e48bf962e4df9d0549 Mon Sep 17 00:00:00 2001 From: Charles Lyding <19598772+clydin@users.noreply.github.com> Date: Thu, 9 Feb 2023 12:21:36 -0500 Subject: [PATCH] fix(@angular-devkit/build-angular): use babel default export helper in 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. (cherry picked from commit 8356240dda74f772435e7b0a639b2e928b61a657) --- package.json | 1 + .../angular_devkit/build_angular/BUILD.bazel | 1 + .../angular_devkit/build_angular/package.json | 1 + .../plugins/adjust-static-class-members.ts | 53 +++++++++++-------- .../adjust-static-class-members_spec.ts | 27 +++++++--- .../build_angular/src/typings.d.ts | 8 +++ yarn.lock | 2 +- 7 files changed, 64 insertions(+), 29 deletions(-) diff --git a/package.json b/package.json index eaf04344e4e6..f1211c77af22 100644 --- a/package.json +++ b/package.json @@ -84,6 +84,7 @@ "@babel/core": "7.20.12", "@babel/generator": "7.20.7", "@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", diff --git a/packages/angular_devkit/build_angular/BUILD.bazel b/packages/angular_devkit/build_angular/BUILD.bazel index 5fb3680e2a6a..6858186e725f 100644 --- a/packages/angular_devkit/build_angular/BUILD.bazel +++ b/packages/angular_devkit/build_angular/BUILD.bazel @@ -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", diff --git a/packages/angular_devkit/build_angular/package.json b/packages/angular_devkit/build_angular/package.json index d347658d9768..136dbde5067a 100644 --- a/packages/angular_devkit/build_angular/package.json +++ b/packages/angular_devkit/build_angular/package.json @@ -13,6 +13,7 @@ "@babel/core": "7.20.12", "@babel/generator": "7.20.7", "@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", diff --git a/packages/angular_devkit/build_angular/src/babel/plugins/adjust-static-class-members.ts b/packages/angular_devkit/build_angular/src/babel/plugins/adjust-static-class-members.ts index 35943ea9e5e7..4c9adc23c7bd 100644 --- a/packages/angular_devkit/build_angular/src/babel/plugins/adjust-static-class-members.ts +++ b/packages/angular_devkit/build_angular/src/babel/plugins/adjust-static-class-members.ts @@ -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. @@ -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(); +/** + * 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>(); + /** * 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 @@ -201,6 +208,25 @@ const visitedClasses = new WeakSet(); 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, 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, state: PluginPass) { const { node: classNode, parentPath } = path; const { wrapDecorators } = state.opts as { wrapDecorators: boolean }; @@ -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); @@ -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, state: PluginPass) { const { node: classNode, parentPath } = path; diff --git a/packages/angular_devkit/build_angular/src/babel/plugins/adjust-static-class-members_spec.ts b/packages/angular_devkit/build_angular/src/babel/plugins/adjust-static-class-members_spec.ts index a62f87d38504..4b078a0f02b1 100644 --- a/packages/angular_devkit/build_angular/src/babel/plugins/adjust-static-class-members_spec.ts +++ b/packages/angular_devkit/build_angular/src/babel/plugins/adjust-static-class-members_spec.ts @@ -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', () => { diff --git a/packages/angular_devkit/build_angular/src/typings.d.ts b/packages/angular_devkit/build_angular/src/typings.d.ts index 204fa0d207d0..2468d5b261ba 100644 --- a/packages/angular_devkit/build_angular/src/typings.d.ts +++ b/packages/angular_devkit/build_angular/src/typings.d.ts @@ -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; +} diff --git a/yarn.lock b/yarn.lock index 21f42640bd0e..f7fe3caa08ac 100644 --- a/yarn.lock +++ b/yarn.lock @@ -556,7 +556,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==