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
This commit is contained in:
ivanwonder 2020-04-01 21:29:01 +08:00 committed by Kara Erickson
parent 96a3de6364
commit 81195a238b
2 changed files with 17 additions and 7 deletions

View File

@ -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 * Gets the span of word in a template that surrounds `position`. If there is no word around
* `position`, nothing is returned. * `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 {template} = templateInfo;
const templateSrc = template.source; const templateSrc = template.source;
if (!templateSrc) return; 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 // 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 // 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 // majority of) identifiers, but the current solution involes a number of branchings and we can't
@ -185,7 +195,7 @@ export function getTemplateCompletions(
null); null);
} }
const replacementSpan = getBoundedWordSpan(templateInfo, position); const replacementSpan = getBoundedWordSpan(templateInfo, position, mostSpecific);
return result.map(entry => { return result.map(entry => {
return { return {
...entry, ...entry,

View File

@ -670,18 +670,18 @@ describe('completions', () => {
@Component({ @Component({
selector: 'foo-component', selector: 'foo-component',
template: \` template: \`
<di~{div}></div> <test-comp~{test-comp}></test-comp>
\`, \`,
}) })
export class FooComponent {} export class FooComponent {}
`); `);
const location = mockHost.getLocationMarkerFor(fileName, 'div'); const location = mockHost.getLocationMarkerFor(fileName, 'test-comp');
const completions = ngLS.getCompletionsAtPosition(fileName, location.start)!; const completions = ngLS.getCompletionsAtPosition(fileName, location.start)!;
expect(completions).toBeDefined(); 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).toBeDefined();
expect(completion.kind).toBe('html element'); expect(completion.kind).toBe('component');
expect(completion.replacementSpan).toEqual({start: location.start - 2, length: 2}); expect(completion.replacementSpan).toEqual({start: location.start - 9, length: 9});
}); });
it('should work for bindings', () => { it('should work for bindings', () => {