fix(compiler-cli): Allow analysis to continue with invalid style url (#41403)
Currently, we throw a FatalDiagnosticError when we fail to load a resource (`templateUrl` or `styleUrl`) at various stages in the compiler. This prevents analysis of the component from completing. This will result in in users not being able to get any information in the component template when there is a missing `styleUrl`, for example. This commit simply tracks the diagnostic, marks the component as poisoned, and continues merrily along. Environments configured to use poisoned data (like the language service) will then be able to use other information from the analysis. Fixes https://github.com/angular/vscode-ng-language-service/issues/1241 PR Close #41403
This commit is contained in:
parent
f98f379dd8
commit
8f12f47492
|
@ -10,7 +10,7 @@ import {compileComponentFromMetadata, compileDeclareComponentFromMetadata, Const
|
||||||
import * as ts from 'typescript';
|
import * as ts from 'typescript';
|
||||||
|
|
||||||
import {Cycle, CycleAnalyzer, CycleHandlingStrategy} from '../../cycles';
|
import {Cycle, CycleAnalyzer, CycleHandlingStrategy} from '../../cycles';
|
||||||
import {ErrorCode, FatalDiagnosticError, makeRelatedInformation} from '../../diagnostics';
|
import {ErrorCode, FatalDiagnosticError, makeDiagnostic, makeRelatedInformation} from '../../diagnostics';
|
||||||
import {absoluteFrom, relative} from '../../file_system';
|
import {absoluteFrom, relative} from '../../file_system';
|
||||||
import {DefaultImportRecorder, ImportedFile, ModuleResolver, Reference, ReferenceEmitter} from '../../imports';
|
import {DefaultImportRecorder, ImportedFile, ModuleResolver, Reference, ReferenceEmitter} from '../../imports';
|
||||||
import {DependencyTracker} from '../../incremental/api';
|
import {DependencyTracker} from '../../incremental/api';
|
||||||
|
@ -262,12 +262,15 @@ export class ComponentDecoratorHandler implements
|
||||||
const component = reflectObjectLiteral(meta);
|
const component = reflectObjectLiteral(meta);
|
||||||
const containingFile = node.getSourceFile().fileName;
|
const containingFile = node.getSourceFile().fileName;
|
||||||
|
|
||||||
const resolveStyleUrl =
|
const resolveStyleUrl = (styleUrl: string): Promise<void>|undefined => {
|
||||||
(styleUrl: string, nodeForError: ts.Node,
|
try {
|
||||||
resourceType: ResourceTypeForDiagnostics): Promise<void>|undefined => {
|
const resourceUrl = this.resourceLoader.resolve(styleUrl, containingFile);
|
||||||
const resourceUrl =
|
|
||||||
this._resolveResourceOrThrow(styleUrl, containingFile, nodeForError, resourceType);
|
|
||||||
return this.resourceLoader.preload(resourceUrl, {type: 'style', containingFile});
|
return this.resourceLoader.preload(resourceUrl, {type: 'style', containingFile});
|
||||||
|
} catch {
|
||||||
|
// Don't worry about failures to preload. We can handle this problem during analysis by
|
||||||
|
// producing a diagnostic.
|
||||||
|
return undefined;
|
||||||
|
}
|
||||||
};
|
};
|
||||||
|
|
||||||
// A Promise that waits for the template and all <link>ed styles within it to be preloaded.
|
// A Promise that waits for the template and all <link>ed styles within it to be preloaded.
|
||||||
|
@ -278,12 +281,7 @@ export class ComponentDecoratorHandler implements
|
||||||
return undefined;
|
return undefined;
|
||||||
}
|
}
|
||||||
|
|
||||||
const nodeForError = getTemplateDeclarationNodeForError(template.declaration);
|
return Promise.all(template.styleUrls.map(styleUrl => resolveStyleUrl(styleUrl)))
|
||||||
return Promise
|
|
||||||
.all(template.styleUrls.map(
|
|
||||||
styleUrl => resolveStyleUrl(
|
|
||||||
styleUrl, nodeForError,
|
|
||||||
ResourceTypeForDiagnostics.StylesheetFromTemplate)))
|
|
||||||
.then(() => undefined);
|
.then(() => undefined);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
@ -311,10 +309,7 @@ export class ComponentDecoratorHandler implements
|
||||||
return Promise
|
return Promise
|
||||||
.all([
|
.all([
|
||||||
templateAndTemplateStyleResources, inlineStyles,
|
templateAndTemplateStyleResources, inlineStyles,
|
||||||
...componentStyleUrls.map(
|
...componentStyleUrls.map(styleUrl => resolveStyleUrl(styleUrl.url))
|
||||||
styleUrl => resolveStyleUrl(
|
|
||||||
styleUrl.url, styleUrl.nodeForError,
|
|
||||||
ResourceTypeForDiagnostics.StylesheetFromDecorator))
|
|
||||||
])
|
])
|
||||||
.then(() => undefined);
|
.then(() => undefined);
|
||||||
}
|
}
|
||||||
|
@ -326,6 +321,8 @@ export class ComponentDecoratorHandler implements
|
||||||
const containingFile = node.getSourceFile().fileName;
|
const containingFile = node.getSourceFile().fileName;
|
||||||
this.literalCache.delete(decorator);
|
this.literalCache.delete(decorator);
|
||||||
|
|
||||||
|
let diagnostics: ts.Diagnostic[]|undefined;
|
||||||
|
let isPoisoned = false;
|
||||||
// @Component inherits @Directive, so begin by extracting the @Directive metadata and building
|
// @Component inherits @Directive, so begin by extracting the @Directive metadata and building
|
||||||
// on it.
|
// on it.
|
||||||
const directiveResult = extractDirectiveMetadata(
|
const directiveResult = extractDirectiveMetadata(
|
||||||
|
@ -408,17 +405,25 @@ export class ComponentDecoratorHandler implements
|
||||||
];
|
];
|
||||||
|
|
||||||
for (const styleUrl of styleUrls) {
|
for (const styleUrl of styleUrls) {
|
||||||
const resourceType = styleUrl.source === ResourceTypeForDiagnostics.StylesheetFromDecorator ?
|
try {
|
||||||
ResourceTypeForDiagnostics.StylesheetFromDecorator :
|
const resourceUrl = this.resourceLoader.resolve(styleUrl.url, containingFile);
|
||||||
ResourceTypeForDiagnostics.StylesheetFromTemplate;
|
|
||||||
const resourceUrl = this._resolveResourceOrThrow(
|
|
||||||
styleUrl.url, containingFile, styleUrl.nodeForError, resourceType);
|
|
||||||
const resourceStr = this.resourceLoader.load(resourceUrl);
|
const resourceStr = this.resourceLoader.load(resourceUrl);
|
||||||
|
|
||||||
styles.push(resourceStr);
|
styles.push(resourceStr);
|
||||||
if (this.depTracker !== null) {
|
if (this.depTracker !== null) {
|
||||||
this.depTracker.addResourceDependency(node.getSourceFile(), absoluteFrom(resourceUrl));
|
this.depTracker.addResourceDependency(node.getSourceFile(), absoluteFrom(resourceUrl));
|
||||||
}
|
}
|
||||||
|
} catch {
|
||||||
|
if (diagnostics === undefined) {
|
||||||
|
diagnostics = [];
|
||||||
|
}
|
||||||
|
const resourceType =
|
||||||
|
styleUrl.source === ResourceTypeForDiagnostics.StylesheetFromDecorator ?
|
||||||
|
ResourceTypeForDiagnostics.StylesheetFromDecorator :
|
||||||
|
ResourceTypeForDiagnostics.StylesheetFromTemplate;
|
||||||
|
diagnostics.push(
|
||||||
|
this.makeResourceNotFoundError(styleUrl.url, styleUrl.nodeForError, resourceType)
|
||||||
|
.toDiagnostic());
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// If inline styles were preprocessed use those
|
// If inline styles were preprocessed use those
|
||||||
|
@ -496,8 +501,9 @@ export class ComponentDecoratorHandler implements
|
||||||
styles: styleResources,
|
styles: styleResources,
|
||||||
template: templateResource,
|
template: templateResource,
|
||||||
},
|
},
|
||||||
isPoisoned: false,
|
isPoisoned,
|
||||||
},
|
},
|
||||||
|
diagnostics,
|
||||||
};
|
};
|
||||||
if (changeDetection !== null) {
|
if (changeDetection !== null) {
|
||||||
output.analysis!.meta.changeDetection = changeDetection;
|
output.analysis!.meta.changeDetection = changeDetection;
|
||||||
|
@ -833,14 +839,14 @@ export class ComponentDecoratorHandler implements
|
||||||
let styles: string[] = [];
|
let styles: string[] = [];
|
||||||
if (analysis.styleUrls !== null) {
|
if (analysis.styleUrls !== null) {
|
||||||
for (const styleUrl of analysis.styleUrls) {
|
for (const styleUrl of analysis.styleUrls) {
|
||||||
const resourceType =
|
try {
|
||||||
styleUrl.source === ResourceTypeForDiagnostics.StylesheetFromDecorator ?
|
const resolvedStyleUrl = this.resourceLoader.resolve(styleUrl.url, containingFile);
|
||||||
ResourceTypeForDiagnostics.StylesheetFromDecorator :
|
|
||||||
ResourceTypeForDiagnostics.StylesheetFromTemplate;
|
|
||||||
const resolvedStyleUrl = this._resolveResourceOrThrow(
|
|
||||||
styleUrl.url, containingFile, styleUrl.nodeForError, resourceType);
|
|
||||||
const styleText = this.resourceLoader.load(resolvedStyleUrl);
|
const styleText = this.resourceLoader.load(resolvedStyleUrl);
|
||||||
styles.push(styleText);
|
styles.push(styleText);
|
||||||
|
} catch (e) {
|
||||||
|
// Resource resolve failures should already be in the diagnostics list from the analyze
|
||||||
|
// stage. We do not need to do anything with them when updating resources.
|
||||||
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
if (analysis.inlineStyles !== null) {
|
if (analysis.inlineStyles !== null) {
|
||||||
|
@ -978,10 +984,14 @@ export class ComponentDecoratorHandler implements
|
||||||
const styleUrlsExpr = component.get('styleUrls');
|
const styleUrlsExpr = component.get('styleUrls');
|
||||||
if (styleUrlsExpr !== undefined && ts.isArrayLiteralExpression(styleUrlsExpr)) {
|
if (styleUrlsExpr !== undefined && ts.isArrayLiteralExpression(styleUrlsExpr)) {
|
||||||
for (const expression of stringLiteralElements(styleUrlsExpr)) {
|
for (const expression of stringLiteralElements(styleUrlsExpr)) {
|
||||||
const resourceUrl = this._resolveResourceOrThrow(
|
try {
|
||||||
expression.text, containingFile, expression,
|
const resourceUrl = this.resourceLoader.resolve(expression.text, containingFile);
|
||||||
ResourceTypeForDiagnostics.StylesheetFromDecorator);
|
|
||||||
styles.add({path: absoluteFrom(resourceUrl), expression});
|
styles.add({path: absoluteFrom(resourceUrl), expression});
|
||||||
|
} catch {
|
||||||
|
// Errors in style resource extraction do not need to be handled here. We will produce
|
||||||
|
// diagnostics for each one that fails in the analysis, after we evaluate the `styleUrls`
|
||||||
|
// expression to determine _all_ style resources, not just the string literals.
|
||||||
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -1006,8 +1016,8 @@ export class ComponentDecoratorHandler implements
|
||||||
throw createValueHasWrongTypeError(
|
throw createValueHasWrongTypeError(
|
||||||
templateUrlExpr, templateUrl, 'templateUrl must be a string');
|
templateUrlExpr, templateUrl, 'templateUrl must be a string');
|
||||||
}
|
}
|
||||||
const resourceUrl = this._resolveResourceOrThrow(
|
try {
|
||||||
templateUrl, containingFile, templateUrlExpr, ResourceTypeForDiagnostics.Template);
|
const resourceUrl = this.resourceLoader.resolve(templateUrl, containingFile);
|
||||||
const templatePromise =
|
const templatePromise =
|
||||||
this.resourceLoader.preload(resourceUrl, {type: 'template', containingFile});
|
this.resourceLoader.preload(resourceUrl, {type: 'template', containingFile});
|
||||||
|
|
||||||
|
@ -1015,7 +1025,8 @@ export class ComponentDecoratorHandler implements
|
||||||
// URLs to resolve.
|
// URLs to resolve.
|
||||||
if (templatePromise !== undefined) {
|
if (templatePromise !== undefined) {
|
||||||
return templatePromise.then(() => {
|
return templatePromise.then(() => {
|
||||||
const templateDecl = this.parseTemplateDeclaration(decorator, component, containingFile);
|
const templateDecl =
|
||||||
|
this.parseTemplateDeclaration(decorator, component, containingFile);
|
||||||
const template = this.extractTemplate(node, templateDecl);
|
const template = this.extractTemplate(node, templateDecl);
|
||||||
this.preanalyzeTemplateCache.set(node, template);
|
this.preanalyzeTemplateCache.set(node, template);
|
||||||
return template;
|
return template;
|
||||||
|
@ -1023,6 +1034,10 @@ export class ComponentDecoratorHandler implements
|
||||||
} else {
|
} else {
|
||||||
return Promise.resolve(null);
|
return Promise.resolve(null);
|
||||||
}
|
}
|
||||||
|
} catch (e) {
|
||||||
|
throw this.makeResourceNotFoundError(
|
||||||
|
templateUrl, templateUrlExpr, ResourceTypeForDiagnostics.Template);
|
||||||
|
}
|
||||||
} else {
|
} else {
|
||||||
const templateDecl = this.parseTemplateDeclaration(decorator, component, containingFile);
|
const templateDecl = this.parseTemplateDeclaration(decorator, component, containingFile);
|
||||||
const template = this.extractTemplate(node, templateDecl);
|
const template = this.extractTemplate(node, templateDecl);
|
||||||
|
@ -1186,9 +1201,8 @@ export class ComponentDecoratorHandler implements
|
||||||
throw createValueHasWrongTypeError(
|
throw createValueHasWrongTypeError(
|
||||||
templateUrlExpr, templateUrl, 'templateUrl must be a string');
|
templateUrlExpr, templateUrl, 'templateUrl must be a string');
|
||||||
}
|
}
|
||||||
const resourceUrl = this._resolveResourceOrThrow(
|
try {
|
||||||
templateUrl, containingFile, templateUrlExpr, ResourceTypeForDiagnostics.Template);
|
const resourceUrl = this.resourceLoader.resolve(templateUrl, containingFile);
|
||||||
|
|
||||||
return {
|
return {
|
||||||
isInline: false,
|
isInline: false,
|
||||||
interpolationConfig,
|
interpolationConfig,
|
||||||
|
@ -1198,6 +1212,10 @@ export class ComponentDecoratorHandler implements
|
||||||
resolvedTemplateUrl: resourceUrl,
|
resolvedTemplateUrl: resourceUrl,
|
||||||
sourceMapUrl: sourceMapUrl(resourceUrl),
|
sourceMapUrl: sourceMapUrl(resourceUrl),
|
||||||
};
|
};
|
||||||
|
} catch (e) {
|
||||||
|
throw this.makeResourceNotFoundError(
|
||||||
|
templateUrl, templateUrlExpr, ResourceTypeForDiagnostics.Template);
|
||||||
|
}
|
||||||
} else if (component.has('template')) {
|
} else if (component.has('template')) {
|
||||||
return {
|
return {
|
||||||
isInline: true,
|
isInline: true,
|
||||||
|
@ -1260,17 +1278,9 @@ export class ComponentDecoratorHandler implements
|
||||||
this.cycleAnalyzer.recordSyntheticImport(origin, imported);
|
this.cycleAnalyzer.recordSyntheticImport(origin, imported);
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
private makeResourceNotFoundError(
|
||||||
* Resolve the url of a resource relative to the file that contains the reference to it.
|
file: string, nodeForError: ts.Node,
|
||||||
*
|
resourceType: ResourceTypeForDiagnostics): FatalDiagnosticError {
|
||||||
* Throws a FatalDiagnosticError when unable to resolve the file.
|
|
||||||
*/
|
|
||||||
private _resolveResourceOrThrow(
|
|
||||||
file: string, basePath: string, nodeForError: ts.Node,
|
|
||||||
resourceType: ResourceTypeForDiagnostics): string {
|
|
||||||
try {
|
|
||||||
return this.resourceLoader.resolve(file, basePath);
|
|
||||||
} catch (e) {
|
|
||||||
let errorText: string;
|
let errorText: string;
|
||||||
switch (resourceType) {
|
switch (resourceType) {
|
||||||
case ResourceTypeForDiagnostics.Template:
|
case ResourceTypeForDiagnostics.Template:
|
||||||
|
@ -1284,10 +1294,9 @@ export class ComponentDecoratorHandler implements
|
||||||
break;
|
break;
|
||||||
}
|
}
|
||||||
|
|
||||||
throw new FatalDiagnosticError(
|
return new FatalDiagnosticError(
|
||||||
ErrorCode.COMPONENT_RESOURCE_NOT_FOUND, nodeForError, errorText);
|
ErrorCode.COMPONENT_RESOURCE_NOT_FOUND, nodeForError, errorText);
|
||||||
}
|
}
|
||||||
}
|
|
||||||
|
|
||||||
private _extractTemplateStyleUrls(template: ParsedTemplateWithSource): StyleUrlMeta[] {
|
private _extractTemplateStyleUrls(template: ParsedTemplateWithSource): StyleUrlMeta[] {
|
||||||
if (template.styleUrls === null) {
|
if (template.styleUrls === null) {
|
||||||
|
|
|
@ -553,8 +553,40 @@ describe('quick info', () => {
|
||||||
expectQuickInfo(
|
expectQuickInfo(
|
||||||
{templateOverride, expectedSpanText: 'date', expectedDisplayString: '(pipe) DatePipe'});
|
{templateOverride, expectedSpanText: 'date', expectedDisplayString: '(pipe) DatePipe'});
|
||||||
});
|
});
|
||||||
});
|
|
||||||
|
|
||||||
|
it('should still get quick info if there is an invalid css resource', () => {
|
||||||
|
project = env.addProject('test', {
|
||||||
|
'app.ts': `
|
||||||
|
import {Component, NgModule} from '@angular/core';
|
||||||
|
|
||||||
|
@Component({
|
||||||
|
selector: 'some-cmp',
|
||||||
|
templateUrl: './app.html',
|
||||||
|
styleUrls: ['./does_not_exist'],
|
||||||
|
})
|
||||||
|
export class SomeCmp {
|
||||||
|
myValue!: string;
|
||||||
|
}
|
||||||
|
|
||||||
|
@NgModule({
|
||||||
|
declarations: [SomeCmp],
|
||||||
|
})
|
||||||
|
export class AppModule{
|
||||||
|
}
|
||||||
|
`,
|
||||||
|
'app.html': `{{myValue}}`,
|
||||||
|
});
|
||||||
|
const diagnostics = project.getDiagnosticsForFile('app.ts');
|
||||||
|
expect(diagnostics.length).toBe(1);
|
||||||
|
expect(diagnostics[0].messageText)
|
||||||
|
.toEqual(`Could not find stylesheet file './does_not_exist'.`);
|
||||||
|
|
||||||
|
const template = project.openFile('app.html');
|
||||||
|
template.moveCursorToText('{{myVa¦lue}}');
|
||||||
|
const quickInfo = template.getQuickInfoAtPosition();
|
||||||
|
expect(toText(quickInfo!.displayParts)).toEqual('(property) SomeCmp.myValue: string');
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
function expectQuickInfo(
|
function expectQuickInfo(
|
||||||
{templateOverride, expectedSpanText, expectedDisplayString}:
|
{templateOverride, expectedSpanText, expectedDisplayString}:
|
||||||
|
|
Loading…
Reference in New Issue