Merge pull request #57 from angular/master

fix(compiler): compile `.ngfactory.ts` files even if nobody reference…

build.sh

@@ -266,7 +266,7 @@ compilePackage() {
     $NGC -p ${1}/tsconfig-build.json
     echo "======           Create ${1}/../${package_name}.d.ts re-export file for Closure"
     echo "$(cat ${LICENSE_BANNER}) ${N} export * from './${package_name}/index'" > ${2}/../${package_name}.d.ts
-    echo "{\"__symbolic\":\"module\",\"version\":3,\"metadata\":{},\"exports\":[{\"from\":\"./${package_name}/index\"}]}" > ${2}/../${package_name}.metadata.json
+    echo "{\"__symbolic\":\"module\",\"version\":3,\"metadata\":{},\"exports\":[{\"from\":\"./${package_name}/index\"}],\"bundleRedirect\":true}" > ${2}/../${package_name}.metadata.json
   fi
 
   for DIR in ${1}/* ; do

integration/hello_world__closure/tsconfig.json

@@ -13,7 +13,8 @@
     "noImplicitAny": false,
     "sourceMap": false,
     "experimentalDecorators": true,
-    "outDir": "built/src",
+    "outDir": "built",
+    "rootDir": ".",
     "declaration": true,
     "types": []
   },

packages/compiler-cli/src/codegen.ts

@@ -36,18 +36,19 @@ export class CodeGenerator {
       public host: ts.CompilerHost, private compiler: compiler.AotCompiler,
       private ngCompilerHost: CompilerHost) {}
 
-  codegen(): Promise<any> {
+  codegen(): Promise<string[]> {
     return this.compiler
         .analyzeModulesAsync(this.program.getSourceFiles().map(
             sf => this.ngCompilerHost.getCanonicalFileName(sf.fileName)))
         .then(analyzedModules => this.compiler.emitAllImpls(analyzedModules))
         .then(generatedModules => {
-          generatedModules.forEach(generatedModule => {
+          return generatedModules.map(generatedModule => {
             const sourceFile = this.program.getSourceFile(generatedModule.srcFileUrl);
             const emitPath = this.ngCompilerHost.calculateEmitPath(generatedModule.genFileUrl);
             const source =
                 generatedModule.source || compiler.toTypeScript(generatedModule, PREAMBLE);
             this.host.writeFile(emitPath, source, false, () => {}, [sourceFile]);
+            return emitPath;
           });
         });
   }

packages/compiler-cli/src/compiler_host.ts

@@ -33,6 +33,7 @@ export class CompilerHost implements AotCompilerHost {
   private resolverCache = new Map<string, ModuleMetadata[]>();
   private bundleIndexCache = new Map<string, boolean>();
   private bundleIndexNames = new Set<string>();
+  private bundleRedirectNames = new Set<string>();
   private moduleFileNames = new Map<string, string|null>();
   protected resolveModuleNameHost: CompilerHostContext;
 
@@ -280,7 +281,8 @@ export class CompilerHost implements AotCompilerHost {
       // Check for a bundle index.
       if (this.hasBundleIndex(filePath)) {
         const normalFilePath = path.normalize(filePath);
-        return this.bundleIndexNames.has(normalFilePath);
+        return this.bundleIndexNames.has(normalFilePath) ||
+            this.bundleRedirectNames.has(normalFilePath);
       }
     }
     return true;
@@ -331,7 +333,13 @@ export class CompilerHost implements AotCompilerHost {
                   const metadataFile = typings.replace(DTS, '.metadata.json');
                   if (this.context.fileExists(metadataFile)) {
                     const metadata = JSON.parse(this.context.readFile(metadataFile));
-                    if (metadata.importAs) {
+                    if (metadata.bundleRedirect) {
+                      this.bundleRedirectNames.add(typings);
+                      // Note: don't set result = true,
+                      // as this would mark this folder
+                      // as having a bundleIndex too early without
+                      // filling the bundleIndexNames.
+                    } else if (metadata.importAs) {
                       this.bundleIndexNames.add(typings);
                       result = true;
                     }

packages/compiler-cli/src/extract_i18n.ts

@@ -21,7 +21,7 @@ import {Extractor} from './extractor';
 
 function extract(
     ngOptions: tsc.AngularCompilerOptions, cliOptions: tsc.I18nExtractionCliOptions,
-    program: ts.Program, host: ts.CompilerHost): Promise<void> {
+    program: ts.Program, host: ts.CompilerHost) {
   return Extractor.create(ngOptions, program, host, cliOptions.locale)
       .extract(cliOptions.i18nFormat !, cliOptions.outFile);
 }

packages/compiler-cli/src/extractor.ts

@@ -27,7 +27,7 @@ export class Extractor {
       public host: ts.CompilerHost, private ngCompilerHost: CompilerHost,
       private program: ts.Program) {}
 
-  extract(formatName: string, outFile: string|null): Promise<void> {
+  extract(formatName: string, outFile: string|null): Promise<string[]> {
     // Checks the format and returns the extension
     const ext = this.getExtension(formatName);
 
@@ -38,6 +38,7 @@ export class Extractor {
       const dstFile = outFile || `messages.${ext}`;
       const dstPath = path.join(this.options.genDir, dstFile);
       this.host.writeFile(dstPath, content, false);
+      return [dstPath];
     });
   }
 

packages/compiler-cli/src/ngtools_api.ts

@@ -89,7 +89,7 @@ export class NgTools_InternalApi_NG_2 {
    * @internal
    * @private
    */
-  static codeGen(options: NgTools_InternalApi_NG2_CodeGen_Options): Promise<void> {
+  static codeGen(options: NgTools_InternalApi_NG2_CodeGen_Options): Promise<any> {
     const hostContext: CompilerHostContext =
         new CustomLoaderModuleResolutionHostAdapter(options.readResource, options.host);
     const cliOptions: NgcCliOptions = {
@@ -141,7 +141,7 @@ export class NgTools_InternalApi_NG_2 {
    * @internal
    * @private
    */
-  static extractI18n(options: NgTools_InternalApi_NG2_ExtractI18n_Options): Promise<void> {
+  static extractI18n(options: NgTools_InternalApi_NG2_ExtractI18n_Options): Promise<any> {
     const hostContext: CompilerHostContext =
         new CustomLoaderModuleResolutionHostAdapter(options.readResource, options.host);
 

packages/compiler-cli/test/main_spec.ts

@@ -12,36 +12,50 @@ import * as path from 'path';
 
 import {main} from '../src/main';
 
+function getNgRootDir() {
+  const moduleFilename = module.filename.replace(/\\/g, '/');
+  const distIndex = moduleFilename.indexOf('/dist/all');
+  return moduleFilename.substr(0, distIndex);
+}
 
 describe('compiler-cli', () => {
   let basePath: string;
+  let outDir: string;
   let write: (fileName: string, content: string) => void;
 
+  function writeConfig(tsconfig: string = '{"extends": "./tsconfig-base.json"}') {
+    write('tsconfig.json', tsconfig);
+  }
+
   beforeEach(() => {
     basePath = makeTempDir();
     write = (fileName: string, content: string) => {
       fs.writeFileSync(path.join(basePath, fileName), content, {encoding: 'utf-8'});
     };
-    write('tsconfig.json', `{
+    write('tsconfig-base.json', `{
       "compilerOptions": {
         "experimentalDecorators": true,
         "types": [],
         "outDir": "built",
         "declaration": true,
         "module": "es2015",
-        "moduleResolution": "node"
-      },
-      "angularCompilerOptions": {
-        "annotateForClosureCompiler": true
-      },
-      "files": ["test.ts"]
+        "moduleResolution": "node",
+        "lib": ["es6", "dom"]
+      }
     }`);
+    outDir = path.resolve(basePath, 'built');
+    const ngRootDir = getNgRootDir();
     const nodeModulesPath = path.resolve(basePath, 'node_modules');
     fs.mkdirSync(nodeModulesPath);
-    fs.symlinkSync(path.resolve(__dirname, '..', '..'), path.resolve(nodeModulesPath, '@angular'));
+    fs.symlinkSync(
+        path.resolve(ngRootDir, 'dist', 'all', '@angular'),
+        path.resolve(nodeModulesPath, '@angular'));
+    fs.symlinkSync(
+        path.resolve(ngRootDir, 'node_modules', 'rxjs'), path.resolve(nodeModulesPath, 'rxjs'));
   });
 
   it('should compile without errors', (done) => {
+    writeConfig();
     write('test.ts', 'export const A = 1;');
 
     const mockConsole = {error: (s: string) => {}};
@@ -58,6 +72,10 @@ describe('compiler-cli', () => {
   });
 
   it('should not print the stack trace if user input file does not exist', (done) => {
+    writeConfig(`{
+      "extends": "./tsconfig-base.json",
+      "files": ["test.ts"]
+    }`);
     const mockConsole = {error: (s: string) => {}};
 
     spyOn(mockConsole, 'error');
@@ -75,6 +93,7 @@ describe('compiler-cli', () => {
   });
 
   it('should not print the stack trace if user input file is malformed', (done) => {
+    writeConfig();
     write('test.ts', 'foo;');
 
     const mockConsole = {error: (s: string) => {}};
@@ -94,6 +113,7 @@ describe('compiler-cli', () => {
   });
 
   it('should not print the stack trace if cannot find the imported module', (done) => {
+    writeConfig();
     write('test.ts', `import {MyClass} from './not-exist-deps';`);
 
     const mockConsole = {error: (s: string) => {}};
@@ -114,6 +134,7 @@ describe('compiler-cli', () => {
   });
 
   it('should not print the stack trace if cannot import', (done) => {
+    writeConfig();
     write('empty-deps.ts', 'export const A = 1;');
     write('test.ts', `import {MyClass} from './empty-deps';`);
 
@@ -135,6 +156,7 @@ describe('compiler-cli', () => {
   });
 
   it('should not print the stack trace if type mismatches', (done) => {
+    writeConfig();
     write('empty-deps.ts', 'export const A = "abc";');
     write('test.ts', `
       import {A} from './empty-deps';
@@ -175,4 +197,69 @@ describe('compiler-cli', () => {
         })
         .catch(e => done.fail(e));
   });
+
+  describe('compile ngfactory files', () => {
+    it('should report errors for ngfactory files that are not referenced by root files', (done) => {
+      writeConfig(`{
+          "extends": "./tsconfig-base.json",
+          "files": ["mymodule.ts"]
+        }`);
+      write('mymodule.ts', `
+        import {NgModule, Component} from '@angular/core';
+
+        @Component({template: '{{unknownProp}}'})
+        export class MyComp {}
+
+        @NgModule({declarations: [MyComp]})
+        export class MyModule {}
+      `);
+
+      const mockConsole = {error: (s: string) => {}};
+
+      const errorSpy = spyOn(mockConsole, 'error');
+
+      main({p: basePath}, mockConsole.error)
+          .then((exitCode) => {
+            expect(errorSpy).toHaveBeenCalledTimes(1);
+            expect(errorSpy.calls.mostRecent().args[0])
+                .toContain('Error at ' + path.join(basePath, 'mymodule.ngfactory.ts'));
+            expect(errorSpy.calls.mostRecent().args[0])
+                .toContain(`Property 'unknownProp' does not exist on type 'MyComp'`);
+
+            expect(exitCode).toEqual(1);
+            done();
+          })
+          .catch(e => done.fail(e));
+    });
+
+    it('should compile ngfactory files that are not referenced by root files', (done) => {
+      writeConfig(`{
+          "extends": "./tsconfig-base.json",
+          "files": ["mymodule.ts"]
+        }`);
+      write('mymodule.ts', `
+        import {CommonModule} from '@angular/common';
+        import {NgModule} from '@angular/core';
+
+        @NgModule({
+          imports: [CommonModule]
+        })
+        export class MyModule {}
+      `);
+
+      main({p: basePath})
+          .then((exitCode) => {
+            expect(exitCode).toEqual(0);
+
+            expect(fs.existsSync(path.resolve(outDir, 'mymodule.ngfactory.js'))).toBe(true);
+            expect(fs.existsSync(path.resolve(
+                       outDir, 'node_modules', '@angular', 'core', 'src',
+                       'application_module.ngfactory.js')))
+                .toBe(true);
+
+            done();
+          })
+          .catch(e => done.fail(e));
+    });
+  });
 });

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

@@ -11,22 +11,27 @@ import * as path from 'path';
 import * as tsickle from 'tsickle';
 import * as ts from 'typescript';
 
-import {check, tsc} from './tsc';
-
-import NgOptions from './options';
-import {MetadataWriterHost, SyntheticIndexHost} from './compiler_host';
+import {CompilerHostAdapter, MetadataBundler} from './bundler';
 import {CliOptions} from './cli_options';
-import {VinylFile, isVinylFile} from './vinyl_file';
-import {MetadataBundler, CompilerHostAdapter} from './bundler';
+import {MetadataWriterHost, SyntheticIndexHost} from './compiler_host';
 import {privateEntriesToIndex} from './index_writer';
+import NgOptions from './options';
+import {check, tsc} from './tsc';
+import {isVinylFile, VinylFile} from './vinyl_file';
+
 export {UserError} from './tsc';
 
 const DTS = /\.d\.ts$/;
 const JS_EXT = /(\.js|)$/;
-
-export type CodegenExtension =
-    (ngOptions: NgOptions, cliOptions: CliOptions, program: ts.Program, host: ts.CompilerHost) =>
-        Promise<void>;
+const TS_EXT = /\.ts$/;
+
+export interface CodegenExtension {
+  /**
+   * Returns the generated file names.
+   */
+  (ngOptions: NgOptions, cliOptions: CliOptions, program: ts.Program,
+   host: ts.CompilerHost): Promise<string[]>;
+}
 
 export function main(
     project: string | VinylFile, cliOptions: CliOptions, codegen?: CodegenExtension,
@@ -46,10 +51,18 @@ export function main(
     const basePath = path.resolve(process.cwd(), cliOptions.basePath || projectDir);
 
     // read the configuration options from wherever you store them
-    const {parsed, ngOptions} = tsc.readConfiguration(project, basePath, options);
+    let {parsed, ngOptions} = tsc.readConfiguration(project, basePath, options);
     ngOptions.basePath = basePath;
-    const createProgram = (host: ts.CompilerHost, oldProgram?: ts.Program) =>
-        ts.createProgram(parsed.fileNames, parsed.options, host, oldProgram);
+    let rootFileNames: string[] = parsed.fileNames.slice(0);
+    const createProgram = (host: ts.CompilerHost, oldProgram?: ts.Program) => {
+      return ts.createProgram(rootFileNames.slice(0), parsed.options, host, oldProgram);
+    };
+    const addGeneratedFileName = (genFileName: string) => {
+      if (genFileName.startsWith(basePath) && TS_EXT.exec(genFileName)) {
+        rootFileNames.push(genFileName);
+      }
+    };
+
     const diagnostics = (parsed.options as any).diagnostics;
     if (diagnostics) (ts as any).performance.enable();
 
@@ -83,7 +96,7 @@ export function main(
       const libraryIndex = `./${path.basename(indexModule)}`;
       const content = privateEntriesToIndex(libraryIndex, metadataBundle.privates);
       host = new SyntheticIndexHost(host, {name, content, metadata});
-      parsed.fileNames.push(name);
+      addGeneratedFileName(name);
     }
 
     const tsickleCompilerHostOptions: tsickle.Options = {
@@ -109,12 +122,14 @@ export function main(
     check(errors);
 
     if (ngOptions.skipTemplateCodegen || !codegen) {
-      codegen = () => Promise.resolve(null);
+      codegen = () => Promise.resolve([]);
     }
 
     if (diagnostics) console.time('NG codegen');
-    return codegen(ngOptions, cliOptions, program, host).then(() => {
+    return codegen(ngOptions, cliOptions, program, host).then((genFiles) => {
       if (diagnostics) console.timeEnd('NG codegen');
+      // Add the generated files to the configuration so they will become part of the program.
+      genFiles.forEach(genFileName => addGeneratedFileName(genFileName));
       let definitionsHost: ts.CompilerHost = tsickleCompilerHost;
       if (!ngOptions.skipMetadataEmit) {
         // if tsickle is not not used for emitting, but we do use the MetadataWriterHost,
@@ -123,6 +138,7 @@ export function main(
             ngOptions.annotationsAs === 'decorators' && !ngOptions.annotateForClosureCompiler;
         definitionsHost = new MetadataWriterHost(tsickleCompilerHost, ngOptions, emitJsFiles);
       }
+
       // Create a new program since codegen files were created after making the old program
       let programWithCodegen = createProgram(definitionsHost, program);
       tsc.typeCheck(host, programWithCodegen);