From 794dcb58d397360dc201c26c3a7713816062646b Mon Sep 17 00:00:00 2001 From: ayazhafiz Date: Sat, 21 Dec 2019 19:23:09 -0600 Subject: [PATCH] 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
{{}}
``` 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
{{$implicit}}
``` 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 --- .../language-service/src/typescript_host.ts | 2 +- .../language-service/test/completions_spec.ts | 7 ++++++ .../language-service/test/definitions_spec.ts | 25 +++++++++++++++++++ packages/language-service/test/hover_spec.ts | 16 ++++++++++++ 4 files changed, 49 insertions(+), 1 deletion(-) diff --git a/packages/language-service/src/typescript_host.ts b/packages/language-service/src/typescript_host.ts index 34cd7e94d7..a9576638b4 100644 --- a/packages/language-service/src/typescript_host.ts +++ b/packages/language-service/src/typescript_host.ts @@ -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(), diff --git a/packages/language-service/test/completions_spec.ts b/packages/language-service/test/completions_spec.ts index 8b27a382ed..ec8c6c32fe 100644 --- a/packages/language-service/test/completions_spec.ts +++ b/packages/language-service/test/completions_spec.ts @@ -734,6 +734,13 @@ describe('completions', () => { }); }); }); + + it('should not expand i18n templates', () => { + mockHost.override(TEST_TEMPLATE, `
{{~{cursor}}}
`); + const marker = mockHost.getLocationMarkerFor(TEST_TEMPLATE, 'cursor'); + const completions = ngLS.getCompletionsAt(TEST_TEMPLATE, marker.start); + expectContain(completions, CompletionKind.PROPERTY, ['title']); + }); }); function expectContain( diff --git a/packages/language-service/test/definitions_spec.ts b/packages/language-service/test/definitions_spec.ts index 8a7d731d7c..175a8ecfd3 100644 --- a/packages/language-service/test/definitions_spec.ts +++ b/packages/language-service/test/definitions_spec.ts @@ -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: '
{{«name»}}
' + }) + 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')); + }); }); diff --git a/packages/language-service/test/hover_spec.ts b/packages/language-service/test/hover_spec.ts index 2bc04211d5..d94a94ff63 100644 --- a/packages/language-service/test/hover_spec.ts +++ b/packages/language-service/test/hover_spec.ts @@ -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: '
{{«name»}}
' + }) + 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 {