fix(ngc): add an option to produce TS1.9-pathMapping imports (#10602)

This fixes a regression in #10486

modules/@angular/compiler-cli/integrationtest/tsconfig.json

@@ -13,7 +13,6 @@
         "moduleResolution": "node",
         "rootDir": "",
         "declaration": true,
-        "lib": ["es6", "dom"],
+        "lib": ["es6", "dom"]
-        "baseUrl": "."
     }
 }

modules/@angular/compiler-cli/src/codegen.ts

@@ -18,12 +18,10 @@ import * as ts from 'typescript';
 
 import {CompileMetadataResolver, DirectiveNormalizer, DomElementSchemaRegistry, HtmlParser, Lexer, NgModuleCompiler, Parser, StyleCompiler, TemplateParser, TypeScriptEmitter, ViewCompiler} from './compiler_private';
 import {Console} from './core_private';
-import {ReflectorHost, ReflectorHostContext} from './reflector_host';
+import {GENERATED_FILES, ReflectorHost, ReflectorHostContext} from './reflector_host';
 import {StaticAndDynamicReflectionCapabilities} from './static_reflection_capabilities';
 import {StaticReflector, StaticSymbol} from './static_reflector';
 
-const GENERATED_FILES = /\.ngfactory\.ts$|\.css\.ts$|\.css\.shim\.ts$/;
-
 const PREAMBLE = `/**
  * This file is generated by the Angular 2 template compiler.
  * Do not edit.
@@ -69,7 +67,7 @@ export class CodeGenerator {
   }
 
   // Write codegen in a directory structure matching the sources.
-  private calculateEmitPath(filePath: string) {
+  private calculateEmitPath(filePath: string): string {
     let root = this.options.basePath;
     for (let eachRootDir of this.options.rootDirs || []) {
       if (this.options.trace) {

modules/@angular/compiler-cli/src/reflector_host.ts

@@ -16,8 +16,7 @@ import {StaticReflectorHost, StaticSymbol} from './static_reflector';
 
 const EXT = /(\.ts|\.d\.ts|\.js|\.jsx|\.tsx)$/;
 const DTS = /\.d\.ts$/;
-const NODE_MODULES = path.sep + 'node_modules' + path.sep;
+export const GENERATED_FILES = /\.ngfactory\.ts$|\.css\.ts$|\.css\.shim\.ts$/;
-const IS_GENERATED = /\.(ngfactory|css(\.shim)?)$/;
 
 export interface ReflectorHostContext {
   fileExists(fileName: string): boolean;
@@ -58,6 +57,9 @@ export class ReflectorHost implements StaticReflectorHost, ImportGenerator {
   private resolve(m: string, containingFile: string) {
     const resolved =
         ts.resolveModuleName(m, containingFile, this.options, this.context).resolvedModule;
+    if (this.options.traceResolution) {
+      console.log('resolve', m, containingFile, '=>', resolved);
+    }
     return resolved ? resolved.resolvedFileName : null;
   };
 
@@ -79,8 +81,8 @@ export class ReflectorHost implements StaticReflectorHost, ImportGenerator {
    * These need to be in a form that system.js can load, so absolute file paths don't work.
    *
    * The `containingFile` is always in the `genDir`, where as the `importedFile` can be in
-   * `genDir`, `node_module` or `basePath`.  The `importedFile` is either a generated file or
+   * `genDir`, `node_module` or `rootDir`/`rootDirs`.
-   * existing file.
+   * The `importedFile` is either a generated file or an existing file.
    *
    *               | genDir   | node_module |  rootDir
    * --------------+----------+-------------+----------
@@ -93,45 +95,59 @@ export class ReflectorHost implements StaticReflectorHost, ImportGenerator {
     importedFile = this.resolveAssetUrl(importedFile, containingFile);
     containingFile = this.resolveAssetUrl(containingFile, '');
 
+    if (this.options.traceResolution) {
+      console.log(
+          'getImportPath from containingFile', containingFile, 'to importedFile', importedFile);
+    }
+
     // If a file does not yet exist (because we compile it later), we still need to
     // assume it exists it so that the `resolve` method works!
     if (!this.compilerHost.fileExists(importedFile)) {
       this.context.assumeFileExists(importedFile);
     }
 
-    containingFile = this.rewriteGenDirPath(containingFile);
+    let importModuleName = importedFile.replace(EXT, '');
-    const containingDir = path.dirname(containingFile);
+    const parts = importModuleName.split(path.sep).filter(p => !!p);
-    // drop extension
+    let foundRelativeImport: string;
-    importedFile = importedFile.replace(EXT, '');
+    for (let index = parts.length - 1; index >= 0; index--) {
-
+      let candidate = parts.slice(index, parts.length).join(path.sep);
-    var nodeModulesIndex = importedFile.indexOf(NODE_MODULES);
+      if (this.resolve(candidate, containingFile) === importedFile) {
-    const importModule = nodeModulesIndex === -1 ?
+        return candidate;
-        null :
-        importedFile.substring(nodeModulesIndex + NODE_MODULES.length);
-    const isGeneratedFile = IS_GENERATED.test(importedFile);
-
-    if (isGeneratedFile) {
-      // rewrite to genDir path
-      if (importModule) {
-        // it is generated, therefore we do a relative path to the factory
-        return this.dotRelative(containingDir, this.genDir + NODE_MODULES + importModule);
-      } else {
-        // assume that import is also in `genDir`
-        importedFile = this.rewriteGenDirPath(importedFile);
-        return this.dotRelative(containingDir, importedFile);
       }
-    } else {
+      candidate = '.' + path.sep + candidate;
-      // user code import
+      if (this.resolve(candidate, containingFile) === importedFile) {
-      if (importModule) {
+        if (this.options.writeImportsForRootDirs) {
-        return importModule;
+          foundRelativeImport = candidate;
-      } else {
+        } else {
-        if (!this.isGenDirChildOfRootDir) {
+          foundRelativeImport = this.fixupGendirRelativePath(containingFile, importedFile);
-          // assume that they are on top of each other.
-          importedFile = importedFile.replace(this.basePath, this.genDir);
         }
-        return this.dotRelative(containingDir, importedFile);
       }
     }
+
+    if (foundRelativeImport) return foundRelativeImport;
+
+    // Try a relative import
+    let candidate = path.relative(path.dirname(containingFile), importModuleName);
+    if (this.resolve(candidate, containingFile) === importedFile) {
+      return this.fixupGendirRelativePath(containingFile, importedFile);
+    }
+
+    throw new Error(
+        `Unable to find any resolvable import for ${importedFile} relative to ${containingFile}`);
+  }
+
+  private fixupGendirRelativePath(containingFile: string, importedFile: string) {
+    let importModuleName = importedFile.replace(EXT, '');
+
+    if (!this.options.writeImportsForRootDirs && this.isGenDirChildOfRootDir) {
+      if (GENERATED_FILES.test(importedFile)) {
+        importModuleName = importModuleName.replace(this.basePath, this.genDir);
+      }
+      if (GENERATED_FILES.test(containingFile)) {
+        containingFile = containingFile.replace(this.basePath, this.genDir);
+      }
+    }
+    return this.dotRelative(path.dirname(containingFile), importModuleName);
   }
 
   private dotRelative(from: string, to: string): string {
@@ -139,21 +155,6 @@ export class ReflectorHost implements StaticReflectorHost, ImportGenerator {
     return rPath.startsWith('.') ? rPath : './' + rPath;
   }
 
-  /**
-   * Moves the path into `genDir` folder while preserving the `node_modules` directory.
-   */
-  private rewriteGenDirPath(filepath: string) {
-    var nodeModulesIndex = filepath.indexOf(NODE_MODULES);
-    if (nodeModulesIndex !== -1) {
-      // If we are in node_modulse, transplant them into `genDir`.
-      return path.join(this.genDir, filepath.substring(nodeModulesIndex));
-    } else {
-      // pretend that containing file is on top of the `genDir` to normalize the paths.
-      // we apply the `genDir` => `rootDir` delta through `rootDirPrefix` later.
-      return filepath.replace(this.basePath, this.genDir);
-    }
-  }
-
   findDeclaration(
       module: string, symbolName: string, containingFile: string,
       containingModule?: string): StaticSymbol {

modules/@angular/compiler-cli/test/mocks.ts

@@ -17,6 +17,8 @@ export interface Directory { [name: string]: Entry; }
 export class MockContext implements ReflectorHostContext {
   constructor(public currentDirectory: string, private files: Entry) {}
 
+  trace(s: string) { console.log(s); }
+
   fileExists(fileName: string): boolean { return typeof this.getEntry(fileName) === 'string'; }
 
   directoryExists(path: string): boolean { return typeof this.getEntry(path) === 'object'; }

modules/@angular/compiler-cli/test/reflector_host_spec.ts

@@ -20,6 +20,8 @@ describe('reflector_host', () => {
   var reflectorNestedGenDir: ReflectorHost;
   var reflectorSiblingGenDir: ReflectorHost;
 
+  const DEBUG = false;
+
   beforeEach(() => {
     context = new MockContext('/tmp/src', clone(FILES));
     host = new MockCompilerHost(context);
@@ -35,8 +37,9 @@ describe('reflector_host', () => {
     }
     reflectorNestedGenDir = new ReflectorHost(
         program, host, {
-          genDir: '/tmp/project/src/gen/',
+          // Intentional trailing slash, check for regression of #10533
-          basePath: '/tmp/project/src',
+          genDir: '/tmp/src/gen/',
+          basePath: '/tmp/src',
           skipMetadataEmit: false,
           skipTemplateCodegen: false,
           trace: false
@@ -44,77 +47,97 @@ describe('reflector_host', () => {
         context);
     reflectorSiblingGenDir = new ReflectorHost(
         program, host, {
-          genDir: '/tmp/project/gen',
+          genDir: '/tmp/gen',
-          basePath: '/tmp/project/src/',
+          // Intentional trailing slash, check for regression of #10533
+          basePath: '/tmp/src/',
           skipMetadataEmit: false,
           skipTemplateCodegen: false,
           trace: false
         },
         context);
+
   });
 
-  describe('nestedGenDir', () => {
+  describe('path mapping', () => {
+    it('should use rootDirs for calculating relative imports', () => {
+      const reflectorHost = new ReflectorHost(
+          program, host, {
+            genDir: '/tmp/gen',
+            basePath: '/tmp/src/',
+            skipMetadataEmit: false,
+            skipTemplateCodegen: false,
+            trace: false,
+            traceResolution: DEBUG,
+            rootDirs: ['/tmp/src/', '/tmp/genfiles/'],
+            writeImportsForRootDirs: true,
+          },
+          context);
+      expect(reflectorHost.getImportPath(
+                 '/tmp/src/pathmapping/bootstrap.ts', '/tmp/genfiles/pathmapping/comp.d.ts'))
+          .toEqual('./comp');
+    });
+  });
+
+  describe('nested genDir', () => {
     it('should import node_module from factory', () => {
       expect(reflectorNestedGenDir.getImportPath(
-                 '/tmp/project/src/gen/my.ngfactory.ts',
+                 '/tmp/src/gen/my.ngfactory.ts', '/tmp/src/node_modules/@angular/core.d.ts'))
-                 '/tmp/project/node_modules/@angular/core.d.ts'))
           .toEqual('@angular/core');
     });
 
     it('should import factory from factory', () => {
       expect(reflectorNestedGenDir.getImportPath(
-                 '/tmp/project/src/my.ngfactory.ts', '/tmp/project/src/my.other.ngfactory.ts'))
+                 '/tmp/src/my.ngfactory.ts', '/tmp/src/my.other.ngfactory.ts'))
           .toEqual('./my.other.ngfactory');
       expect(reflectorNestedGenDir.getImportPath(
-                 '/tmp/project/src/a/my.ngfactory.ts', '/tmp/project/src/my.other.css.ts'))
+                 '/tmp/src/a/my.ngfactory.ts', '/tmp/src/my.other.css.ts'))
           .toEqual('../my.other.css');
       expect(reflectorNestedGenDir.getImportPath(
-                 '/tmp/project/src/my.ngfactory.ts', '/tmp/project/src/a/my.other.css.shim.ts'))
+                 '/tmp/src/my.ngfactory.ts', '/tmp/src/a/my.other.css.shim.ts'))
           .toEqual('./a/my.other.css.shim');
     });
 
     it('should import application from factory', () => {
-      expect(reflectorNestedGenDir.getImportPath(
+      expect(
-                 '/tmp/project/src/my.ngfactory.ts', '/tmp/project/src/my.other.ts'))
+          reflectorNestedGenDir.getImportPath('/tmp/src/my.ngfactory.ts', '/tmp/src/my.other.ts'))
           .toEqual('../my.other');
-      expect(reflectorNestedGenDir.getImportPath(
+      expect(
-                 '/tmp/project/src/a/my.ngfactory.ts', '/tmp/project/src/my.other.ts'))
+          reflectorNestedGenDir.getImportPath('/tmp/src/a/my.ngfactory.ts', '/tmp/src/my.other.ts'))
           .toEqual('../../my.other');
-      expect(reflectorNestedGenDir.getImportPath(
+      expect(
-                 '/tmp/project/src/my.ngfactory.ts', '/tmp/project/src/a/my.other.ts'))
+          reflectorNestedGenDir.getImportPath('/tmp/src/my.ngfactory.ts', '/tmp/src/a/my.other.ts'))
           .toEqual('../a/my.other');
     });
   });
 
-  describe('nestedGenDir', () => {
+  describe('sibling genDir', () => {
     it('should import node_module from factory', () => {
       expect(reflectorSiblingGenDir.getImportPath(
-                 '/tmp/project/src/gen/my.ngfactory.ts',
+                 '/tmp/src/gen/my.ngfactory.ts', '/tmp/src/node_modules/@angular/core.d.ts'))
-                 '/tmp/project/node_modules/@angular/core.d.ts'))
           .toEqual('@angular/core');
     });
 
     it('should import factory from factory', () => {
       expect(reflectorSiblingGenDir.getImportPath(
-                 '/tmp/project/src/my.ngfactory.ts', '/tmp/project/src/my.other.ngfactory.ts'))
+                 '/tmp/src/my.ngfactory.ts', '/tmp/src/my.other.ngfactory.ts'))
           .toEqual('./my.other.ngfactory');
       expect(reflectorSiblingGenDir.getImportPath(
-                 '/tmp/project/src/a/my.ngfactory.ts', '/tmp/project/src/my.other.css.ts'))
+                 '/tmp/src/a/my.ngfactory.ts', '/tmp/src/my.other.css.ts'))
           .toEqual('../my.other.css');
       expect(reflectorSiblingGenDir.getImportPath(
-                 '/tmp/project/src/my.ngfactory.ts', '/tmp/project/src/a/my.other.css.shim.ts'))
+                 '/tmp/src/my.ngfactory.ts', '/tmp/src/a/my.other.css.shim.ts'))
           .toEqual('./a/my.other.css.shim');
     });
 
     it('should import application from factory', () => {
-      expect(reflectorSiblingGenDir.getImportPath(
+      expect(
-                 '/tmp/project/src/my.ngfactory.ts', '/tmp/project/src/my.other.ts'))
+          reflectorSiblingGenDir.getImportPath('/tmp/src/my.ngfactory.ts', '/tmp/src/my.other.ts'))
           .toEqual('./my.other');
       expect(reflectorSiblingGenDir.getImportPath(
-                 '/tmp/project/src/a/my.ngfactory.ts', '/tmp/project/src/my.other.ts'))
+                 '/tmp/src/a/my.ngfactory.ts', '/tmp/src/my.other.ts'))
           .toEqual('../my.other');
       expect(reflectorSiblingGenDir.getImportPath(
-                 '/tmp/project/src/my.ngfactory.ts', '/tmp/project/src/a/my.other.ts'))
+                 '/tmp/src/my.ngfactory.ts', '/tmp/src/a/my.other.ts'))
           .toEqual('./a/my.other');
     });
   });
@@ -131,7 +154,7 @@ describe('reflector_host', () => {
 
   it('should be able to produce an import from main @angular/core', () => {
     expect(reflectorNestedGenDir.getImportPath(
-               '/tmp/project/src/main.ts', '/tmp/project/node_modules/@angular/core.d.ts'))
+               '/tmp/src/main.ts', '/tmp/src/node_modules/@angular/core.d.ts'))
         .toEqual('@angular/core');
   });
 
@@ -295,6 +318,11 @@ const FILES: Entry = {
           })
         }
       },
+      'pathmapping': {'bootstrap.ts': `import {a} from './comp.d.ts';`},
+      'a': {
+        'my.other.css.shim.ts': dummyModule,
+      },
+      'my.other.ts': dummyModule,
       'node_modules': {
         '@angular': {
           'core.d.ts': dummyModule,
@@ -304,6 +332,13 @@ const FILES: Entry = {
           'unused.d.ts': dummyModule
         }
       }
+    },
+    'genfiles': {
+      'pathmapping': {
+        'comp.d.ts': `
+          export declare let a: string;
+        `
+      }
     }
   }
 };

tools/@angular/tsc-wrapped/src/options.ts

@@ -18,6 +18,18 @@ interface Options extends ts.CompilerOptions {
 
   // Whether to embed debug information in the compiled templates
   debug?: boolean;
+
+  // Starting with TypeScript 1.9, the 'rootDirs' option can be used
+  // to allow multiple source directories to have relative imports
+  // between them.
+  // This option causes generated code to use imports relative to the
+  // current directory, and requires you configure the 'rootDirs' to
+  // include both the genDir and rootDir.
+  // However, due to https://github.com/Microsoft/TypeScript/issues/8245
+  // note that using this option does not lay out into a flat directory
+  // with application and generated sources side-by-side, so you must
+  // teach your module loader how to resolve such imports as well.
+  writeImportsForRootDirs?: boolean;
 }
 
 export default Options;