fix(compiler): correctly calculate the outDir if it repeats a parts of the rootDir. (#19836)

Fixes #19718

PR Close #19836

packages/compiler-cli/src/transformers/compiler_host.ts

@@ -15,7 +15,7 @@ import {METADATA_VERSION, ModuleMetadata} from '../metadata/index';
 
 import {CompilerHost, CompilerOptions, LibrarySummary} from './api';
 import {MetadataReaderHost, createMetadataReaderCache, readMetadata} from './metadata_reader';
-import {DTS, GENERATED_FILES} from './util';
+import {DTS, GENERATED_FILES, isInRootDir, relativeToRootDirs} from './util';
 
 const NODE_MODULES_PACKAGE_NAME = /node_modules\/((\w|-)+|(@(\w|-)+\/(\w|-)+))/;
 const EXT = /(\.ts|\.d\.ts|\.js|\.jsx|\.tsx)$/;
@@ -305,7 +305,7 @@ export class TsCompilerAotCompilerTypeCheckHostAdapter implements ts.CompilerHos
   shouldGenerateFile(fileName: string): {generate: boolean, baseFileName?: string} {
     // TODO(tbosch): allow generating files that are not in the rootDir
     // See https://github.com/angular/angular/issues/19337
-    if (this.options.rootDir && !pathStartsWithPrefix(this.options.rootDir, fileName)) {
+    if (!isInRootDir(fileName, this.options)) {
       return {generate: false};
     }
     const genMatch = GENERATED_FILES.exec(fileName);
@@ -339,7 +339,7 @@ export class TsCompilerAotCompilerTypeCheckHostAdapter implements ts.CompilerHos
     // TODO(tbosch): allow generating files that are not in the rootDir
     // See https://github.com/angular/angular/issues/19337
     return !GENERATED_FILES.test(fileName) && this.isSourceFile(fileName) &&
-        (!this.options.rootDir || pathStartsWithPrefix(this.options.rootDir, fileName));
+        isInRootDir(fileName, this.options);
   }
 
   getSourceFile(
@@ -573,22 +573,6 @@ function getPackageName(filePath: string): string|null {
   return match ? match[1] : null;
 }
 
-export function relativeToRootDirs(filePath: string, rootDirs: string[]): string {
-  if (!filePath) return filePath;
-  for (const dir of rootDirs || []) {
-    const rel = pathStartsWithPrefix(dir, filePath);
-    if (rel) {
-      return rel;
-    }
-  }
-  return filePath;
-}
-
-function pathStartsWithPrefix(prefix: string, fullPath: string): string|null {
-  const rel = path.relative(prefix, fullPath);
-  return rel.startsWith('..') ? null : rel;
-}
-
 function stripNodeModulesPrefix(filePath: string): string {
   return filePath.replace(/.*node_modules\//, '');
 }

packages/compiler-cli/src/transformers/program.ts

@@ -18,7 +18,8 @@ import {CompilerHost, CompilerOptions, CustomTransformers, DEFAULT_ERROR_CODE, D
 import {CodeGenerator, TsCompilerAotCompilerTypeCheckHostAdapter, getOriginalReferences} from './compiler_host';
 import {LowerMetadataCache, getExpressionLoweringTransformFactory} from './lower_expressions';
 import {getAngularEmitterTransformFactory} from './node_emitter_transform';
-import {GENERATED_FILES, StructureIsReused, createMessageDiagnostic, tsStructureIsReused} from './util';
+import {GENERATED_FILES, StructureIsReused, createMessageDiagnostic, isInRootDir, tsStructureIsReused} from './util';
+
 
 
 /**
@@ -538,7 +539,10 @@ class AngularCompilerProgram implements Program {
       if (!(emitFlags & EmitFlags.Codegen)) {
         return {genFiles: [], genDiags: []};
       }
-      let genFiles = this.compiler.emitAllImpls(this.analyzedModules);
+      // TODO(tbosch): allow generating files that are not in the rootDir
+      // See https://github.com/angular/angular/issues/19337
+      let genFiles = this.compiler.emitAllImpls(this.analyzedModules)
+                         .filter(genFile => isInRootDir(genFile.genFileUrl, this.options));
       if (this.oldProgramEmittedGeneratedFiles) {
         const oldProgramEmittedGeneratedFiles = this.oldProgramEmittedGeneratedFiles;
         genFiles = genFiles.filter(genFile => {
@@ -727,9 +731,10 @@ export function createSrcToOutPathMapper(
     if (srcFileDir === outFileDir) {
       return (srcFileName) => srcFileName;
     }
+    // calculate the common suffix, stopping
+    // at `outDir`.
     const srcDirParts = srcFileDir.split('/');
-    const outDirParts = outFileDir.split('/');
+    const outDirParts = path.relative(outDir, outFileDir).split('/');
-    // calculate the common suffix
     let i = 0;
     while (i < Math.min(srcDirParts.length, outDirParts.length) &&
            srcDirParts[srcDirParts.length - 1 - i] === outDirParts[outDirParts.length - 1 - i])

packages/compiler-cli/src/transformers/util.ts

@@ -6,9 +6,10 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
+import * as path from 'path';
 import * as ts from 'typescript';
 
-import {DEFAULT_ERROR_CODE, Diagnostic, SOURCE} from './api';
+import {CompilerOptions, DEFAULT_ERROR_CODE, Diagnostic, SOURCE} from './api';
 
 export const GENERATED_FILES = /(.*?)\.(ngfactory|shim\.ngstyle|ngstyle|ngsummary)\.(js|d\.ts|ts)$/;
 export const DTS = /\.d\.ts$/;
@@ -30,3 +31,23 @@ export function createMessageDiagnostic(messageText: string): ts.Diagnostic&Diag
     source: SOURCE,
   };
 }
+
+export function isInRootDir(fileName: string, options: CompilerOptions) {
+  return !options.rootDir || pathStartsWithPrefix(options.rootDir, fileName);
+}
+
+export function relativeToRootDirs(filePath: string, rootDirs: string[]): string {
+  if (!filePath) return filePath;
+  for (const dir of rootDirs || []) {
+    const rel = pathStartsWithPrefix(dir, filePath);
+    if (rel) {
+      return rel;
+    }
+  }
+  return filePath;
+}
+
+function pathStartsWithPrefix(prefix: string, fullPath: string): string|null {
+  const rel = path.relative(prefix, fullPath);
+  return rel.startsWith('..') ? null : rel;
+}

packages/compiler-cli/test/transformers/program_spec.ts

@@ -454,25 +454,33 @@ describe('ng program', () => {
   });
 
   it('should not emit generated files whose sources are outside of the rootDir', () => {
-    compileLib('lib');
     testSupport.writeFiles({
       'src/main.ts': createModuleAndCompSource('main'),
       'src/index.ts': `
           export * from './main';
-          export * from 'lib/index';
         `
     });
-    compile(undefined, {rootDir: path.resolve(testSupport.basePath, 'src')});
+    const options =
+        testSupport.createCompilerOptions({rootDir: path.resolve(testSupport.basePath, 'src')});
+    const host = ng.createCompilerHost({options});
+    const writtenFileNames: string[] = [];
+    const oldWriteFile = host.writeFile;
+    host.writeFile = (fileName, data, writeByteOrderMark) => {
+      writtenFileNames.push(fileName);
+      oldWriteFile(fileName, data, writeByteOrderMark);
+    };
+
+    compile(/*oldProgram*/ undefined, options, /*rootNames*/ undefined, host);
+
+    // no emit for files from node_modules as they are outside of rootDir
+    expect(writtenFileNames.some(f => /node_modules/.test(f))).toBe(false);
+
+    // emit all gen files for files under src/
     testSupport.shouldExist('built/main.js');
     testSupport.shouldExist('built/main.d.ts');
     testSupport.shouldExist('built/main.ngfactory.js');
     testSupport.shouldExist('built/main.ngfactory.d.ts');
     testSupport.shouldExist('built/main.ngsummary.json');
-    testSupport.shouldNotExist('built/node_modules/lib/index.js');
-    testSupport.shouldNotExist('built/node_modules/lib/index.d.ts');
-    testSupport.shouldNotExist('built/node_modules/lib/index.ngfactory.js');
-    testSupport.shouldNotExist('built/node_modules/lib/index.ngfactory.d.ts');
-    testSupport.shouldNotExist('built/node_modules/lib/index.ngsummary.json');
   });
 
   describe('createSrcToOutPathMapper', () => {
@@ -492,10 +500,17 @@ describe('ng program', () => {
     });
 
     it('should adjust the filename if the outDir is outside of the rootDir', () => {
-      const mapper = createSrcToOutPathMapper('/out', '/tmp/a/x.ts', '/a/x.js');
+      const mapper = createSrcToOutPathMapper('/out', '/tmp/a/x.ts', '/out/a/x.js');
       expect(mapper('/tmp/b/y.js')).toBe('/out/b/y.js');
     });
 
+    it('should adjust the filename if the common prefix of sampleSrc and sampleOut is outside of outDir',
+       () => {
+         const mapper =
+             createSrcToOutPathMapper('/dist/common', '/src/common/x.ts', '/dist/common/x.js');
+         expect(mapper('/src/common/y.js')).toBe('/dist/common/y.js');
+       });
+
     it('should work on windows with normalized paths', () => {
       const mapper =
           createSrcToOutPathMapper('c:/tmp/out', 'c:/tmp/a/x.ts', 'c:/tmp/out/a/x.js', path.win32);

scripts/ci/test-e2e.sh

@@ -26,7 +26,6 @@ travisFoldEnd "test.e2e.check-cycle"
 
 # Serve files for e2e tests
 (
-  cd dist/
   $(npm bin)/gulp serve &
   $(npm bin)/gulp serve-examples &
 )