Skip to content

Commit a05eb13

Browse files
atscottAndrewKushnir
atscott
authored and
AndrewKushnir
committed
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
1 parent 1d4fc94 commit a05eb13

File tree

4 files changed

+80
-25
lines changed

4 files changed

+80
-25
lines changed

‎packages/language-service/ivy/attribute_completions.ts‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -180,7 +180,8 @@ export type AttributeCompletion = DomAttributeCompletion|DomPropertyCompletion|
180180
*/
181181
export function buildAttributeCompletionTable(
182182
component: ts.ClassDeclaration, element: TmplAstElement|TmplAstTemplate,
183-
checker: TemplateTypeChecker): Map<string, AttributeCompletion> {
183+
checker: TemplateTypeChecker,
184+
includeDomSchemaAttributes: boolean): Map<string, AttributeCompletion> {
184185
const table = new Map<string, AttributeCompletion>();
185186

186187
// Use the `ElementSymbol` or `TemplateSymbol` to iterate over directives present on the node, and
@@ -332,7 +333,7 @@ export function buildAttributeCompletionTable(
332333
}
333334

334335
// Finally, add any DOM attributes not already covered by inputs.
335-
if (element instanceof TmplAstElement) {
336+
if (element instanceof TmplAstElement && includeDomSchemaAttributes) {
336337
for (const {attribute, property} of checker.getPotentialDomBindings(element.name)) {
337338
const isAlsoProperty = attribute === property;
338339
if (!table.has(attribute)) {

‎packages/language-service/ivy/completions.ts‎

Lines changed: 16 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ export class CompletionBuilder<N extends TmplAstNode|AST> {
5757
constructor(
5858
private readonly tsLS: ts.LanguageService, private readonly compiler: NgCompiler,
5959
private readonly component: ts.ClassDeclaration, private readonly node: N,
60-
private readonly targetDetails: TemplateTarget) {}
60+
private readonly targetDetails: TemplateTarget, private readonly inlineTemplate: boolean) {}
6161

6262
/**
6363
* Analogue for `ts.LanguageService.getCompletionsAtPosition`.
@@ -370,14 +370,18 @@ export class CompletionBuilder<N extends TmplAstNode|AST> {
370370

371371
const replacementSpan: ts.TextSpan = {start, length};
372372

373-
const entries: ts.CompletionEntry[] =
374-
Array.from(templateTypeChecker.getPotentialElementTags(this.component))
375-
.map(([tag, directive]) => ({
376-
kind: tagCompletionKind(directive),
377-
name: tag,
378-
sortText: tag,
379-
replacementSpan,
380-
}));
373+
let potentialTags = Array.from(templateTypeChecker.getPotentialElementTags(this.component));
374+
if (!this.inlineTemplate) {
375+
// If we are in an external template, don't provide non-Angular tags (directive === null)
376+
// because we expect other extensions (i.e. Emmet) to provide those for HTML files.
377+
potentialTags = potentialTags.filter(([_, directive]) => directive !== null);
378+
}
379+
const entries: ts.CompletionEntry[] = potentialTags.map(([tag, directive]) => ({
380+
kind: tagCompletionKind(directive),
381+
name: tag,
382+
sortText: tag,
383+
replacementSpan,
384+
}));
381385

382386
return {
383387
entries,
@@ -458,7 +462,7 @@ export class CompletionBuilder<N extends TmplAstNode|AST> {
458462
}
459463

460464
const attrTable = buildAttributeCompletionTable(
461-
this.component, element, this.compiler.getTemplateTypeChecker());
465+
this.component, element, this.compiler.getTemplateTypeChecker(), this.inlineTemplate);
462466

463467
let entries: ts.CompletionEntry[] = [];
464468

@@ -532,7 +536,7 @@ export class CompletionBuilder<N extends TmplAstNode|AST> {
532536
}
533537

534538
const attrTable = buildAttributeCompletionTable(
535-
this.component, element, this.compiler.getTemplateTypeChecker());
539+
this.component, element, this.compiler.getTemplateTypeChecker(), this.inlineTemplate);
536540

537541
if (!attrTable.has(name)) {
538542
return undefined;
@@ -599,7 +603,7 @@ export class CompletionBuilder<N extends TmplAstNode|AST> {
599603
}
600604

601605
const attrTable = buildAttributeCompletionTable(
602-
this.component, element, this.compiler.getTemplateTypeChecker());
606+
this.component, element, this.compiler.getTemplateTypeChecker(), this.inlineTemplate);
603607

604608
if (!attrTable.has(name)) {
605609
return undefined;

‎packages/language-service/ivy/language_service.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -199,7 +199,8 @@ export class LanguageService {
199199
positionDetails.context.nodes[0] :
200200
positionDetails.context.node;
201201
return new CompletionBuilder(
202-
this.tsLS, compiler, templateInfo.component, node, positionDetails);
202+
this.tsLS, compiler, templateInfo.component, node, positionDetails,
203+
isTypeScriptFile(fileName));
203204
}
204205

205206
getCompletionsAtPosition(

‎packages/language-service/ivy/test/completions_spec.ts‎

Lines changed: 59 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -299,10 +299,19 @@ describe('completions', () => {
299299
});
300300

301301
describe('element tag scope', () => {
302-
it('should return DOM completions', () => {
302+
it('should not return DOM completions for external template', () => {
303303
const {templateFile} = setup(`<div>`, '');
304304
templateFile.moveCursorToText('<div¦>');
305305
const completions = templateFile.getCompletionsAtPosition();
306+
expectDoesNotContain(
307+
completions, unsafeCastDisplayInfoKindToScriptElementKind(DisplayInfoKind.ELEMENT),
308+
['div', 'span']);
309+
});
310+
311+
it('should return DOM completions', () => {
312+
const {appFile} = setupInlineTemplate(`<div>`, '');
313+
appFile.moveCursorToText('<div¦>');
314+
const completions = appFile.getCompletionsAtPosition();
306315
expectContain(
307316
completions, unsafeCastDisplayInfoKindToScriptElementKind(DisplayInfoKind.ELEMENT),
308317
['div', 'span']);
@@ -422,11 +431,19 @@ describe('completions', () => {
422431

423432
describe('element attribute scope', () => {
424433
describe('dom completions', () => {
425-
it('should return completions for a new element attribute', () => {
434+
it('should not return completions dom completions in external template', () => {
426435
const {templateFile} = setup(`<input >`, '');
427436
templateFile.moveCursorToText('<input ¦>');
428437

429438
const completions = templateFile.getCompletionsAtPosition();
439+
expect(completions?.entries.length).toBe(0);
440+
});
441+
442+
it('should return completions for a new element attribute', () => {
443+
const {appFile} = setupInlineTemplate(`<input >`, '');
444+
appFile.moveCursorToText('<input ¦>');
445+
446+
const completions = appFile.getCompletionsAtPosition();
430447
expectContain(
431448
completions, unsafeCastDisplayInfoKindToScriptElementKind(DisplayInfoKind.ATTRIBUTE),
432449
['value']);
@@ -436,24 +453,24 @@ describe('completions', () => {
436453
});
437454

438455
it('should return completions for a partial attribute', () => {
439-
const {templateFile} = setup(`<input val>`, '');
440-
templateFile.moveCursorToText('<input val¦>');
456+
const {appFile} = setupInlineTemplate(`<input val>`, '');
457+
appFile.moveCursorToText('<input val¦>');
441458

442-
const completions = templateFile.getCompletionsAtPosition();
459+
const completions = appFile.getCompletionsAtPosition();
443460
expectContain(
444461
completions, unsafeCastDisplayInfoKindToScriptElementKind(DisplayInfoKind.ATTRIBUTE),
445462
['value']);
446463
expectContain(
447464
completions, unsafeCastDisplayInfoKindToScriptElementKind(DisplayInfoKind.PROPERTY),
448465
['[value]']);
449-
expectReplacementText(completions, templateFile.contents, 'val');
466+
expectReplacementText(completions, appFile.contents, 'val');
450467
});
451468

452469
it('should return completions for a partial property binding', () => {
453-
const {templateFile} = setup(`<input [val]>`, '');
454-
templateFile.moveCursorToText('[val¦]');
470+
const {appFile} = setupInlineTemplate(`<input [val]>`, '');
471+
appFile.moveCursorToText('[val¦]');
455472

456-
const completions = templateFile.getCompletionsAtPosition();
473+
const completions = appFile.getCompletionsAtPosition();
457474
expectDoesNotContain(
458475
completions, unsafeCastDisplayInfoKindToScriptElementKind(DisplayInfoKind.ATTRIBUTE),
459476
['value']);
@@ -463,7 +480,7 @@ describe('completions', () => {
463480
expectContain(
464481
completions, unsafeCastDisplayInfoKindToScriptElementKind(DisplayInfoKind.PROPERTY),
465482
['value']);
466-
expectReplacementText(completions, templateFile.contents, 'val');
483+
expectReplacementText(completions, appFile.contents, 'val');
467484
});
468485
});
469486

@@ -779,3 +796,35 @@ function setup(
779796
});
780797
return {templateFile: project.openFile('test.html')};
781798
}
799+
800+
function setupInlineTemplate(
801+
template: string, classContents: string, otherDeclarations: {[name: string]: string} = {}): {
802+
appFile: OpenBuffer,
803+
} {
804+
const decls = ['AppCmp', ...Object.keys(otherDeclarations)];
805+
806+
const otherDirectiveClassDecls = Object.values(otherDeclarations).join('\n\n');
807+
808+
const env = LanguageServiceTestEnv.setup();
809+
const project = env.addProject('test', {
810+
'test.ts': `
811+
import {Component, Directive, NgModule, Pipe, TemplateRef} from '@angular/core';
812+
813+
@Component({
814+
template: '${template}',
815+
selector: 'app-cmp',
816+
})
817+
export class AppCmp {
818+
${classContents}
819+
}
820+
821+
${otherDirectiveClassDecls}
822+
823+
@NgModule({
824+
declarations: [${decls.join(', ')}],
825+
})
826+
export class AppModule {}
827+
`,
828+
});
829+
return {appFile: project.openFile('test.ts')};
830+
}

0 commit comments

Comments
 (0)