honeymoose
/
angular-cn
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

fix(compiler): recover from structural errors in watch mode (#19953) 

This also changes the compiler so that we throw less often
on structural changes and produce a meaningful state
in the `ng.Program` in case of errors.

Related to #19951

PR Close #19953
This commit is contained in:
Tobias Bosch 2017-10-26 15:24:54 -07:00 committed by Matias Niemelä
parent 18e9d86a3b
commit 957be960d2
7 changed files with 157 additions and 44 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
packages/compiler-cli/src/perform_watch.ts
View File

@ -191,6 +191,10 @@ export function performWatchCompilation(host: PerformWatchHost):
};
}
ingoreFilesForWatch.clear();
const oldProgram = cachedProgram;
// We clear out the `cachedProgram` here as a
// program can only be used as `oldProgram` 1x
cachedProgram = undefined;
const compileResult = performCompilation({
rootNames: cachedOptions.rootNames,
options: cachedOptions.options,

69
packages/compiler-cli/src/transformers/program.ts
View File

@ -175,15 +175,17 @@ class AngularCompilerProgram implements Program {
if (this._analyzedModules) {
throw new Error('Angular structure already loaded');
}
const {tmpProgram, sourceFiles, rootNames} = this._createProgramWithBasicStubs();
return this.compiler.loadFilesAsync(sourceFiles)
.catch(this.catchAnalysisError.bind(this))
.then(analyzedModules => {
if (this._analyzedModules) {
throw new Error('Angular structure loaded both synchronously and asynchronsly');
}
this._updateProgramWithTypeCheckStubs(tmpProgram, analyzedModules, rootNames);
});
return Promise.resolve()
.then(() => {
const {tmpProgram, sourceFiles, rootNames} = this._createProgramWithBasicStubs();
return this.compiler.loadFilesAsync(sourceFiles).then(analyzedModules => {
if (this._analyzedModules) {
throw new Error('Angular structure loaded both synchronously and asynchronsly');
}
this._updateProgramWithTypeCheckStubs(tmpProgram, analyzedModules, rootNames);
});
})
.catch(e => this._createProgramOnError(e));
}
listLazyRoutes(route?: string): LazyRoute[] {
@ -402,14 +404,13 @@ class AngularCompilerProgram implements Program {
if (this._analyzedModules) {
return;
}
const {tmpProgram, sourceFiles, rootNames} = this._createProgramWithBasicStubs();
let analyzedModules: NgAnalyzedModules|null;
try {
analyzedModules = this.compiler.loadFilesSync(sourceFiles);
const {tmpProgram, sourceFiles, rootNames} = this._createProgramWithBasicStubs();
const analyzedModules = this.compiler.loadFilesSync(sourceFiles);
this._updateProgramWithTypeCheckStubs(tmpProgram, analyzedModules, rootNames);
} catch (e) {
analyzedModules = this.catchAnalysisError(e);
this._createProgramOnError(e);
}
this._updateProgramWithTypeCheckStubs(tmpProgram, analyzedModules, rootNames);
}
private _createCompiler() {
@ -457,7 +458,7 @@ class AngularCompilerProgram implements Program {
let rootNames = this.rootNames;
if (this.options.generateCodeForLibraries !== false) {
// if we should generateCodeForLibraries, enver include
// if we should generateCodeForLibraries, never include
// generated files in the program as otherwise we will
// ovewrite them and typescript will report the error
// TS5055: Cannot write file ... because it would overwrite input file.
@ -482,23 +483,21 @@ class AngularCompilerProgram implements Program {
}
private _updateProgramWithTypeCheckStubs(
tmpProgram: ts.Program, analyzedModules: NgAnalyzedModules|null, rootNames: string[]) {
this._analyzedModules = analyzedModules || emptyModules;
if (analyzedModules) {
tmpProgram.getSourceFiles().forEach(sf => {
if (sf.fileName.endsWith('.ngfactory.ts')) {
const {generate, baseFileName} = this.hostAdapter.shouldGenerateFile(sf.fileName);
if (generate) {
// Note: ! is ok as hostAdapter.shouldGenerateFile will always return a basefileName
// for .ngfactory.ts files.
const genFile = this.compiler.emitTypeCheckStub(sf.fileName, baseFileName !);
if (genFile) {
this.hostAdapter.updateGeneratedFile(genFile);
}
tmpProgram: ts.Program, analyzedModules: NgAnalyzedModules, rootNames: string[]) {
this._analyzedModules = analyzedModules;
tmpProgram.getSourceFiles().forEach(sf => {
if (sf.fileName.endsWith('.ngfactory.ts')) {
const {generate, baseFileName} = this.hostAdapter.shouldGenerateFile(sf.fileName);
if (generate) {
// Note: ! is ok as hostAdapter.shouldGenerateFile will always return a basefileName
// for .ngfactory.ts files.
const genFile = this.compiler.emitTypeCheckStub(sf.fileName, baseFileName !);
if (genFile) {
this.hostAdapter.updateGeneratedFile(genFile);
}
}
});
}
}
});
this._tsProgram = ts.createProgram(rootNames, this.options, this.hostAdapter, tmpProgram);
// Note: the new ts program should be completely reusable by TypeScript as:
// - we cache all the files in the hostAdapter
@ -509,7 +508,13 @@ class AngularCompilerProgram implements Program {
}
}
private catchAnalysisError(e: any): NgAnalyzedModules|null {
private _createProgramOnError(e: any) {
// Still fill the analyzedModules and the tsProgram
// so that we don't cause other errors for users who e.g. want to emit the ngProgram.
this._analyzedModules = emptyModules;
this.oldTsProgram = undefined;
this._hostAdapter.isSourceFile = () => false;
this._tsProgram = ts.createProgram(this.rootNames, this.options, this.hostAdapter);
if (isSyntaxError(e)) {
const parserErrors = getParseErrors(e);
if (parserErrors && parserErrors.length) {
@ -533,7 +538,7 @@ class AngularCompilerProgram implements Program {
}
];
}
return null;
return;
}
throw e;
}

2
packages/compiler-cli/test/ngc_spec.ts
View File

@ -1464,7 +1464,7 @@ describe('ngc transformer command-line', () => {
const messages: string[] = [];
const exitCode =
main(['-p', path.join(basePath, 'src/tsconfig.json')], message => messages.push(message));
expect(exitCode).toBe(2, 'Compile was expected to fail');
expect(exitCode).toBe(1, 'Compile was expected to fail');
expect(messages[0]).toContain(['Tagged template expressions are not supported in metadata']);
});

58
packages/compiler-cli/test/perform_watch_spec.ts
View File

@ -105,6 +105,47 @@ describe('perform watch', () => {
expect(getSourceFileSpy !).toHaveBeenCalledWith(mainTsPath, ts.ScriptTarget.ES5);
expect(getSourceFileSpy !).toHaveBeenCalledWith(utilTsPath, ts.ScriptTarget.ES5);
});
it('should recover from static analysis errors', () => {
const config = createConfig();
const host = new MockWatchHost(config);
const okFileContent = `
import {NgModule} from '@angular/core';
@NgModule()
export class MyModule {}
`;
const errorFileContent = `
import {NgModule} from '@angular/core';
@NgModule(() => (1===1 ? null as any : null as any))
export class MyModule {}
`;
const indexTsPath = path.resolve(testSupport.basePath, 'src', 'index.ts');
testSupport.write(indexTsPath, okFileContent);
performWatchCompilation(host);
expectNoDiagnostics(config.options, host.diagnostics);
// Do it multiple times as the watch mode switches internal modes.
// E.g. from regular compile to using summaries, ...
for (let i = 0; i < 3; i++) {
host.diagnostics = [];
testSupport.write(indexTsPath, okFileContent);
host.triggerFileChange(FileChangeEvent.Change, indexTsPath);
expectNoDiagnostics(config.options, host.diagnostics);
host.diagnostics = [];
testSupport.write(indexTsPath, errorFileContent);
host.triggerFileChange(FileChangeEvent.Change, indexTsPath);
const errDiags = host.diagnostics.filter(d => d.category === ts.DiagnosticCategory.Error);
expect(errDiags.length).toBe(1);
expect(errDiags[0].messageText).toContain('Function calls are not supported.');
}
});
});
function createModuleAndCompSource(prefix: string, template: string = prefix + 'template') {
@ -122,7 +163,8 @@ function createModuleAndCompSource(prefix: string, template: string = prefix + '
}
class MockWatchHost {
timeoutListeners: Array<(() => void)|null> = [];
nextTimeoutListenerId = 1;
timeoutListeners: {[id: string]: (() => void)} = {};
fileChangeListeners: Array<((event: FileChangeEvent, fileName: string) => void)|null> = [];
diagnostics: ng.Diagnostics = [];
constructor(public config: ng.ParsedConfiguration) {}
@ -141,16 +183,16 @@ class MockWatchHost {
close: () => this.fileChangeListeners[id] = null,
};
}
setTimeout(callback: () => void, ms: number): any {
const id = this.timeoutListeners.length;
this.timeoutListeners.push(callback);
setTimeout(callback: () => void): any {
const id = this.nextTimeoutListenerId++;
this.timeoutListeners[id] = callback;
return id;
}
clearTimeout(timeoutId: any): void { this.timeoutListeners[timeoutId] = null; }
clearTimeout(timeoutId: any): void { delete this.timeoutListeners[timeoutId]; }
flushTimeouts() {
this.timeoutListeners.forEach(cb => {
if (cb) cb();
});
const listeners = this.timeoutListeners;
this.timeoutListeners = {};
Object.keys(listeners).forEach(id => listeners[id]());
}
triggerFileChange(event: FileChangeEvent, fileName: string) {
this.fileChangeListeners.forEach(listener => {

4
packages/compiler-cli/test/test_support.ts
View File

@ -64,10 +64,10 @@ export function setup(): TestSupport {
function write(fileName: string, content: string) {
const dir = path.dirname(fileName);
if (dir != '.') {
const newDir = path.join(basePath, dir);
const newDir = path.resolve(basePath, dir);
if (!fs.existsSync(newDir)) fs.mkdirSync(newDir);
}
fs.writeFileSync(path.join(basePath, fileName), content, {encoding: 'utf-8'});
fs.writeFileSync(path.resolve(basePath, fileName), content, {encoding: 'utf-8'});
}
function writeFiles(...mockDirs: {[fileName: string]: string}[]) {

62
packages/compiler-cli/test/transformers/program_spec.ts
View File

@ -889,4 +889,66 @@ describe('ng program', () => {
.toContain(
`src/main.html(1,1): error TS100: Property 'nonExistent' does not exist on type 'MyComp'.`);
});
describe('errors', () => {
const fileWithStructuralError = `
import {NgModule} from '@angular/core';
@NgModule(() => (1===1 ? null as any : null as any))
export class MyModule {}
`;
const fileWithGoodContent = `
import {NgModule} from '@angular/core';
@NgModule()
export class MyModule {}
`;
it('should not throw on structural errors but collect them', () => {
testSupport.write('src/index.ts', fileWithStructuralError);
const options = testSupport.createCompilerOptions();
const host = ng.createCompilerHost({options});
const program = ng.createProgram(
{rootNames: [path.resolve(testSupport.basePath, 'src/index.ts')], options, host});
const structuralErrors = program.getNgStructuralDiagnostics();
expect(structuralErrors.length).toBe(1);
expect(structuralErrors[0].messageText).toContain('Function calls are not supported.');
});
it('should not throw on structural errors but collect them (loadNgStructureAsync)', (done) => {
testSupport.write('src/index.ts', fileWithStructuralError);
const options = testSupport.createCompilerOptions();
const host = ng.createCompilerHost({options});
const program = ng.createProgram(
{rootNames: [path.resolve(testSupport.basePath, 'src/index.ts')], options, host});
program.loadNgStructureAsync().then(() => {
const structuralErrors = program.getNgStructuralDiagnostics();
expect(structuralErrors.length).toBe(1);
expect(structuralErrors[0].messageText).toContain('Function calls are not supported.');
done();
});
});
it('should be able to use a program with structural errors as oldProgram', () => {
testSupport.write('src/index.ts', fileWithStructuralError);
const options = testSupport.createCompilerOptions();
const host = ng.createCompilerHost({options});
const program1 = ng.createProgram(
{rootNames: [path.resolve(testSupport.basePath, 'src/index.ts')], options, host});
expect(program1.getNgStructuralDiagnostics().length).toBe(1);
testSupport.write('src/index.ts', fileWithGoodContent);
const program2 = ng.createProgram({
rootNames: [path.resolve(testSupport.basePath, 'src/index.ts')],
options,
host,
oldProgram: program1
});
expectNoDiagnosticsInProgram(options, program2);
});
});
});

2
packages/compiler/src/aot/static_reflector.ts
View File

@ -752,7 +752,7 @@ class PopulatedScope extends BindingScope {
}
function positionalError(message: string, fileName: string, line: number, column: number): Error {
const result = new Error(message);
const result = syntaxError(message);
(result as any).fileName = fileName;
(result as any).line = line;
(result as any).column = column;