Skip to content

Commit

Permalink
fix(compiler): compile .ngfactory.ts files even if nobody reference…
Browse files Browse the repository at this point in the history
…s them.

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

Fixes angular#16741
  • Loading branch information
tbosch committed May 23, 2017
1 parent 3b28c75 commit 81f5be3
Show file tree
Hide file tree
Showing 9 changed files with 164 additions and 58 deletions.
2 changes: 1 addition & 1 deletion build.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
5 changes: 3 additions & 2 deletions packages/compiler-cli/src/codegen.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,17 +36,18 @@ export class CodeGenerator {
public host: ts.CompilerHost, private compiler: compiler.AotCompiler,
private ngCompilerHost: CompilerHost) {}

codegen(): Promise<any> {
codegen(): Promise<string[]> {
return this.compiler
.compileAll(this.program.getSourceFiles().map(
sf => this.ngCompilerHost.getCanonicalFileName(sf.fileName)))
.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 = GENERATED_META_FILES.test(emitPath) ? generatedModule.source :
generatedModule.source;
this.host.writeFile(emitPath, source, false, () => {}, [sourceFile]);
return emitPath;
});
});
}
Expand Down
12 changes: 10 additions & 2 deletions packages/compiler-cli/src/compiler_host.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}
Expand Down
2 changes: 1 addition & 1 deletion packages/compiler-cli/src/extract_i18n.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
3 changes: 2 additions & 1 deletion packages/compiler-cli/src/extractor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand All @@ -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];
});
}

Expand Down
4 changes: 2 additions & 2 deletions packages/compiler-cli/src/ngtools_api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand Down Expand Up @@ -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);

Expand Down
103 changes: 95 additions & 8 deletions packages/compiler-cli/test/main_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,39 +13,53 @@ 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'));
});

// Restore reflector since AoT compiler will update it with a new static reflector
afterEach(() => { ɵreflector.updateCapabilities(new ɵReflectionCapabilities()); });

it('should compile without errors', (done) => {
writeConfig();
write('test.ts', 'export const A = 1;');

const mockConsole = {error: (s: string) => {}};
Expand All @@ -62,6 +76,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');
Expand All @@ -79,6 +97,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) => {}};
Expand All @@ -98,6 +117,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) => {}};
Expand All @@ -118,6 +138,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';`);

Expand All @@ -139,6 +160,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';
Expand Down Expand Up @@ -179,4 +201,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));
});
});
});
48 changes: 32 additions & 16 deletions tools/@angular/tsc-wrapped/src/main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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();

Expand Down Expand Up @@ -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 = {
Expand All @@ -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,
Expand All @@ -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);
Expand Down
Loading

0 comments on commit 81f5be3

Please sign in to comment.