fix(@ngtools/webpack): remove app decorators on AOT

packages/@ngtools/webpack/src/angular_compiler_plugin.ts

@@ -21,6 +21,7 @@ import {
   replaceBootstrap,
   exportNgFactory,
   exportLazyModuleMap,
+  removeDecorators,
   registerLocaleData,
   findResources,
   replaceResources,
@@ -637,10 +638,14 @@ export class AngularCompilerPlugin implements Tapable {
     const isMainPath = (fileName: string) => fileName === this._mainPath;
     const getEntryModule = () => this.entryModule;
     const getLazyRoutes = () => this._lazyRoutes;
+    const getTypeChecker = () => this._getTsProgram().getTypeChecker();
 
     if (this._JitMode) {
       // Replace resources in JIT.
       this._transformers.push(replaceResources(isAppPath));
+    } else {
+      // Remove unneeded angular decorators.
+      this._transformers.push(removeDecorators(isAppPath, getTypeChecker));
     }
 
     if (this._platform === PLATFORM.Browser) {
@@ -654,7 +659,7 @@ export class AngularCompilerPlugin implements Tapable {
 
       if (!this._JitMode) {
         // Replace bootstrap in browser AOT.
-        this._transformers.push(replaceBootstrap(isAppPath, getEntryModule));
+        this._transformers.push(replaceBootstrap(isAppPath, getEntryModule, getTypeChecker));
       }
     } else if (this._platform === PLATFORM.Server) {
       this._transformers.push(exportLazyModuleMap(isMainPath, getLazyRoutes));

packages/@ngtools/webpack/src/transformers/ast_helpers.ts

@@ -32,11 +32,11 @@ export function getLastNode(sourceFile: ts.SourceFile): ts.Node | null {
 }
 
 
-export function transformTypescript(
-  content: string,
-  transformers: ts.TransformerFactory<ts.SourceFile>[]
-) {
+// Test transform helpers.
+const basePath = '/project/src/';
+const fileName = basePath + 'test-file.ts';
 
+export function createTypescriptContext(content: string) {
   // Set compiler options.
   const compilerOptions: ts.CompilerOptions = {
     noEmitOnError: false,
@@ -49,15 +49,31 @@ export function transformTypescript(
   };
 
   // Create compiler host.
-  const basePath = '/project/src/';
   const compilerHost = new WebpackCompilerHost(compilerOptions, basePath);
 
   // Add a dummy file to host content.
-  const fileName = basePath + 'test-file.ts';
   compilerHost.writeFile(fileName, content, false);
 
   // Create the TypeScript program.
   const program = ts.createProgram([fileName], compilerOptions, compilerHost);
+  return { compilerHost, program };
+}
+
+export function transformTypescript(
+  content: string | undefined,
+  transformers: ts.TransformerFactory<ts.SourceFile>[],
+  program?: ts.Program,
+  compilerHost?: WebpackCompilerHost
+) {
+
+  // Use given context or create a new one.
+  if (content !== undefined) {
+    const typescriptContext = createTypescriptContext(content);
+    program = typescriptContext.program;
+    compilerHost = typescriptContext.compilerHost;
+  } else if (!program || !compilerHost) {
+    throw new Error('transformTypescript needs either `content` or a `program` and `compilerHost');
+  }
 
   // Emit.
   const { emitSkipped, diagnostics } = program.emit(

packages/@ngtools/webpack/src/transformers/elide_imports.ts

@@ -0,0 +1,121 @@
+// @ignoreDep typescript
+import * as ts from 'typescript';
+
+import { collectDeepNodes } from './ast_helpers';
+import { RemoveNodeOperation, TransformOperation } from './interfaces';
+
+
+interface RemovedSymbol {
+  symbol: ts.Symbol;
+  importDecl: ts.ImportDeclaration;
+  importSpec: ts.ImportSpecifier;
+  singleImport: boolean;
+  removed: ts.Identifier[];
+  all: ts.Identifier[];
+}
+
+// Remove imports for which all identifiers have been removed.
+// Needs type checker, and works even if it's not the first transformer.
+// Works by removing imports for symbols whose identifiers have all been removed.
+// Doesn't use the `symbol.declarations` because that previous transforms might have removed nodes
+// but the type checker doesn't know.
+// See https://github.com/Microsoft/TypeScript/issues/17552 for more information.
+export function elideImports(
+  sourceFile: ts.SourceFile,
+  removedNodes: ts.Node[],
+  getTypeChecker: () => ts.TypeChecker,
+): TransformOperation[] {
+  const ops: TransformOperation[] = [];
+
+  if (removedNodes.length === 0) {
+    return [];
+  }
+
+  // Get all children identifiers inside the removed nodes.
+  const removedIdentifiers = removedNodes
+    .map((node) => collectDeepNodes<ts.Identifier>(node, ts.SyntaxKind.Identifier))
+    .reduce((prev, curr) => prev.concat(curr), [])
+    // Also add the top level nodes themselves if they are identifiers.
+    .concat(removedNodes.filter((node) =>
+      node.kind === ts.SyntaxKind.Identifier) as ts.Identifier[]);
+
+  if (removedIdentifiers.length === 0) {
+    return [];
+  }
+
+  // Get all imports in the source file.
+  const allImports = collectDeepNodes<ts.ImportDeclaration>(
+    sourceFile, ts.SyntaxKind.ImportDeclaration);
+
+  if (allImports.length === 0) {
+    return [];
+  }
+
+  const removedSymbolMap: Map<string, RemovedSymbol> = new Map();
+  const typeChecker = getTypeChecker();
+
+  // Find all imports that use a removed identifier and add them to the map.
+  allImports
+    .filter((node: ts.ImportDeclaration) => {
+      // TODO: try to support removing `import * as X from 'XYZ'`.
+      // Filter out import statements that are either `import 'XYZ'` or `import * as X from 'XYZ'`.
+      const clause = node.importClause as ts.ImportClause;
+      if (!clause || clause.name || !clause.namedBindings) {
+        return false;
+      }
+      return clause.namedBindings.kind == ts.SyntaxKind.NamedImports;
+    })
+    .forEach((importDecl: ts.ImportDeclaration) => {
+      const importClause = importDecl.importClause as ts.ImportClause;
+      const namedImports = importClause.namedBindings as ts.NamedImports;
+
+      namedImports.elements.forEach((importSpec: ts.ImportSpecifier) => {
+        const importId = importSpec.name;
+        const symbol = typeChecker.getSymbolAtLocation(importId);
+
+        const removedNodesForImportId = removedIdentifiers.filter((id) =>
+          id.text === importId.text && typeChecker.getSymbolAtLocation(id) === symbol);
+
+        if (removedNodesForImportId.length > 0) {
+          removedSymbolMap.set(importId.text, {
+            symbol,
+            importDecl,
+            importSpec,
+            singleImport: namedImports.elements.length === 1,
+            removed: removedNodesForImportId,
+            all: []
+          });
+        }
+      });
+    });
+
+
+  if (removedSymbolMap.size === 0) {
+    return [];
+  }
+
+  // Find all identifiers in the source file that have a removed symbol, and add them to the map.
+  collectDeepNodes<ts.Identifier>(sourceFile, ts.SyntaxKind.Identifier)
+    .forEach((id) => {
+      if (removedSymbolMap.has(id.text)) {
+        const symbol = removedSymbolMap.get(id.text);
+        if (typeChecker.getSymbolAtLocation(id) === symbol.symbol) {
+          symbol.all.push(id);
+        }
+      }
+    });
+
+  Array.from(removedSymbolMap.values())
+    .filter((symbol) => {
+      // If the number of removed imports plus one (the import specifier) is equal to the total
+      // number of identifiers for that symbol, it's safe to remove the import.
+      return symbol.removed.length + 1 === symbol.all.length;
+    })
+    .forEach((symbol) => {
+      // Remove the whole declaration if it's a single import.
+      const nodeToRemove = symbol.singleImport ? symbol.importSpec : symbol.importDecl;
+      ops.push(new RemoveNodeOperation(sourceFile, nodeToRemove));
+    });
+
+  return ops;
+}

packages/@ngtools/webpack/src/transformers/index.ts

@@ -2,9 +2,10 @@ export * from './interfaces';
 export * from './ast_helpers';
 export * from './make_transform';
 export * from './insert_import';
-export * from './remove_import';
+export * from './elide_imports';
 export * from './replace_bootstrap';
 export * from './export_ngfactory';
 export * from './export_lazy_module_map';
 export * from './register_locale_data';
 export * from './replace_resources';
+export * from './remove_decorators';

packages/@ngtools/webpack/src/transformers/make_transform.ts

@@ -9,6 +9,7 @@ import {
   AddNodeOperation,
   ReplaceNodeOperation,
 } from './interfaces';
+import { elideImports } from './elide_imports';
 
 
 // Typescript below 2.5.0 needs a workaround.
@@ -17,7 +18,8 @@ const visitEachChild = satisfies(ts.version, '^2.5.0')
   : visitEachChildWorkaround;
 
 export function makeTransform(
-  standardTransform: StandardTransform
+  standardTransform: StandardTransform,
+  getTypeChecker?: () => ts.TypeChecker,
 ): ts.TransformerFactory<ts.SourceFile> {
 
   return (context: ts.TransformationContext): ts.Transformer<ts.SourceFile> => {
@@ -30,6 +32,16 @@ export function makeTransform(
       const replaceOps = ops
         .filter((op) => op.kind === OPERATION_KIND.Replace) as ReplaceNodeOperation[];
 
+      // If nodes are removed, elide the imports as well.
+      // Mainly a workaround for https://github.com/Microsoft/TypeScript/issues/17552.
+      // WARNING: this assumes that replaceOps DO NOT reuse any of the nodes they are replacing.
+      // This is currently true for transforms that use replaceOps (replace_bootstrap and
+      // replace_resources), but may not be true for new transforms.
+      if (getTypeChecker && removeOps.length + replaceOps.length > 0) {
+        const removedNodes = removeOps.concat(replaceOps).map((op) => op.target);
+        removeOps.push(...elideImports(sf, removedNodes, getTypeChecker));
+      }
+
       const visitor: ts.Visitor = (node) => {
         let modified = false;
         let modifiedNodes = [node];
@@ -66,8 +78,19 @@ export function makeTransform(
         }
       };
 
-      // Only visit source files we have ops for.
-      return ops.length > 0 ? ts.visitNode(sf, visitor) : sf;
+      // Don't visit the sourcefile at all if we don't have ops for it.
+      if (ops.length === 0) {
+        return sf;
+      }
+
+      const result = ts.visitNode(sf, visitor);
+
+      // If we removed any decorators, we need to clean up the decorator arrays.
+      if (removeOps.some((op) => op.target.kind === ts.SyntaxKind.Decorator)) {
+        cleanupDecorators(result);
+      }
+
+      return result;
     };
 
     return transformer;
@@ -104,3 +127,14 @@ function visitEachChildWorkaround(node: ts.Node, visitor: ts.Visitor,
 
   return ts.visitEachChild(node, visitor, context);
 }
+
+
+// If TS sees an empty decorator array, it will still emit a `__decorate` call.
+// This seems to be a TS bug.
+function cleanupDecorators(node: ts.Node) {
+  if (node.decorators && node.decorators.length == 0) {
+    node.decorators = undefined;
+  }
+
+  ts.forEachChild(node, node => cleanupDecorators(node));
+}

packages/@ngtools/webpack/src/transformers/multiple_transformers.spec.ts

@@ -1,19 +1,31 @@
 import { oneLine, stripIndent } from 'common-tags';
-import { transformTypescript } from './ast_helpers';
+import { createTypescriptContext, transformTypescript } from './ast_helpers';
 import { replaceBootstrap } from './replace_bootstrap';
 import { exportNgFactory } from './export_ngfactory';
 import { exportLazyModuleMap } from './export_lazy_module_map';
+import { removeDecorators } from './remove_decorators';
+
 
 describe('@ngtools/webpack transformers', () => {
   describe('multiple_transformers', () => {
     it('should apply multiple transformers on the same file', () => {
       const input = stripIndent`
         import { enableProdMode } from '@angular/core';
         import { platformBrowserDynamic } from '@angular/platform-browser-dynamic';
+        import { Component } from '@angular/core';
 
         import { AppModule } from './app/app.module';
         import { environment } from './environments/environment';
 
+        @Component({
+          selector: 'app-root',
+          templateUrl: './app.component.html',
+          styleUrls: ['./app.component.css']
+        })
+        class AppComponent {
+          title = 'app';
+        }
+
         if (environment.production) {
           enableProdMode();
         }
@@ -31,6 +43,11 @@ describe('@ngtools/webpack transformers', () => {
 
         import * as __NgCli_bootstrap_1 from "./app/app.module.ngfactory";
         import * as __NgCli_bootstrap_2 from "@angular/platform-browser";
+
+        class AppComponent {
+          constructor() { this.title = 'app'; }
+        }
+
         if (environment.production) {
           enableProdMode();
         }
@@ -40,12 +57,16 @@ describe('@ngtools/webpack transformers', () => {
       `;
       // tslint:enable:max-line-length
 
+      const { program, compilerHost } = createTypescriptContext(input);
+
       const shouldTransform = () => true;
       const getEntryModule = () =>
         ({ path: '/project/src/app/app.module', className: 'AppModule' });
+      const getTypeChecker = () => program.getTypeChecker();
+
 
       const transformers = [
-        replaceBootstrap(shouldTransform, getEntryModule),
+        replaceBootstrap(shouldTransform, getEntryModule, getTypeChecker),
         exportNgFactory(shouldTransform, getEntryModule),
         exportLazyModuleMap(shouldTransform,
           () => ({
@@ -54,9 +75,10 @@ describe('@ngtools/webpack transformers', () => {
             './lazy2/lazy2.module.ngfactory#LazyModule2NgFactory':
             '/project/src/app/lazy2/lazy2.module.ngfactory.ts',
           })),
+        removeDecorators(shouldTransform, getTypeChecker),
       ];
 
-      const result = transformTypescript(input, transformers);
+      const result = transformTypescript(undefined, transformers, program, compilerHost);
 
       expect(oneLine`${result}`).toEqual(oneLine`${output}`);
     });