fix(compiler): compile `.ngfactory.ts` files even if nobody references them. (#16899)

This is especially important for library authors, as they will
not reference the .ngfactory.ts files.

Fixes #16741
This commit is contained in:
Tobias Bosch 2017-05-25 10:00:26 -07:00 committed by Chuck Jazdzewski
parent 966eb2fbd0
commit 573b8611bc
10 changed files with 165 additions and 58 deletions

View File

@ -266,7 +266,7 @@ compilePackage() {
$NGC -p ${1}/tsconfig-build.json $NGC -p ${1}/tsconfig-build.json
echo "====== Create ${1}/../${package_name}.d.ts re-export file for Closure" 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 "$(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 fi
for DIR in ${1}/* ; do for DIR in ${1}/* ; do

View File

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

View File

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

View File

@ -33,6 +33,7 @@ export class CompilerHost implements AotCompilerHost {
private resolverCache = new Map<string, ModuleMetadata[]>(); private resolverCache = new Map<string, ModuleMetadata[]>();
private bundleIndexCache = new Map<string, boolean>(); private bundleIndexCache = new Map<string, boolean>();
private bundleIndexNames = new Set<string>(); private bundleIndexNames = new Set<string>();
private bundleRedirectNames = new Set<string>();
private moduleFileNames = new Map<string, string|null>(); private moduleFileNames = new Map<string, string|null>();
protected resolveModuleNameHost: CompilerHostContext; protected resolveModuleNameHost: CompilerHostContext;
@ -280,7 +281,8 @@ export class CompilerHost implements AotCompilerHost {
// Check for a bundle index. // Check for a bundle index.
if (this.hasBundleIndex(filePath)) { if (this.hasBundleIndex(filePath)) {
const normalFilePath = path.normalize(filePath); const normalFilePath = path.normalize(filePath);
return this.bundleIndexNames.has(normalFilePath); return this.bundleIndexNames.has(normalFilePath) ||
this.bundleRedirectNames.has(normalFilePath);
} }
} }
return true; return true;
@ -331,7 +333,13 @@ export class CompilerHost implements AotCompilerHost {
const metadataFile = typings.replace(DTS, '.metadata.json'); const metadataFile = typings.replace(DTS, '.metadata.json');
if (this.context.fileExists(metadataFile)) { if (this.context.fileExists(metadataFile)) {
const metadata = JSON.parse(this.context.readFile(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); this.bundleIndexNames.add(typings);
result = true; result = true;
} }

View File

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

View File

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

View File

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

View File

@ -12,36 +12,50 @@ import * as path from 'path';
import {main} from '../src/main'; 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', () => { describe('compiler-cli', () => {
let basePath: string; let basePath: string;
let outDir: string;
let write: (fileName: string, content: string) => void; let write: (fileName: string, content: string) => void;
function writeConfig(tsconfig: string = '{"extends": "./tsconfig-base.json"}') {
write('tsconfig.json', tsconfig);
}
beforeEach(() => { beforeEach(() => {
basePath = makeTempDir(); basePath = makeTempDir();
write = (fileName: string, content: string) => { write = (fileName: string, content: string) => {
fs.writeFileSync(path.join(basePath, fileName), content, {encoding: 'utf-8'}); fs.writeFileSync(path.join(basePath, fileName), content, {encoding: 'utf-8'});
}; };
write('tsconfig.json', `{ write('tsconfig-base.json', `{
"compilerOptions": { "compilerOptions": {
"experimentalDecorators": true, "experimentalDecorators": true,
"types": [], "types": [],
"outDir": "built", "outDir": "built",
"declaration": true, "declaration": true,
"module": "es2015", "module": "es2015",
"moduleResolution": "node" "moduleResolution": "node",
}, "lib": ["es6", "dom"]
"angularCompilerOptions": { }
"annotateForClosureCompiler": true
},
"files": ["test.ts"]
}`); }`);
outDir = path.resolve(basePath, 'built');
const ngRootDir = getNgRootDir();
const nodeModulesPath = path.resolve(basePath, 'node_modules'); const nodeModulesPath = path.resolve(basePath, 'node_modules');
fs.mkdirSync(nodeModulesPath); 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) => { it('should compile without errors', (done) => {
writeConfig();
write('test.ts', 'export const A = 1;'); write('test.ts', 'export const A = 1;');
const mockConsole = {error: (s: string) => {}}; 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) => { 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) => {}}; const mockConsole = {error: (s: string) => {}};
spyOn(mockConsole, 'error'); spyOn(mockConsole, 'error');
@ -75,6 +93,7 @@ describe('compiler-cli', () => {
}); });
it('should not print the stack trace if user input file is malformed', (done) => { it('should not print the stack trace if user input file is malformed', (done) => {
writeConfig();
write('test.ts', 'foo;'); write('test.ts', 'foo;');
const mockConsole = {error: (s: string) => {}}; 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) => { it('should not print the stack trace if cannot find the imported module', (done) => {
writeConfig();
write('test.ts', `import {MyClass} from './not-exist-deps';`); write('test.ts', `import {MyClass} from './not-exist-deps';`);
const mockConsole = {error: (s: string) => {}}; const mockConsole = {error: (s: string) => {}};
@ -114,6 +134,7 @@ describe('compiler-cli', () => {
}); });
it('should not print the stack trace if cannot import', (done) => { it('should not print the stack trace if cannot import', (done) => {
writeConfig();
write('empty-deps.ts', 'export const A = 1;'); write('empty-deps.ts', 'export const A = 1;');
write('test.ts', `import {MyClass} from './empty-deps';`); 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) => { it('should not print the stack trace if type mismatches', (done) => {
writeConfig();
write('empty-deps.ts', 'export const A = "abc";'); write('empty-deps.ts', 'export const A = "abc";');
write('test.ts', ` write('test.ts', `
import {A} from './empty-deps'; import {A} from './empty-deps';
@ -175,4 +197,69 @@ describe('compiler-cli', () => {
}) })
.catch(e => done.fail(e)); .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));
});
});
}); });

View File

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

View File

@ -103,16 +103,19 @@ export function validateAngularCompilerOptions(options: AngularCompilerOptions):
} }
export class Tsc implements CompilerInterface { export class Tsc implements CompilerInterface {
public ngOptions: AngularCompilerOptions; private parseConfigHost: ts.ParseConfigHost;
public parsed: ts.ParsedCommandLine;
private basePath: string;
constructor(private readFile = ts.sys.readFile, private readDirectory = ts.sys.readDirectory) {} constructor(private readFile = ts.sys.readFile, private readDirectory = ts.sys.readDirectory) {
this.parseConfigHost = {
useCaseSensitiveFileNames: true,
fileExists: existsSync,
readDirectory: this.readDirectory,
readFile: ts.sys.readFile
};
}
readConfiguration( readConfiguration(
project: string|VinylFile, basePath: string, existingOptions?: ts.CompilerOptions) { project: string|VinylFile, basePath: string, existingOptions?: ts.CompilerOptions) {
this.basePath = basePath;
// Allow a directory containing tsconfig.json as the project value // Allow a directory containing tsconfig.json as the project value
// Note, TS@next returns an empty array, while earlier versions throw // Note, TS@next returns an empty array, while earlier versions throw
try { try {
@ -135,31 +138,21 @@ export class Tsc implements CompilerInterface {
})(); })();
check([error]); check([error]);
// Do not inline `host` into `parseJsonConfigFileContent` until after const parsed =
// g3 is updated to the latest TypeScript. ts.parseJsonConfigFileContent(config, this.parseConfigHost, basePath, existingOptions);
// The issue is that old typescript only has `readDirectory` while
// the newer TypeScript has additional `useCaseSensitiveFileNames` and
// `fileExists`. Inlining will trigger an error of extra parameters.
const host = {
useCaseSensitiveFileNames: true,
fileExists: existsSync,
readDirectory: this.readDirectory,
readFile: ts.sys.readFile
};
this.parsed = ts.parseJsonConfigFileContent(config, host, basePath, existingOptions);
check(this.parsed.errors); check(parsed.errors);
// Default codegen goes to the current directory // Default codegen goes to the current directory
// Parsed options are already converted to absolute paths // Parsed options are already converted to absolute paths
this.ngOptions = config.angularCompilerOptions || {}; const ngOptions = config.angularCompilerOptions || {};
this.ngOptions.genDir = path.join(basePath, this.ngOptions.genDir || '.'); ngOptions.genDir = path.join(basePath, ngOptions.genDir || '.');
for (const key of Object.keys(this.parsed.options)) { for (const key of Object.keys(parsed.options)) {
this.ngOptions[key] = this.parsed.options[key]; ngOptions[key] = parsed.options[key];
} }
check(validateAngularCompilerOptions(this.ngOptions)); check(validateAngularCompilerOptions(ngOptions));
return {parsed: this.parsed, ngOptions: this.ngOptions}; return {parsed, ngOptions};
} }
typeCheck(compilerHost: ts.CompilerHost, program: ts.Program): void { typeCheck(compilerHost: ts.CompilerHost, program: ts.Program): void {