fix(language-service): do not use an i18n parser for templates (#34531)

The compiler's `I18NHtmlParser` may expand template nodes that have
internationalization metadata attached to them; for instance,

```html
<div i18n="@@i18n-el">{{}}</div>
```

gets expanded to an AST with the i18n metadata extracted and text filled
in as necessary; to the language service, the template above, as read in
the AST, now looks something like

```html
<div>{{$implicit}}</div>
```

This is undesirable for the language service because we want to preserve
the original form of the source template source code, and have
information about the original values of the template. The language
service also does not need to use an i18n parser -- we don't generate
any template output.

To fix this turns out to be as easy as moving to using a raw
`HtmlParser`.

---

A note on the testing strategy: as mentioned above, we don't need to use
an i18n parser, but we don't **not** need to use one if the parser
does not heavily modify the template AST. For this reason, the tests
target the functionality of not modifying a template with i18n metadata
rather than testing that the language service does not use an i18n parser.

---

Closes https://github.com/angular/vscode-ng-language-service/issues/272

PR Close #34531
This commit is contained in:
ayazhafiz 2019-12-21 19:23:09 -06:00 committed by atscott
parent b3441eea4c
commit 794dcb58d3
4 changed files with 49 additions and 1 deletions

View File

@ -501,7 +501,7 @@ export class TypeScriptServiceHost implements LanguageServiceHost {
if (!data) {
return;
}
const htmlParser = new I18NHtmlParser(new HtmlParser());
const htmlParser = new HtmlParser();
const expressionParser = new Parser(new Lexer());
const parser = new TemplateParser(
new CompilerConfig(), this.reflector, expressionParser, new DomElementSchemaRegistry(),

View File

@ -734,6 +734,13 @@ describe('completions', () => {
});
});
});
it('should not expand i18n templates', () => {
mockHost.override(TEST_TEMPLATE, `<div i18n="@@el">{{~{cursor}}}</div>`);
const marker = mockHost.getLocationMarkerFor(TEST_TEMPLATE, 'cursor');
const completions = ngLS.getCompletionsAt(TEST_TEMPLATE, marker.start);
expectContain(completions, CompletionKind.PROPERTY, ['title']);
});
});
function expectContain(

View File

@ -306,4 +306,29 @@ describe('definitions', () => {
expect(def.fileName).toBe('/app/test.css');
expect(def.textSpan).toEqual({start: 0, length: 0});
});
it('should not expand i18n templates', () => {
const fileName = mockHost.addCode(`
@Component({
template: '<div i18n="@@el">{{«name»}}</div>'
})
export class MyComponent {
«ᐱnameᐱ: string;»
}`);
const marker = mockHost.getReferenceMarkerFor(fileName, 'name');
const result = ngService.getDefinitionAt(fileName, marker.start);
expect(result).toBeDefined();
const {textSpan, definitions} = result !;
expect(textSpan).toEqual(marker);
expect(definitions).toBeDefined();
expect(definitions !.length).toBe(1);
const def = definitions ![0];
expect(def.fileName).toBe(fileName);
expect(def.name).toBe('name');
expect(def.kind).toBe('property');
expect(def.textSpan).toEqual(mockHost.getDefinitionMarkerFor(fileName, 'name'));
});
});

View File

@ -223,6 +223,22 @@ describe('hover', () => {
const documentation = toText(quickInfo !.documentation);
expect(documentation).toBe('This Component provides the `test-comp` selector.');
});
it('should not expand i18n templates', () => {
const fileName = mockHost.addCode(`
@Component({
template: '<div i18n="@@el">{{«name»}}</div>'
})
export class MyComponent {
name: string;
}`);
const marker = mockHost.getReferenceMarkerFor(fileName, 'name');
const quickInfo = ngLS.getHoverAt(fileName, marker.start);
expect(quickInfo).toBeTruthy();
const {textSpan, displayParts} = quickInfo !;
expect(textSpan).toEqual(marker);
expect(toText(displayParts)).toBe('(property) MyComponent.name: string');
});
});
function toText(displayParts?: ts.SymbolDisplayPart[]): string {