From 81195a238b91cefc35867b8cc0b115141761d577 Mon Sep 17 00:00:00 2001 From: ivanwonder Date: Wed, 1 Apr 2020 21:29:01 +0800 Subject: [PATCH] fix(language-service): use the `HtmlAst` to get the span of HTML tag (#36371) The HTML tag may include `-` (e.g. `app-root`), so use the `HtmlAst` to get the span. PR Close #36371 --- packages/language-service/src/completions.ts | 14 ++++++++++++-- packages/language-service/test/completions_spec.ts | 10 +++++----- 2 files changed, 17 insertions(+), 7 deletions(-) diff --git a/packages/language-service/src/completions.ts b/packages/language-service/src/completions.ts index e5afe71617..80f7b363be 100644 --- a/packages/language-service/src/completions.ts +++ b/packages/language-service/src/completions.ts @@ -55,12 +55,22 @@ function isIdentifierPart(code: number) { * Gets the span of word in a template that surrounds `position`. If there is no word around * `position`, nothing is returned. */ -function getBoundedWordSpan(templateInfo: AstResult, position: number): ts.TextSpan|undefined { +function getBoundedWordSpan( + templateInfo: AstResult, position: number, ast: HtmlAst|undefined): ts.TextSpan|undefined { const {template} = templateInfo; const templateSrc = template.source; if (!templateSrc) return; + if (ast instanceof Element) { + // The HTML tag may include `-` (e.g. `app-root`), + // so use the HtmlAst to get the span before ayazhafiz refactor the code. + return { + start: templateInfo.template.span.start + ast.startSourceSpan!.start.offset + 1, + length: ast.name.length + }; + } + // TODO(ayazhafiz): A solution based on word expansion will always be expensive compared to one // based on ASTs. Whatever penalty we incur is probably manageable for small-length (i.e. the // majority of) identifiers, but the current solution involes a number of branchings and we can't @@ -185,7 +195,7 @@ export function getTemplateCompletions( null); } - const replacementSpan = getBoundedWordSpan(templateInfo, position); + const replacementSpan = getBoundedWordSpan(templateInfo, position, mostSpecific); return result.map(entry => { return { ...entry, diff --git a/packages/language-service/test/completions_spec.ts b/packages/language-service/test/completions_spec.ts index 0bf3e4238e..96f6c52c13 100644 --- a/packages/language-service/test/completions_spec.ts +++ b/packages/language-service/test/completions_spec.ts @@ -670,18 +670,18 @@ describe('completions', () => { @Component({ selector: 'foo-component', template: \` - + \`, }) export class FooComponent {} `); - const location = mockHost.getLocationMarkerFor(fileName, 'div'); + const location = mockHost.getLocationMarkerFor(fileName, 'test-comp'); const completions = ngLS.getCompletionsAtPosition(fileName, location.start)!; expect(completions).toBeDefined(); - const completion = completions.entries.find(entry => entry.name === 'div')!; + const completion = completions.entries.find(entry => entry.name === 'test-comp')!; expect(completion).toBeDefined(); - expect(completion.kind).toBe('html element'); - expect(completion.replacementSpan).toEqual({start: location.start - 2, length: 2}); + expect(completion.kind).toBe('component'); + expect(completion.replacementSpan).toEqual({start: location.start - 9, length: 9}); }); it('should work for bindings', () => {