fix(@angular-devkit/build-optimizer): identify relative imports in angular core

Build optimizer was broken for non-FESM files inside @angular/core because it couldn't identify relative imports were still inside core.

This change adds a known list of angular core files as a default, and also allows passing in a override.

Author: filipesilva

etc/api/angular_devkit/build_optimizer/src/_golden-api.d.ts

@@ -12,6 +12,8 @@ export declare function getPrefixFunctionsTransformer(): ts.TransformerFactory<t
 
 export declare function getScrubFileTransformer(program: ts.Program): ts.TransformerFactory<ts.SourceFile>;
 
+export declare function getScrubFileTransformerForCore(program: ts.Program): ts.TransformerFactory<ts.SourceFile>;
+
 export declare function getWrapEnumsTransformer(): ts.TransformerFactory<ts.SourceFile>;
 
 export declare function testImportTslib(content: string): boolean;

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

@@ -15,7 +15,11 @@ import { getFoldFileTransformer } from '../transforms/class-fold';
 import { getImportTslibTransformer, testImportTslib } from '../transforms/import-tslib';
 import { getPrefixClassesTransformer, testPrefixClasses } from '../transforms/prefix-classes';
 import { getPrefixFunctionsTransformer } from '../transforms/prefix-functions';
-import { getScrubFileTransformer, testScrubFile } from '../transforms/scrub-file';
+import {
+  getScrubFileTransformer,
+  getScrubFileTransformerForCore,
+  testScrubFile,
+} from '../transforms/scrub-file';
 import { getWrapEnumsTransformer } from '../transforms/wrap-enums';
 
 
@@ -44,6 +48,18 @@ const ngFactories = [
   /\.ngstyle\.[jt]s/,
 ];
 
+// Known locations for the source files of @angular/core.
+const coreFilesRegex = [
+  /[\\/]node_modules[\\/]@angular[\\/]core[\\/]esm5[\\/]/,
+  /[\\/]node_modules[\\/]@angular[\\/]core[\\/]fesm5[\\/]/,
+  /[\\/]node_modules[\\/]@angular[\\/]core[\\/]esm2015[\\/]/,
+  /[\\/]node_modules[\\/]@angular[\\/]core[\\/]fesm2015[\\/]/,
+];
+
+function isKnownCoreFile(filePath: string) {
+  return coreFilesRegex.some(re => re.test(filePath));
+}
+
 function isKnownSideEffectFree(filePath: string) {
   return ngFactories.some((re) => re.test(filePath)) ||
     whitelistedAngularModules.some((re) => re.test(filePath));
@@ -57,11 +73,12 @@ export interface BuildOptimizerOptions {
   emitSourceMap?: boolean;
   strict?: boolean;
   isSideEffectFree?: boolean;
+  isAngularCoreFile?: boolean;
 }
 
 export function buildOptimizer(options: BuildOptimizerOptions): TransformJavascriptOutput {
 
-  const { inputFilePath } = options;
+  const { inputFilePath, isAngularCoreFile } = options;
   let { originalFilePath, content } = options;
 
   if (!originalFilePath && inputFilePath) {
@@ -84,6 +101,15 @@ export function buildOptimizer(options: BuildOptimizerOptions): TransformJavascr
     };
   }
 
+  let selectedGetScrubFileTransformer = getScrubFileTransformer;
+
+  if (
+    isAngularCoreFile === true ||
+    (isAngularCoreFile === undefined && originalFilePath && isKnownCoreFile(originalFilePath))
+  ) {
+    selectedGetScrubFileTransformer = getScrubFileTransformerForCore;
+  }
+
   const isWebpackBundle = content.indexOf('__webpack_require__') !== -1;
 
   // Determine which transforms to apply.
@@ -97,14 +123,14 @@ export function buildOptimizer(options: BuildOptimizerOptions): TransformJavascr
       // We only apply it to whitelisted modules, since we know they are safe.
       // getPrefixFunctionsTransformer needs to be before getFoldFileTransformer.
       getPrefixFunctionsTransformer,
-      getScrubFileTransformer,
+      selectedGetScrubFileTransformer,
       getFoldFileTransformer,
     );
     typeCheck = true;
   } else if (testScrubFile(content)) {
     // Always test as these require the type checker
     getTransforms.push(
-      getScrubFileTransformer,
+      selectedGetScrubFileTransformer,
       getFoldFileTransformer,
     );
     typeCheck = true;

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

@@ -20,6 +20,7 @@ const DEBUG = false;
 
 export interface Options {
   sideEffectFreeModules?: string[];
+  angularCoreModules?: string[];
 }
 
 export default function optimizer(options: Options) {
@@ -34,8 +35,10 @@ export default function optimizer(options: Options) {
       const normalizedId = id.replace(/\\/g, '/');
       const isSideEffectFree = options.sideEffectFreeModules &&
         options.sideEffectFreeModules.some(m => normalizedId.indexOf(m) >= 0);
+      const isAngularCoreFile = options.angularCoreModules &&
+        options.angularCoreModules.some(m => normalizedId.indexOf(m) >= 0);
       const { content: code, sourceMap: map } = buildOptimizer({
-        content, inputFilePath: id, emitSourceMap: true, isSideEffectFree,
+        content, inputFilePath: id, emitSourceMap: true, isSideEffectFree, isAngularCoreFile,
       });
       if (!code) {
         if (DEBUG) {

packages/angular_devkit/build_optimizer/src/index.ts

@@ -14,5 +14,9 @@ export { getFoldFileTransformer } from './transforms/class-fold';
 export { getImportTslibTransformer, testImportTslib } from './transforms/import-tslib';
 export { getPrefixClassesTransformer, testPrefixClasses } from './transforms/prefix-classes';
 export { getPrefixFunctionsTransformer } from './transforms/prefix-functions';
-export { getScrubFileTransformer, testScrubFile } from './transforms/scrub-file';
+export {
+  getScrubFileTransformer,
+  getScrubFileTransformerForCore,
+  testScrubFile,
+} from './transforms/scrub-file';
 export { getWrapEnumsTransformer } from './transforms/wrap-enums';

packages/angular_devkit/build_optimizer/src/transforms/scrub-file.ts

@@ -50,13 +50,21 @@ const angularSpecifiers = [
 ];
 
 export function getScrubFileTransformer(program: ts.Program): ts.TransformerFactory<ts.SourceFile> {
-  const checker = program.getTypeChecker();
+  return scrubFileTransformer(program.getTypeChecker(), false);
+}
+
+export function getScrubFileTransformerForCore(
+  program: ts.Program,
+): ts.TransformerFactory<ts.SourceFile> {
+  return scrubFileTransformer(program.getTypeChecker(), true);
+}
 
+function scrubFileTransformer(checker: ts.TypeChecker, isAngularCoreFile: boolean) {
   return (context: ts.TransformationContext): ts.Transformer<ts.SourceFile> => {
 
     const transformer: ts.Transformer<ts.SourceFile> = (sf: ts.SourceFile) => {
 
-      const ngMetadata = findAngularMetadata(sf);
+      const ngMetadata = findAngularMetadata(sf, isAngularCoreFile);
       const tslibImports = findTslibImports(sf);
 
       const nodes: ts.Node[] = [];
@@ -116,22 +124,27 @@ function nameOfSpecifier(node: ts.ImportSpecifier): string {
   return node.name && node.name.text || '<unknown>';
 }
 
-function findAngularMetadata(node: ts.Node): ts.Node[] {
+function findAngularMetadata(node: ts.Node, isAngularCoreFile: boolean): ts.Node[] {
   let specs: ts.Node[] = [];
+  // Find all specifiers from imports of `@angular/core`.
   ts.forEachChild(node, (child) => {
     if (child.kind === ts.SyntaxKind.ImportDeclaration) {
       const importDecl = child as ts.ImportDeclaration;
-      if (isAngularCoreImport(importDecl)) {
+      if (isAngularCoreImport(importDecl, isAngularCoreFile)) {
         specs.push(...collectDeepNodes<ts.ImportSpecifier>(node, ts.SyntaxKind.ImportSpecifier)
           .filter((spec) => isAngularCoreSpecifier(spec)));
       }
     }
   });
 
-  const localDecl = findAllDeclarations(node)
-    .filter((decl) => angularSpecifiers.indexOf((decl.name as ts.Identifier).text) !== -1);
-  if (localDecl.length === angularSpecifiers.length) {
-    specs = specs.concat(localDecl);
+  // Check if the current module contains all know `@angular/core` specifiers.
+  // If it does, we assume it's a `@angular/core` FESM.
+  if (isAngularCoreFile) {
+    const localDecl = findAllDeclarations(node)
+      .filter((decl) => angularSpecifiers.indexOf((decl.name as ts.Identifier).text) !== -1);
+    if (localDecl.length === angularSpecifiers.length) {
+      specs = specs.concat(localDecl);
+    }
   }
 
   return specs;
@@ -154,11 +167,23 @@ function findAllDeclarations(node: ts.Node): ts.VariableDeclaration[] {
   return nodes;
 }
 
-function isAngularCoreImport(node: ts.ImportDeclaration): boolean {
-  return true &&
-    node.moduleSpecifier &&
-    node.moduleSpecifier.kind === ts.SyntaxKind.StringLiteral &&
-    (node.moduleSpecifier as ts.StringLiteral).text === '@angular/core';
+function isAngularCoreImport(node: ts.ImportDeclaration, isAngularCoreFile: boolean): boolean {
+  if (!(node.moduleSpecifier && ts.isStringLiteral(node.moduleSpecifier))) {
+    return false;
+  }
+  const importText = node.moduleSpecifier.text;
+
+  // Imports to `@angular/core` are always core imports.
+  if (importText === '@angular/core') {
+    return true;
+  }
+
+  // Relative imports from a Angular core file are also core imports.
+  if (isAngularCoreFile && (importText.startsWith('./') || importText.startsWith('../'))) {
+    return true;
+  }
+
+  return false;
 }
 
 function isAngularCoreSpecifier(node: ts.ImportSpecifier): boolean {

packages/angular_devkit/build_optimizer/src/transforms/scrub-file_spec.ts

@@ -9,11 +9,17 @@
 // tslint:disable-next-line:no-implicit-dependencies
 import { tags } from '@angular-devkit/core';
 import { transformJavascript } from '../helpers/transform-javascript';
-import { getScrubFileTransformer, testScrubFile } from './scrub-file';
+import {
+  getScrubFileTransformer,
+  getScrubFileTransformerForCore,
+  testScrubFile,
+} from './scrub-file';
 
 
 const transform = (content: string) => transformJavascript(
   { content, getTransforms: [getScrubFileTransformer], typeCheck: true }).content;
+const transformCore = (content: string) => transformJavascript(
+  { content, getTransforms: [getScrubFileTransformerForCore], typeCheck: true }).content;
 
 describe('scrub-file', () => {
   const clazz = 'var Clazz = (function () { function Clazz() { } return Clazz; }());';
@@ -289,6 +295,47 @@ describe('scrub-file', () => {
       expect(testScrubFile(input)).toBeTruthy();
       expect(tags.oneLine`${transform(input)}`).toEqual(tags.oneLine`${output}`);
     });
+
+    it('recognizes decorator imports in Angular core', () => {
+      const input = tags.stripIndent`
+        import * as tslib_1 from "tslib";
+        import { Injectable } from './di';
+        var Console = /** @class */ (function () {
+            function Console() {
+            }
+            Console.prototype.log = function (message) {
+                console.log(message);
+            };
+            Console.prototype.warn = function (message) {
+                console.warn(message);
+            };
+            Console = tslib_1.__decorate([
+                Injectable()
+            ], Console);
+            return Console;
+        }());
+        export { Console };
+      `;
+      const output = tags.stripIndent`
+        import * as tslib_1 from "tslib";
+        import { Injectable } from './di';
+        var Console = /** @class */ (function () {
+            function Console() {
+            }
+            Console.prototype.log = function (message) {
+                console.log(message);
+            };
+            Console.prototype.warn = function (message) {
+                console.warn(message);
+            };
+            return Console;
+        }());
+        export { Console };
+      `;
+
+      expect(testScrubFile(input)).toBeTruthy();
+      expect(tags.oneLine`${transformCore(input)}`).toEqual(tags.oneLine`${output}`);
+    });
   });
 
   describe('__metadata', () => {