fix(ivy): capture template source mapping details during preanalysis (#32544)

Prior to this change, the template source mapping details were always
built during the analysis phase, under the assumption that pre-analysed
templates would always correspond with external templates. This has
turned out to be a false assumption, as inline templates are also
pre-analyzed to be able to preload any stylesheets included in the
template.

This commit fixes the bug by capturing the template source mapping
details at the moment the template is parsed, which is either during the
preanalysis phase when preloading is available, or during the analysis
phase when preloading is not supported.

Tests have been added to exercise the template error mapping in
asynchronous compilations where preloading is enabled, similar to how
the CLI performs compilations.

Fixes #32538

PR Close #32544
This commit is contained in:
JoostK
2019-09-07 23:48:32 +02:00
committed by Matias Niemelä
parent a391aebbcf
commit a64eded521
5 changed files with 140 additions and 111 deletions

View File

@ -6,7 +6,7 @@
* found in the LICENSE file at https://angular.io/license
*/
import {CustomTransformers, Program} from '@angular/compiler-cli';
import {CustomTransformers, Program, defaultGatherDiagnostics} from '@angular/compiler-cli';
import * as api from '@angular/compiler-cli/src/transformers/api';
import * as ts from 'typescript';
@ -100,6 +100,15 @@ export class NgtscTestEnvironment {
setWrapHostForTest(makeWrapHost(this.multiCompileHostExt));
}
/**
* Installs a compiler host that allows for asynchronous reading of resources by implementing the
* `CompilerHost.readResource` method. Note that only asynchronous compilations are affected, as
* synchronous compilations do not use the asynchronous resource loader.
*/
enablePreloading(): void {
setWrapHostForTest(makeWrapHost(new ResourceLoadingCompileHost(this.fs)));
}
flushWrittenFileTracking(): void {
if (this.multiCompileHostExt === null) {
throw new Error(`Not tracking written files - call enableMultipleCompilations()`);
@ -192,6 +201,16 @@ export class NgtscTestEnvironment {
return mainDiagnosticsForTest(['-p', this.basePath]) as ts.Diagnostic[];
}
async driveDiagnosticsAsync(): Promise<ReadonlyArray<ts.Diagnostic>> {
const {rootNames, options} = readNgcCommandLineAndConfiguration(['-p', this.basePath]);
const host = createCompilerHost({options});
const program = createProgram({rootNames, host, options});
await program.loadNgStructureAsync();
// ngtsc only produces ts.Diagnostic messages.
return defaultGatherDiagnostics(program as api.Program) as ts.Diagnostic[];
}
driveRoutes(entryPoint?: string): LazyRoute[] {
const {rootNames, options} = readNgcCommandLineAndConfiguration(['-p', this.basePath]);
const host = createCompilerHost({options});
@ -251,6 +270,16 @@ class MultiCompileHostExt extends AugmentedCompilerHost implements Partial<ts.Co
invalidate(fileName: string): void { this.cache.delete(fileName); }
}
class ResourceLoadingCompileHost extends AugmentedCompilerHost implements api.CompilerHost {
readResource(fileName: string): Promise<string>|string {
const resource = this.readFile(fileName);
if (resource === undefined) {
throw new Error(`Resource ${fileName} not found`);
}
return resource;
}
}
function makeWrapHost(wrapped: AugmentedCompilerHost): (host: ts.CompilerHost) => ts.CompilerHost {
return (delegate) => {
wrapped.delegate = delegate;

View File

@ -469,9 +469,21 @@ export declare class CommonModule {
});
});
describe('error locations', () => {
it('should be correct for direct templates', () => {
env.write('test.ts', `
// Test both sync and async compilations, see https://github.com/angular/angular/issues/32538
['sync', 'async'].forEach(mode => {
describe(`error locations [${mode}]`, () => {
let driveDiagnostics: () => Promise<ReadonlyArray<ts.Diagnostic>>;
beforeEach(() => {
if (mode === 'async') {
env.enablePreloading();
driveDiagnostics = () => env.driveDiagnosticsAsync();
} else {
driveDiagnostics = () => Promise.resolve(env.driveDiagnostics());
}
});
it('should be correct for direct templates', async() => {
env.write('test.ts', `
import {Component, NgModule} from '@angular/core';
@Component({
@ -484,14 +496,14 @@ export declare class CommonModule {
user: {name: string}[];
}`);
const diags = env.driveDiagnostics();
expect(diags.length).toBe(1);
expect(diags[0].file !.fileName).toBe(_('/test.ts'));
expect(getSourceCodeForDiagnostic(diags[0])).toBe('user.does_not_exist');
});
const diags = await driveDiagnostics();
expect(diags.length).toBe(1);
expect(diags[0].file !.fileName).toBe(_('/test.ts'));
expect(getSourceCodeForDiagnostic(diags[0])).toBe('user.does_not_exist');
});
it('should be correct for indirect templates', () => {
env.write('test.ts', `
it('should be correct for indirect templates', async() => {
env.write('test.ts', `
import {Component, NgModule} from '@angular/core';
const TEMPLATE = \`<p>
@ -506,18 +518,18 @@ export declare class CommonModule {
user: {name: string}[];
}`);
const diags = env.driveDiagnostics();
expect(diags.length).toBe(1);
expect(diags[0].file !.fileName).toBe(_('/test.ts') + ' (TestCmp template)');
expect(getSourceCodeForDiagnostic(diags[0])).toBe('user.does_not_exist');
expect(getSourceCodeForDiagnostic(diags[0].relatedInformation ![0])).toBe('TEMPLATE');
});
const diags = await driveDiagnostics();
expect(diags.length).toBe(1);
expect(diags[0].file !.fileName).toBe(_('/test.ts') + ' (TestCmp template)');
expect(getSourceCodeForDiagnostic(diags[0])).toBe('user.does_not_exist');
expect(getSourceCodeForDiagnostic(diags[0].relatedInformation ![0])).toBe('TEMPLATE');
});
it('should be correct for external templates', () => {
env.write('template.html', `<p>
it('should be correct for external templates', async() => {
env.write('template.html', `<p>
{{user.does_not_exist}}
</p>`);
env.write('test.ts', `
env.write('test.ts', `
import {Component, NgModule} from '@angular/core';
@ -529,12 +541,13 @@ export declare class CommonModule {
user: {name: string}[];
}`);
const diags = env.driveDiagnostics();
expect(diags.length).toBe(1);
expect(diags[0].file !.fileName).toBe(_('/template.html'));
expect(getSourceCodeForDiagnostic(diags[0])).toBe('user.does_not_exist');
expect(getSourceCodeForDiagnostic(diags[0].relatedInformation ![0]))
.toBe(`'./template.html'`);
const diags = await driveDiagnostics();
expect(diags.length).toBe(1);
expect(diags[0].file !.fileName).toBe(_('/template.html'));
expect(getSourceCodeForDiagnostic(diags[0])).toBe('user.does_not_exist');
expect(getSourceCodeForDiagnostic(diags[0].relatedInformation ![0]))
.toBe(`'./template.html'`);
});
});
});
});