Build Optimizer ES2015 fixes

packages/angular_devkit/build_optimizer/src/build-optimizer/build-optimizer.ts

@@ -37,30 +37,16 @@ const whitelistedAngularModules = [
   /[\\/]node_modules[\\/]@angular[\\/]cdk[\\/]/,
 ];
 
-// TODO: this code is very fragile and should be reworked.
-//       See: https://github.com/angular/devkit/issues/523
-const es5AngularModules = [
-  // Angular 4 packaging format has .es5.js as the extension.
-  /\.es5\.js$/, // Angular 4
-  // Angular 5 has esm5 folders.
-  // Angular 6 has fesm5 folders.
-  /[\\/]node_modules[\\/]@angular[\\/][^\\/]+[\\/]f?esm5[\\/]/,
-  // All Angular versions have UMD with es5.
-  /\.umd\.js$/,
-];
-
-// Factories created by AOT are known to have no side effects and contain es5 code.
+// Factories created by AOT are known to have no side effects.
 // In Angular 2/4 the file path for factories can be `.ts`, but in Angular 5 it is `.js`.
 const ngFactories = [
   /\.ngfactory\.[jt]s/,
   /\.ngstyle\.[jt]s/,
 ];
 
 function isKnownSideEffectFree(filePath: string) {
-  return ngFactories.some((re) => re.test(filePath)) || (
-    whitelistedAngularModules.some((re) => re.test(filePath))
-    && es5AngularModules.some((re) => re.test(filePath))
-  );
+  return ngFactories.some((re) => re.test(filePath)) ||
+    whitelistedAngularModules.some((re) => re.test(filePath));
 }
 
 export interface BuildOptimizerOptions {

packages/angular_devkit/build_optimizer/src/build-optimizer/build-optimizer_spec.ts

@@ -89,24 +89,6 @@ describe('build-optimizer', () => {
       });
     });
 
-    it('supports es2015 modules', () => {
-      // prefix-functions would add PURE_IMPORTS_START and PURE to the super call.
-      // This test ensures it isn't applied to es2015 modules.
-      const output = tags.oneLine`
-        import { Injectable } from '@angular/core';
-        class Clazz extends BaseClazz { constructor(e) { super(e); } }
-      `;
-      const input = tags.stripIndent`
-        ${output}
-        Clazz.ctorParameters = () => [ { type: Injectable } ];
-      `;
-
-      const inputFilePath = '/node_modules/@angular/core/@angular/core.js';
-      const boOutput = buildOptimizer({ content: input, inputFilePath });
-      expect(tags.oneLine`${boOutput.content}`).toEqual(output);
-      expect(boOutput.emitSkipped).toEqual(false);
-    });
-
     it('supports flagging module as side-effect free', () => {
       const output = tags.oneLine`
         /** PURE_IMPORTS_START  PURE_IMPORTS_END */

packages/angular_devkit/build_optimizer/src/transforms/prefix-functions.ts

@@ -53,8 +53,17 @@ export function findTopLevelFunctions(parentNode: ts.Node): Set<ts.Node> {
   const topLevelFunctions = new Set<ts.Node>();
 
   function cb(node: ts.Node) {
-    // Stop recursing into this branch if it's a function expression or declaration
-    if (ts.isFunctionDeclaration(node) || ts.isFunctionExpression(node)) {
+    // Stop recursing into this branch if it's a definition construct.
+    // These are function expression, function declaration, class, or arrow function (lambda).
+    // The body of these constructs will not execute when loading the module, so we don't
+    // need to mark function calls inside them as pure.
+    // Class static initializers in ES2015 are an exception we don't cover. They would need similar
+    // processing as enums to prevent property setting from causing the class to be retained.
+    if (ts.isFunctionDeclaration(node)
+      || ts.isFunctionExpression(node)
+      || ts.isClassDeclaration(node)
+      || ts.isArrowFunction(node)
+    ) {
       return;
     }
 

packages/angular_devkit/build_optimizer/src/transforms/prefix-functions_spec.ts

@@ -124,5 +124,43 @@ describe('prefix-functions', () => {
 
       expect(tags.oneLine`${transform(input)}`).toEqual(tags.oneLine`${output}`);
     });
+
+    it('doesn\'t add comment when inside class', () => {
+      const input = tags.stripIndent`
+        class Foo {
+          constructor(e) {
+            super(e);
+          }
+          method() {
+            var newClazz = new Clazz();
+          }
+        }
+      `;
+      const output = tags.stripIndent`
+        ${emptyImportsComment}
+        ${input}
+      `;
+
+      expect(tags.oneLine`${transform(input)}`).toEqual(tags.oneLine`${output}`);
+    });
+
+    it('doesn\'t add comment when inside arrow function', () => {
+      const input = tags.stripIndent`
+        export const subscribeToArray = (array) => (subscriber) => {
+            for (let i = 0, len = array.length; i < len && !subscriber.closed; i++) {
+                subscriber.next(array[i]);
+            }
+            if (!subscriber.closed) {
+                subscriber.complete();
+            }
+        };
+      `;
+      const output = tags.stripIndent`
+        ${emptyImportsComment}
+        ${input}
+      `;
+
+      expect(tags.oneLine`${transform(input)}`).toEqual(tags.oneLine`${output}`);
+    });
   });
 });