fix(language-service): Only provide dom completions for inline templates (#41078)

We currently provide completions for DOM elements in the schema as well
as attributes when we are in the context of an external template.
However, these completions are already provided by other extensions for
HTML contexts (like Emmet). To avoid duplication of results, this commit
updates the language service to exclude DOM completions for external
templates. They are still provided for inline templates because those
are not handled by the HTML language extensions.

PR Close #41078
This commit is contained in:
Andrew Scott 2021-03-04 08:46:50 -08:00 committed by Andrew Kushnir
parent 38524c4d29
commit 45216ccc0d
4 changed files with 80 additions and 25 deletions

View File

@ -180,7 +180,8 @@ export type AttributeCompletion = DomAttributeCompletion|DomPropertyCompletion|
*/
export function buildAttributeCompletionTable(
component: ts.ClassDeclaration, element: TmplAstElement|TmplAstTemplate,
checker: TemplateTypeChecker): Map<string, AttributeCompletion> {
checker: TemplateTypeChecker,
includeDomSchemaAttributes: boolean): Map<string, AttributeCompletion> {
const table = new Map<string, AttributeCompletion>();
// Use the `ElementSymbol` or `TemplateSymbol` to iterate over directives present on the node, and
@ -332,7 +333,7 @@ export function buildAttributeCompletionTable(
}
// Finally, add any DOM attributes not already covered by inputs.
if (element instanceof TmplAstElement) {
if (element instanceof TmplAstElement && includeDomSchemaAttributes) {
for (const {attribute, property} of checker.getPotentialDomBindings(element.name)) {
const isAlsoProperty = attribute === property;
if (!table.has(attribute)) {

View File

@ -57,7 +57,7 @@ export class CompletionBuilder<N extends TmplAstNode|AST> {
constructor(
private readonly tsLS: ts.LanguageService, private readonly compiler: NgCompiler,
private readonly component: ts.ClassDeclaration, private readonly node: N,
private readonly targetDetails: TemplateTarget) {}
private readonly targetDetails: TemplateTarget, private readonly inlineTemplate: boolean) {}
/**
* Analogue for `ts.LanguageService.getCompletionsAtPosition`.
@ -370,14 +370,18 @@ export class CompletionBuilder<N extends TmplAstNode|AST> {
const replacementSpan: ts.TextSpan = {start, length};
const entries: ts.CompletionEntry[] =
Array.from(templateTypeChecker.getPotentialElementTags(this.component))
.map(([tag, directive]) => ({
kind: tagCompletionKind(directive),
name: tag,
sortText: tag,
replacementSpan,
}));
let potentialTags = Array.from(templateTypeChecker.getPotentialElementTags(this.component));
if (!this.inlineTemplate) {
// If we are in an external template, don't provide non-Angular tags (directive === null)
// because we expect other extensions (i.e. Emmet) to provide those for HTML files.
potentialTags = potentialTags.filter(([_, directive]) => directive !== null);
}
const entries: ts.CompletionEntry[] = potentialTags.map(([tag, directive]) => ({
kind: tagCompletionKind(directive),
name: tag,
sortText: tag,
replacementSpan,
}));
return {
entries,
@ -458,7 +462,7 @@ export class CompletionBuilder<N extends TmplAstNode|AST> {
}
const attrTable = buildAttributeCompletionTable(
this.component, element, this.compiler.getTemplateTypeChecker());
this.component, element, this.compiler.getTemplateTypeChecker(), this.inlineTemplate);
let entries: ts.CompletionEntry[] = [];
@ -532,7 +536,7 @@ export class CompletionBuilder<N extends TmplAstNode|AST> {
}
const attrTable = buildAttributeCompletionTable(
this.component, element, this.compiler.getTemplateTypeChecker());
this.component, element, this.compiler.getTemplateTypeChecker(), this.inlineTemplate);
if (!attrTable.has(name)) {
return undefined;
@ -599,7 +603,7 @@ export class CompletionBuilder<N extends TmplAstNode|AST> {
}
const attrTable = buildAttributeCompletionTable(
this.component, element, this.compiler.getTemplateTypeChecker());
this.component, element, this.compiler.getTemplateTypeChecker(), this.inlineTemplate);
if (!attrTable.has(name)) {
return undefined;

View File

@ -201,7 +201,8 @@ export class LanguageService {
positionDetails.context.nodes[0] :
positionDetails.context.node;
return new CompletionBuilder(
this.tsLS, compiler, templateInfo.component, node, positionDetails);
this.tsLS, compiler, templateInfo.component, node, positionDetails,
isTypeScriptFile(fileName));
}
getCompletionsAtPosition(

View File

@ -299,10 +299,19 @@ describe('completions', () => {
});
describe('element tag scope', () => {
it('should return DOM completions', () => {
it('should not return DOM completions for external template', () => {
const {templateFile} = setup(`<div>`, '');
templateFile.moveCursorToText('<div¦>');
const completions = templateFile.getCompletionsAtPosition();
expectDoesNotContain(
completions, unsafeCastDisplayInfoKindToScriptElementKind(DisplayInfoKind.ELEMENT),
['div', 'span']);
});
it('should return DOM completions', () => {
const {appFile} = setupInlineTemplate(`<div>`, '');
appFile.moveCursorToText('<div¦>');
const completions = appFile.getCompletionsAtPosition();
expectContain(
completions, unsafeCastDisplayInfoKindToScriptElementKind(DisplayInfoKind.ELEMENT),
['div', 'span']);
@ -422,11 +431,19 @@ describe('completions', () => {
describe('element attribute scope', () => {
describe('dom completions', () => {
it('should return completions for a new element attribute', () => {
it('should not return completions dom completions in external template', () => {
const {templateFile} = setup(`<input >`, '');
templateFile.moveCursorToText('<input ¦>');
const completions = templateFile.getCompletionsAtPosition();
expect(completions?.entries.length).toBe(0);
});
it('should return completions for a new element attribute', () => {
const {appFile} = setupInlineTemplate(`<input >`, '');
appFile.moveCursorToText('<input ¦>');
const completions = appFile.getCompletionsAtPosition();
expectContain(
completions, unsafeCastDisplayInfoKindToScriptElementKind(DisplayInfoKind.ATTRIBUTE),
['value']);
@ -436,24 +453,24 @@ describe('completions', () => {
});
it('should return completions for a partial attribute', () => {
const {templateFile} = setup(`<input val>`, '');
templateFile.moveCursorToText('<input val¦>');
const {appFile} = setupInlineTemplate(`<input val>`, '');
appFile.moveCursorToText('<input val¦>');
const completions = templateFile.getCompletionsAtPosition();
const completions = appFile.getCompletionsAtPosition();
expectContain(
completions, unsafeCastDisplayInfoKindToScriptElementKind(DisplayInfoKind.ATTRIBUTE),
['value']);
expectContain(
completions, unsafeCastDisplayInfoKindToScriptElementKind(DisplayInfoKind.PROPERTY),
['[value]']);
expectReplacementText(completions, templateFile.contents, 'val');
expectReplacementText(completions, appFile.contents, 'val');
});
it('should return completions for a partial property binding', () => {
const {templateFile} = setup(`<input [val]>`, '');
templateFile.moveCursorToText('[val¦]');
const {appFile} = setupInlineTemplate(`<input [val]>`, '');
appFile.moveCursorToText('[val¦]');
const completions = templateFile.getCompletionsAtPosition();
const completions = appFile.getCompletionsAtPosition();
expectDoesNotContain(
completions, unsafeCastDisplayInfoKindToScriptElementKind(DisplayInfoKind.ATTRIBUTE),
['value']);
@ -463,7 +480,7 @@ describe('completions', () => {
expectContain(
completions, unsafeCastDisplayInfoKindToScriptElementKind(DisplayInfoKind.PROPERTY),
['value']);
expectReplacementText(completions, templateFile.contents, 'val');
expectReplacementText(completions, appFile.contents, 'val');
});
});
@ -779,3 +796,35 @@ function setup(
});
return {templateFile: project.openFile('test.html')};
}
function setupInlineTemplate(
template: string, classContents: string, otherDeclarations: {[name: string]: string} = {}): {
appFile: OpenBuffer,
} {
const decls = ['AppCmp', ...Object.keys(otherDeclarations)];
const otherDirectiveClassDecls = Object.values(otherDeclarations).join('\n\n');
const env = LanguageServiceTestEnv.setup();
const project = env.addProject('test', {
'test.ts': `
import {Component, Directive, NgModule, Pipe, TemplateRef} from '@angular/core';
@Component({
template: '${template}',
selector: 'app-cmp',
})
export class AppCmp {
${classContents}
}
${otherDirectiveClassDecls}
@NgModule({
declarations: [${decls.join(', ')}],
})
export class AppModule {}
`,
});
return {appFile: project.openFile('test.ts')};
}