Skip to content

Commit

Permalink
fix(language-service): use the HtmlAst to get the span of HTML tag (#…
Browse files Browse the repository at this point in the history
…36371)

The HTML tag may include `-` (e.g. `app-root`), so use the `HtmlAst` to get the span.

PR Close #36371
  • Loading branch information
ivanwonder authored and kara committed Apr 7, 2020
1 parent 41a41e7 commit ffa4e11
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 7 deletions.
14 changes: 12 additions & 2 deletions packages/language-service/src/completions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down
10 changes: 5 additions & 5 deletions packages/language-service/test/completions_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -670,18 +670,18 @@ describe('completions', () => {
@Component({
selector: 'foo-component',
template: \`
<di~{div}></div>
<test-comp~{test-comp}></test-comp>
\`,
})
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', () => {
Expand Down

0 comments on commit ffa4e11

Please sign in to comment.