refactor(compiler-cli): Store inline templates and styles in the resource registry (#39482)

The Language Service is not only interested in external resources, but
also inline styles and templates. By storing the expression of the
inline resources, we can more easily determine if a given position is
part of the inline template/style expression.

PR Close #39482
This commit is contained in:
Andrew Scott 2020-10-28 15:34:14 -07:00 committed by Joey Perrott
parent 3241d922fc
commit beb935613e
4 changed files with 183 additions and 61 deletions

View File

@ -262,7 +262,7 @@ export class ComponentDecoratorHandler implements
} }
} }
const templateResource = template.isInline ? const templateResource = template.isInline ?
null : {path: null, expression: component.get('template')!} :
{path: absoluteFrom(template.templateUrl), expression: template.sourceMapping.node}; {path: absoluteFrom(template.templateUrl), expression: template.sourceMapping.node};
let diagnostics: ts.Diagnostic[]|undefined = undefined; let diagnostics: ts.Diagnostic[]|undefined = undefined;
@ -666,21 +666,30 @@ export class ComponentDecoratorHandler implements
private _extractStyleResources(component: Map<string, ts.Expression>, containingFile: string): private _extractStyleResources(component: Map<string, ts.Expression>, containingFile: string):
ReadonlySet<Resource> { ReadonlySet<Resource> {
const styleUrlsExpr = component.get('styleUrls'); const styles = new Set<Resource>();
// If styleUrls is a literal array of strings, process each resource url individually and function stringLiteralElements(array: ts.ArrayLiteralExpression): ts.StringLiteralLike[] {
// register them. Otherwise, give up and return an empty set. return array.elements.filter(
if (styleUrlsExpr === undefined || !ts.isArrayLiteralExpression(styleUrlsExpr) || (e: ts.Expression): e is ts.StringLiteralLike => ts.isStringLiteralLike(e));
!styleUrlsExpr.elements.every(e => ts.isStringLiteralLike(e))) {
return new Set();
} }
const externalStyles = new Set<Resource>(); // If styleUrls is a literal array, process each resource url individually and
for (const expression of styleUrlsExpr.elements) { // register ones that are string literals.
const resourceUrl = const styleUrlsExpr = component.get('styleUrls');
this.resourceLoader.resolve((expression as ts.StringLiteralLike).text, containingFile); if (styleUrlsExpr !== undefined && ts.isArrayLiteralExpression(styleUrlsExpr)) {
externalStyles.add({path: absoluteFrom(resourceUrl), expression}); for (const expression of stringLiteralElements(styleUrlsExpr)) {
const resourceUrl = this.resourceLoader.resolve(expression.text, containingFile);
styles.add({path: absoluteFrom(resourceUrl), expression});
}
} }
return externalStyles;
const stylesExpr = component.get('styles');
if (stylesExpr !== undefined && ts.isArrayLiteralExpression(stylesExpr)) {
for (const expression of stringLiteralElements(stylesExpr)) {
styles.add({path: null, expression});
}
}
return styles;
} }
private _preloadAndParseTemplate( private _preloadAndParseTemplate(

View File

@ -18,18 +18,49 @@ import {getDeclaration, makeProgram} from '../../testing';
import {ResourceLoader} from '../src/api'; import {ResourceLoader} from '../src/api';
import {ComponentDecoratorHandler} from '../src/component'; import {ComponentDecoratorHandler} from '../src/component';
export class NoopResourceLoader implements ResourceLoader { export class StubResourceLoader implements ResourceLoader {
resolve(): string { resolve(v: string): string {
throw new Error('Not implemented.'); return v;
} }
canPreload = false; canPreload = false;
load(): string { load(v: string): string {
throw new Error('Not implemented'); return '';
} }
preload(): Promise<void>|undefined { preload(): Promise<void>|undefined {
throw new Error('Not implemented'); throw new Error('Not implemented');
} }
} }
function setup(program: ts.Program, options: ts.CompilerOptions, host: ts.CompilerHost) {
const checker = program.getTypeChecker();
const reflectionHost = new TypeScriptReflectionHost(checker);
const evaluator = new PartialEvaluator(reflectionHost, checker, /* dependencyTracker */ null);
const moduleResolver =
new ModuleResolver(program, options, host, /* moduleResolutionCache */ null);
const importGraph = new ImportGraph(moduleResolver);
const cycleAnalyzer = new CycleAnalyzer(importGraph);
const metaRegistry = new LocalMetadataRegistry();
const dtsReader = new DtsMetadataReader(checker, reflectionHost);
const scopeRegistry = new LocalModuleScopeRegistry(
metaRegistry, new MetadataDtsModuleScopeResolver(dtsReader, null), new ReferenceEmitter([]),
null);
const metaReader = new CompoundMetadataReader([metaRegistry, dtsReader]);
const refEmitter = new ReferenceEmitter([]);
const injectableRegistry = new InjectableClassRegistry(reflectionHost);
const resourceRegistry = new ResourceRegistry();
const handler = new ComponentDecoratorHandler(
reflectionHost, evaluator, metaRegistry, metaReader, scopeRegistry, scopeRegistry,
resourceRegistry,
/* isCore */ false, new StubResourceLoader(), /* rootDirs */['/'],
/* defaultPreserveWhitespaces */ false, /* i18nUseExternalIds */ true,
/* enableI18nLegacyMessageIdFormat */ false,
/* i18nNormalizeLineEndingsInICUs */ undefined, moduleResolver, cycleAnalyzer, refEmitter,
NOOP_DEFAULT_IMPORT_RECORDER, /* depTracker */ null, injectableRegistry,
/* annotateForClosureCompiler */ false);
return {reflectionHost, handler};
}
runInEachFileSystem(() => { runInEachFileSystem(() => {
describe('ComponentDecoratorHandler', () => { describe('ComponentDecoratorHandler', () => {
let _: typeof absoluteFrom; let _: typeof absoluteFrom;
@ -51,31 +82,7 @@ runInEachFileSystem(() => {
` `
}, },
]); ]);
const checker = program.getTypeChecker(); const {reflectionHost, handler} = setup(program, options, host);
const reflectionHost = new TypeScriptReflectionHost(checker);
const evaluator = new PartialEvaluator(reflectionHost, checker, /* dependencyTracker */ null);
const moduleResolver =
new ModuleResolver(program, options, host, /* moduleResolutionCache */ null);
const importGraph = new ImportGraph(moduleResolver);
const cycleAnalyzer = new CycleAnalyzer(importGraph);
const metaRegistry = new LocalMetadataRegistry();
const dtsReader = new DtsMetadataReader(checker, reflectionHost);
const scopeRegistry = new LocalModuleScopeRegistry(
metaRegistry, new MetadataDtsModuleScopeResolver(dtsReader, null),
new ReferenceEmitter([]), null);
const metaReader = new CompoundMetadataReader([metaRegistry, dtsReader]);
const refEmitter = new ReferenceEmitter([]);
const injectableRegistry = new InjectableClassRegistry(reflectionHost);
const handler = new ComponentDecoratorHandler(
reflectionHost, evaluator, metaRegistry, metaReader, scopeRegistry, scopeRegistry,
new ResourceRegistry(),
/* isCore */ false, new NoopResourceLoader(), /* rootDirs */[''],
/* defaultPreserveWhitespaces */ false, /* i18nUseExternalIds */ true,
/* enableI18nLegacyMessageIdFormat */ false,
/* i18nNormalizeLineEndingsInICUs */ undefined, moduleResolver, cycleAnalyzer, refEmitter,
NOOP_DEFAULT_IMPORT_RECORDER, /* depTracker */ null, injectableRegistry,
/* annotateForClosureCompiler */ false);
const TestCmp = getDeclaration(program, _('/entry.ts'), 'TestCmp', isNamedClassDeclaration); const TestCmp = getDeclaration(program, _('/entry.ts'), 'TestCmp', isNamedClassDeclaration);
const detected = handler.detect(TestCmp, reflectionHost.getDecoratorsOfDeclaration(TestCmp)); const detected = handler.detect(TestCmp, reflectionHost.getDecoratorsOfDeclaration(TestCmp));
if (detected === undefined) { if (detected === undefined) {
@ -94,6 +101,104 @@ runInEachFileSystem(() => {
expect(diag.start).toBe(detected.metadata.args![0].getStart()); expect(diag.start).toBe(detected.metadata.args![0].getStart());
} }
}); });
it('should keep track of inline template', () => {
const template = '<span>inline</span>';
const {program, options, host} = makeProgram([
{
name: _('/node_modules/@angular/core/index.d.ts'),
contents: 'export const Component: any;',
},
{
name: _('/entry.ts'),
contents: `
import {Component} from '@angular/core';
@Component({
template: '${template}',
}) class TestCmp {}
`
},
]);
const {reflectionHost, handler} = setup(program, options, host);
const TestCmp = getDeclaration(program, _('/entry.ts'), 'TestCmp', isNamedClassDeclaration);
const detected = handler.detect(TestCmp, reflectionHost.getDecoratorsOfDeclaration(TestCmp));
if (detected === undefined) {
return fail('Failed to recognize @Component');
}
const {analysis} = handler.analyze(TestCmp, detected.metadata);
expect(analysis?.resources.template.path).toBeNull();
expect(analysis?.resources.template.expression.getText()).toEqual(`'${template}'`);
});
it('should keep track of external template', () => {
const templateUrl = '/myTemplate.ng.html';
const {program, options, host} = makeProgram([
{
name: _('/node_modules/@angular/core/index.d.ts'),
contents: 'export const Component: any;',
},
{
name: _(templateUrl),
contents: '<div>hello world</div>',
},
{
name: _('/entry.ts'),
contents: `
import {Component} from '@angular/core';
@Component({
templateUrl: '${templateUrl}',
}) class TestCmp {}
`
},
]);
const {reflectionHost, handler} = setup(program, options, host);
const TestCmp = getDeclaration(program, _('/entry.ts'), 'TestCmp', isNamedClassDeclaration);
const detected = handler.detect(TestCmp, reflectionHost.getDecoratorsOfDeclaration(TestCmp));
if (detected === undefined) {
return fail('Failed to recognize @Component');
}
const {analysis} = handler.analyze(TestCmp, detected.metadata);
expect(analysis?.resources.template.path).toContain(templateUrl);
expect(analysis?.resources.template.expression.getText()).toContain(`'${templateUrl}'`);
});
it('should keep track of internal and external styles', () => {
const {program, options, host} = makeProgram([
{
name: _('/node_modules/@angular/core/index.d.ts'),
contents: 'export const Component: any;',
},
{
name: _('/myStyle.css'),
contents: '<div>hello world</div>',
},
{
name: _('/entry.ts'),
contents: `
import {Component} from '@angular/core';
// These are ignored because we only attempt to store string literals
const ignoredStyleUrl = 'asdlfkj';
const ignoredStyle = '';
@Component({
template: '',
styleUrls: ['/myStyle.css', ignoredStyleUrl],
styles: ['a { color: red; }', 'b { color: blue; }', ignoredStyle, ...[ignoredStyle]],
}) class TestCmp {}
`
},
]);
const {reflectionHost, handler} = setup(program, options, host);
const TestCmp = getDeclaration(program, _('/entry.ts'), 'TestCmp', isNamedClassDeclaration);
const detected = handler.detect(TestCmp, reflectionHost.getDecoratorsOfDeclaration(TestCmp));
if (detected === undefined) {
return fail('Failed to recognize @Component');
}
const {analysis} = handler.analyze(TestCmp, detected.metadata);
expect(analysis?.resources.styles.size).toBe(3);
});
}); });
function ivyCode(code: ErrorCode): number { function ivyCode(code: ErrorCode): number {

View File

@ -261,6 +261,9 @@ export class NgCompiler {
const {resourceRegistry} = this.ensureAnalyzed(); const {resourceRegistry} = this.ensureAnalyzed();
const styles = resourceRegistry.getStyles(classDecl); const styles = resourceRegistry.getStyles(classDecl);
const template = resourceRegistry.getTemplate(classDecl); const template = resourceRegistry.getTemplate(classDecl);
if (template === null) {
return null;
}
return {styles, template}; return {styles, template};
} }

View File

@ -12,23 +12,24 @@ import {AbsoluteFsPath} from '../../file_system';
import {ClassDeclaration} from '../../reflection'; import {ClassDeclaration} from '../../reflection';
/** /**
* Represents an external resource for a component and contains the `AbsoluteFsPath` * Represents an resource for a component and contains the `AbsoluteFsPath`
* to the file which was resolved by evaluating the `ts.Expression` (generally, a relative or * to the file which was resolved by evaluating the `ts.Expression` (generally, a relative or
* absolute string path to the resource). * absolute string path to the resource).
*
* If the resource is inline, the `path` will be `null`.
*/ */
export interface Resource { export interface Resource {
path: AbsoluteFsPath; path: AbsoluteFsPath|null;
expression: ts.Expression; expression: ts.Expression;
} }
/** /**
* Represents the external resources of a component. * Represents the either inline or external resources of a component.
* *
* If the component uses an inline template, the template resource will be `null`. * A resource with a `path` of `null` is considered inline.
* If the component does not have external styles, the `styles` `Set` will be empty.
*/ */
export interface ComponentResources { export interface ComponentResources {
template: Resource|null; template: Resource;
styles: ReadonlySet<Resource>; styles: ReadonlySet<Resource>;
} }
@ -40,17 +41,17 @@ export interface ComponentResources {
* assistance. * assistance.
*/ */
export class ResourceRegistry { export class ResourceRegistry {
private templateToComponentsMap = new Map<AbsoluteFsPath, Set<ClassDeclaration>>(); private externalTemplateToComponentsMap = new Map<AbsoluteFsPath, Set<ClassDeclaration>>();
private componentToTemplateMap = new Map<ClassDeclaration, Resource>(); private componentToTemplateMap = new Map<ClassDeclaration, Resource>();
private componentToStylesMap = new Map<ClassDeclaration, Set<Resource>>(); private componentToStylesMap = new Map<ClassDeclaration, Set<Resource>>();
private styleToComponentsMap = new Map<AbsoluteFsPath, Set<ClassDeclaration>>(); private externalStyleToComponentsMap = new Map<AbsoluteFsPath, Set<ClassDeclaration>>();
getComponentsWithTemplate(template: AbsoluteFsPath): ReadonlySet<ClassDeclaration> { getComponentsWithTemplate(template: AbsoluteFsPath): ReadonlySet<ClassDeclaration> {
if (!this.templateToComponentsMap.has(template)) { if (!this.externalTemplateToComponentsMap.has(template)) {
return new Set(); return new Set();
} }
return this.templateToComponentsMap.get(template)!; return this.externalTemplateToComponentsMap.get(template)!;
} }
registerResources(resources: ComponentResources, component: ClassDeclaration) { registerResources(resources: ComponentResources, component: ClassDeclaration) {
@ -64,10 +65,12 @@ export class ResourceRegistry {
registerTemplate(templateResource: Resource, component: ClassDeclaration): void { registerTemplate(templateResource: Resource, component: ClassDeclaration): void {
const {path} = templateResource; const {path} = templateResource;
if (!this.templateToComponentsMap.has(path)) { if (path !== null) {
this.templateToComponentsMap.set(path, new Set()); if (!this.externalTemplateToComponentsMap.has(path)) {
this.externalTemplateToComponentsMap.set(path, new Set());
}
this.externalTemplateToComponentsMap.get(path)!.add(component);
} }
this.templateToComponentsMap.get(path)!.add(component);
this.componentToTemplateMap.set(component, templateResource); this.componentToTemplateMap.set(component, templateResource);
} }
@ -83,10 +86,12 @@ export class ResourceRegistry {
if (!this.componentToStylesMap.has(component)) { if (!this.componentToStylesMap.has(component)) {
this.componentToStylesMap.set(component, new Set()); this.componentToStylesMap.set(component, new Set());
} }
if (!this.styleToComponentsMap.has(path)) { if (path !== null) {
this.styleToComponentsMap.set(path, new Set()); if (!this.externalStyleToComponentsMap.has(path)) {
this.externalStyleToComponentsMap.set(path, new Set());
}
this.externalStyleToComponentsMap.get(path)!.add(component);
} }
this.styleToComponentsMap.get(path)!.add(component);
this.componentToStylesMap.get(component)!.add(styleResource); this.componentToStylesMap.get(component)!.add(styleResource);
} }
@ -98,10 +103,10 @@ export class ResourceRegistry {
} }
getComponentsWithStyle(styleUrl: AbsoluteFsPath): ReadonlySet<ClassDeclaration> { getComponentsWithStyle(styleUrl: AbsoluteFsPath): ReadonlySet<ClassDeclaration> {
if (!this.styleToComponentsMap.has(styleUrl)) { if (!this.externalStyleToComponentsMap.has(styleUrl)) {
return new Set(); return new Set();
} }
return this.styleToComponentsMap.get(styleUrl)!; return this.externalStyleToComponentsMap.get(styleUrl)!;
} }
} }