diff --git a/packages/compiler-cli/src/ngtsc/annotations/src/component.ts b/packages/compiler-cli/src/ngtsc/annotations/src/component.ts index 091c0f20fe..72ad3d4893 100644 --- a/packages/compiler-cli/src/ngtsc/annotations/src/component.ts +++ b/packages/compiler-cli/src/ngtsc/annotations/src/component.ts @@ -180,67 +180,32 @@ export class ComponentDecoratorHandler implements let templateSourceMapping: TemplateSourceMapping; if (this.preanalyzeTemplateCache.has(node)) { // The template was parsed in preanalyze. Use it and delete it to save memory. - const template = this.preanalyzeTemplateCache.get(node) !; + const preanalyzed = this.preanalyzeTemplateCache.get(node) !; this.preanalyzeTemplateCache.delete(node); - parseTemplate = template.parseTemplate; - - // A pre-analyzed template is always an external mapping. - templateSourceMapping = { - type: 'external', - componentClass: node, - node: component.get('templateUrl') !, - template: template.template, - templateUrl: template.templateUrl, - }; + parseTemplate = preanalyzed.parseTemplate; + templateSourceMapping = preanalyzed.templateSourceMapping; } else { // The template was not already parsed. Either there's a templateUrl, or an inline template. if (component.has('templateUrl')) { const templateUrlExpr = component.get('templateUrl') !; - const evalTemplateUrl = this.evaluator.evaluate(templateUrlExpr); - if (typeof evalTemplateUrl !== 'string') { + const templateUrl = this.evaluator.evaluate(templateUrlExpr); + if (typeof templateUrl !== 'string') { throw new FatalDiagnosticError( ErrorCode.VALUE_HAS_WRONG_TYPE, templateUrlExpr, 'templateUrl must be a string'); } - const templateUrl = this.resourceLoader.resolve(evalTemplateUrl, containingFile); - const templateStr = this.resourceLoader.load(templateUrl); - this.resourceDependencies.recordResourceDependency(node.getSourceFile(), templateUrl); + const resourceUrl = this.resourceLoader.resolve(templateUrl, containingFile); + const external = + this._extractExternalTemplate(node, component, templateUrlExpr, resourceUrl); - parseTemplate = (options?: ParseTemplateOptions) => this._parseTemplate( - component, templateStr, sourceMapUrl(templateUrl), /* templateRange */ undefined, - /* escapedString */ false, options); - templateSourceMapping = { - type: 'external', - componentClass: node, - node: templateUrlExpr, - template: templateStr, - templateUrl: templateUrl, - }; + parseTemplate = external.parseTemplate; + templateSourceMapping = external.templateSourceMapping; } else { // Expect an inline template to be present. - const inlineTemplate = this._extractInlineTemplate(component, containingFile); - if (inlineTemplate === null) { - throw new FatalDiagnosticError( - ErrorCode.COMPONENT_MISSING_TEMPLATE, decorator.node, - 'component is missing a template'); - } - const {templateStr, templateUrl, templateRange, escapedString} = inlineTemplate; - parseTemplate = (options?: ParseTemplateOptions) => this._parseTemplate( - component, templateStr, templateUrl, templateRange, escapedString, options); - if (escapedString) { - templateSourceMapping = { - type: 'direct', - node: - component.get('template') !as(ts.StringLiteral | ts.NoSubstitutionTemplateLiteral), - }; - } else { - templateSourceMapping = { - type: 'indirect', - node: component.get('template') !, - componentClass: node, - template: templateStr, - }; - } + const inline = this._extractInlineTemplate(node, decorator, component, containingFile); + + parseTemplate = inline.parseTemplate; + templateSourceMapping = inline.templateSourceMapping; } } const template = parseTemplate(); @@ -589,7 +554,7 @@ export class ComponentDecoratorHandler implements } private _preloadAndParseTemplate( - node: ts.Declaration, decorator: Decorator, component: Map, + node: ClassDeclaration, decorator: Decorator, component: Map, containingFile: string): Promise { if (component.has('templateUrl')) { // Extract the templateUrl and preload it. @@ -606,50 +571,64 @@ export class ComponentDecoratorHandler implements // URLs to resolve. if (templatePromise !== undefined) { return templatePromise.then(() => { - const templateStr = this.resourceLoader.load(resourceUrl); - this.resourceDependencies.recordResourceDependency(node.getSourceFile(), resourceUrl); - const parseTemplate = (options?: ParseTemplateOptions) => this._parseTemplate( - component, templateStr, sourceMapUrl(resourceUrl), - /* templateRange */ undefined, - /* escapedString */ false, options); + const {parseTemplate, templateSourceMapping} = + this._extractExternalTemplate(node, component, templateUrlExpr, resourceUrl); const template = parseTemplate(); - this.preanalyzeTemplateCache.set(node, {...template, parseTemplate}); + this.preanalyzeTemplateCache.set( + node, {...template, parseTemplate, templateSourceMapping}); return template; }); } else { return Promise.resolve(null); } } else { - const inlineTemplate = this._extractInlineTemplate(component, containingFile); - if (inlineTemplate === null) { - throw new FatalDiagnosticError( - ErrorCode.COMPONENT_MISSING_TEMPLATE, decorator.node, - 'component is missing a template'); - } - - const {templateStr, templateUrl, escapedString, templateRange} = inlineTemplate; - const parseTemplate = (options?: ParseTemplateOptions) => this._parseTemplate( - component, templateStr, templateUrl, templateRange, escapedString, options); + const {parseTemplate, templateSourceMapping} = + this._extractInlineTemplate(node, decorator, component, containingFile); const template = parseTemplate(); - this.preanalyzeTemplateCache.set(node, {...template, parseTemplate}); + this.preanalyzeTemplateCache.set(node, {...template, parseTemplate, templateSourceMapping}); return Promise.resolve(template); } } - private _extractInlineTemplate(component: Map, containingFile: string): { - templateStr: string, - templateUrl: string, - templateRange: LexerRange|undefined, - escapedString: boolean - }|null { - // If there is no inline template, then return null. + private _extractExternalTemplate( + node: ClassDeclaration, component: Map, templateUrlExpr: ts.Expression, + resourceUrl: string): { + parseTemplate: (options?: ParseTemplateOptions) => ParsedTemplate; + templateSourceMapping: TemplateSourceMapping + } { + const templateStr = this.resourceLoader.load(resourceUrl); + this.resourceDependencies.recordResourceDependency(node.getSourceFile(), resourceUrl); + const parseTemplate = (options?: ParseTemplateOptions) => this._parseTemplate( + component, templateStr, sourceMapUrl(resourceUrl), + /* templateRange */ undefined, + /* escapedString */ false, options); + const templateSourceMapping: TemplateSourceMapping = { + type: 'external', + componentClass: node, + node: templateUrlExpr, + template: templateStr, + templateUrl: resourceUrl, + }; + + return {parseTemplate, templateSourceMapping}; + } + + private _extractInlineTemplate( + node: ClassDeclaration, decorator: Decorator, component: Map, + containingFile: string): { + parseTemplate: (options?: ParseTemplateOptions) => ParsedTemplate; + templateSourceMapping: TemplateSourceMapping + } { if (!component.has('template')) { - return null; + throw new FatalDiagnosticError( + ErrorCode.COMPONENT_MISSING_TEMPLATE, decorator.node, 'component is missing a template'); } const templateExpr = component.get('template') !; + let templateStr: string; let templateUrl: string = ''; let templateRange: LexerRange|undefined = undefined; + let templateSourceMapping: TemplateSourceMapping; let escapedString = false; // We only support SourceMaps for inline templates that are simple string literals. if (ts.isStringLiteral(templateExpr) || ts.isNoSubstitutionTemplateLiteral(templateExpr)) { @@ -660,6 +639,10 @@ export class ComponentDecoratorHandler implements templateStr = templateExpr.getSourceFile().text; templateUrl = containingFile; escapedString = true; + templateSourceMapping = { + type: 'direct', + node: templateExpr as(ts.StringLiteral | ts.NoSubstitutionTemplateLiteral), + }; } else { const resolvedTemplate = this.evaluator.evaluate(templateExpr); if (typeof resolvedTemplate !== 'string') { @@ -667,8 +650,18 @@ export class ComponentDecoratorHandler implements ErrorCode.VALUE_HAS_WRONG_TYPE, templateExpr, 'template must be a string'); } templateStr = resolvedTemplate; + templateSourceMapping = { + type: 'indirect', + node: templateExpr, + componentClass: node, + template: templateStr, + }; } - return {templateStr, templateUrl, templateRange, escapedString}; + + const parseTemplate = (options?: ParseTemplateOptions) => this._parseTemplate( + component, templateStr, templateUrl, templateRange, escapedString, options); + + return {parseTemplate, templateSourceMapping}; } private _parseTemplate( @@ -826,4 +819,5 @@ export interface ParsedTemplate { interface PreanalyzedTemplate extends ParsedTemplate { parseTemplate: (options?: ParseTemplateOptions) => ParsedTemplate; + templateSourceMapping: TemplateSourceMapping; } diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/diagnostics.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/diagnostics.ts index 7e94aca70b..ed40d0a47d 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/src/diagnostics.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/src/diagnostics.ts @@ -141,13 +141,6 @@ export function translateDiagnostic( return null; } - let messageText: string; - if (typeof diagnostic.messageText === 'string') { - messageText = diagnostic.messageText; - } else { - messageText = diagnostic.messageText.messageText; - } - const mapping = resolver.getSourceMapping(sourceLocation.id); return makeTemplateDiagnostic( mapping, span, diagnostic.category, diagnostic.code, diagnostic.messageText); diff --git a/packages/compiler-cli/src/perform_compile.ts b/packages/compiler-cli/src/perform_compile.ts index 059677d9c4..8e7aa8ca59 100644 --- a/packages/compiler-cli/src/perform_compile.ts +++ b/packages/compiler-cli/src/perform_compile.ts @@ -283,7 +283,7 @@ export function performCompilation( return {diagnostics: allDiagnostics, program}; } } -function defaultGatherDiagnostics(program: api.Program): Diagnostics { +export function defaultGatherDiagnostics(program: api.Program): Diagnostics { const allDiagnostics: Array = []; function checkDiagnostics(diags: Diagnostics | undefined) { diff --git a/packages/compiler-cli/test/ngtsc/env.ts b/packages/compiler-cli/test/ngtsc/env.ts index cf274ab618..78cc8b6c90 100644 --- a/packages/compiler-cli/test/ngtsc/env.ts +++ b/packages/compiler-cli/test/ngtsc/env.ts @@ -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> { + 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|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; diff --git a/packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts b/packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts index b19425602c..a3be05aa27 100644 --- a/packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts +++ b/packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts @@ -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>; + 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 = \`

@@ -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', `

+ it('should be correct for external templates', async() => { + env.write('template.html', `

{{user.does_not_exist}}

`); - 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'`); + }); }); }); });