fix(directive-parser) refactor + correct handling of whitespace

src/parsers/directive.parser.ts

@@ -1,87 +1,108 @@
+import {
+	parseTemplate,
+	TmplAstNode as Node,
+	TmplAstElement as Element,
+	TmplAstText as Text,
+	TmplAstTemplate as Template
+} from '@angular/compiler';
+
 import { ParserInterface } from './parser.interface';
 import { TranslationCollection } from '../utils/translation.collection';
 import { isPathAngularComponent, extractComponentInlineTemplate } from '../utils/utils';
 
-import { parseTemplate, TmplAstNode, TmplAstElement, TmplAstTextAttribute } from '@angular/compiler';
+const TRANSLATE_ATTR_NAME = 'translate';
+type ElementLike = Element | Template;
 
 export class DirectiveParser implements ParserInterface {
 	public extract(source: string, filePath: string): TranslationCollection | null {
+		let collection: TranslationCollection = new TranslationCollection();
+
 		if (filePath && isPathAngularComponent(filePath)) {
 			source = extractComponentInlineTemplate(source);
 		}
+		const nodes: Node[] = this.parseTemplate(source, filePath);
+		const elements: ElementLike[] = this.getElementsWithTranslateAttribute(nodes);
 
-		let collection: TranslationCollection = new TranslationCollection();
+		elements.forEach((element) => {
-
+			const attr = this.getAttribute(element, TRANSLATE_ATTR_NAME);
-		const nodes: TmplAstNode[] = this.parseTemplate(source, filePath);
+			if (attr) {
-		this.getTranslatableElements(nodes).forEach((element) => {
+				collection = collection.add(attr);
-			const key = this.getElementTranslateAttrValue(element) || this.getElementContent(element);
+				return;
-			collection = collection.add(key);
+			}
+			const textNodes = this.getTextNodes(element);
+			textNodes.forEach((textNode: Text) => {
+				collection = collection.add(textNode.value.trim());
+			});
 		});
-
 		return collection;
 	}
 
-	protected getTranslatableElements(nodes: TmplAstNode[]): TmplAstElement[] {
+	/**
-		return nodes
+	 * Find all ElementLike nodes with a translate attribute
-			.filter((element) => this.isElement(element))
+	 * @param nodes
-			.reduce((result: TmplAstElement[], element: TmplAstElement) => {
+	 */
-				return result.concat(this.findChildrenElements(element));
+	protected getElementsWithTranslateAttribute(nodes: Node[]): ElementLike[] {
-			}, [])
+		let elements: ElementLike[] = [];
-			.filter((element) => this.isTranslatable(element));
+		nodes.filter(this.isElementLike)
-	}
+			.forEach((element) => {
-
+				if (this.hasAttribute(element, TRANSLATE_ATTR_NAME)) {
-	protected findChildrenElements(node: TmplAstNode): TmplAstElement[] {
+					elements = [...elements, element];
-		if (!this.isElement(node)) {
-			return [];
-		}
-
-		// If element has translate attribute all its content is translatable
-		// so we don't need to traverse any deeper
-		if (this.isTranslatable(node)) {
-			return [node];
-		}
-
-		return node.children.reduce(
-			(result: TmplAstElement[], childNode: TmplAstNode) => {
-				if (this.isElement(childNode)) {
-					const children = this.findChildrenElements(childNode);
-					return result.concat(children);
 				}
-				return result;
+				const childElements = this.getElementsWithTranslateAttribute(element.children);
-			},
+				if (childElements.length) {
-			[node]
+					elements = [...elements, ...childElements];
-		);
+				}
+			});
+		return elements;
 	}
 
-	protected parseTemplate(template: string, path: string): TmplAstNode[] {
+	/**
+	 * Get direct child nodes of type Text
+	 * @param element
+	 */
+	protected getTextNodes(element: ElementLike): Text[] {
+		return element.children.filter(this.isText);
+	}
+
+	/**
+	 * Check if attribute is present on element
+	 * @param element
+	 */
+	protected hasAttribute(element: ElementLike, name: string): boolean {
+		return this.getAttribute(element, name) !== undefined;
+	}
+
+	/**
+	 * Get attribute value if present on element
+	 * @param element
+	 */
+	protected getAttribute(element: ElementLike, name: string): string | undefined {
+		return element.attributes.find((attribute) => attribute.name === name)?.value;
+	}
+
+	/**
+	 * Check if node type is ElementLike
+	 * @param node
+	 */
+	protected isElementLike(node: Node): node is ElementLike {
+		return node instanceof Element || node instanceof Template;
+	}
+
+	/**
+	 * Check if node type is Text
+	 * @param node
+	 */
+	protected isText(node: Node): node is Text {
+		return node instanceof Text;
+	}
+
+	/**
+	 * Parse a template into nodes
+	 * @param template
+	 * @param path
+	 */
+	protected parseTemplate(template: string, path: string): Node[] {
 		return parseTemplate(template, path).nodes;
 	}
 
-	protected isElement(node: any): node is TmplAstElement {
-		return node?.attributes && node?.children;
-	}
-
-	protected isTranslatable(node: TmplAstNode): boolean {
-		if (this.isElement(node) && node.attributes.some((attribute) => attribute.name === 'translate')) {
-			return true;
-		}
-		return false;
-	}
-
-	protected getElementTranslateAttrValue(element: TmplAstElement): string {
-		const attr: TmplAstTextAttribute = element.attributes.find((attribute) => attribute.name === 'translate');
-		return attr?.value ?? '';
-	}
-
-	protected getElementContent(element: TmplAstElement): string {
-		const content = element.sourceSpan.start.file.content;
-		const start = element.startSourceSpan.end.offset;
-		const end = element.endSourceSpan.start.offset;
-		const val = content.substring(start, end);
-		return this.cleanKey(val);
-	}
-
-	protected cleanKey(val: string): string {
-		return val.replace(/\r?\n|\r|\t/g, '').trim();
-	}
 }

tests/parsers/directive.parser.spec.ts

@@ -12,6 +12,25 @@ describe('DirectiveParser', () => {
 		parser = new DirectiveParser();
 	});
 
+	it('should extract keys keeping proper whitespace', () => {
+		const contents = `
+			<div translate>
+				Wubba
+				Lubba
+				Dub Dub
+			</div>
+		`;
+		const keys = parser.extract(contents, templateFilename).keys();
+		expect(keys).to.deep.equal(['Wubba Lubba Dub Dub']);
+	});
+
+	// Source: // https://github.com/ngx-translate/core/blob/7241c863b2eead26e082cd0b7ee15bac3f9336fc/projects/ngx-translate/core/tests/translate.directive.spec.ts#L93
+	it('should extract keys the same way TranslateDirective is using them', () => {
+		const contents = `<div #withOtherElements translate>TEST1 <span>Hey</span> TEST2</div>`;
+		const keys = parser.extract(contents, templateFilename).keys();
+		expect(keys).to.deep.equal(['TEST1', 'TEST2']);
+	});
+
 	it('should not choke when no html is present in template', () => {
 		const contents = 'Hello World';
 		const keys = parser.extract(contents, templateFilename).keys();
@@ -33,13 +52,13 @@ describe('DirectiveParser', () => {
 	it('should not process children when translate attribute is present', () => {
 		const contents = `<div translate>Hello <strong translate>World</strong></div>`;
 		const keys = parser.extract(contents, templateFilename).keys();
-		expect(keys).to.deep.equal(['Hello <strong translate>World</strong>']);
+		expect(keys).to.deep.equal(['Hello', 'World']);
 	});
 
 	it('should not exclude html tags in children', () => {
 		const contents = `<div translate>Hello <strong>World</strong></div>`;
 		const keys = parser.extract(contents, templateFilename).keys();
-		expect(keys).to.deep.equal(['Hello <strong>World</strong>']);
+		expect(keys).to.deep.equal(['Hello']);
 	});
 
 	it('should extract and parse inline template', () => {
@@ -95,7 +114,7 @@ describe('DirectiveParser', () => {
 	it('should extract contents without line breaks', () => {
 		const contents = `
 			<p translate>
-				Please leave a message for your client letting them know why you 
+				Please leave a message for your client letting them know why you
 				rejected the field and what they need to do to fix it.
 			</p>
 		`;
@@ -107,8 +126,8 @@ describe('DirectiveParser', () => {
 
 	it('should extract contents without indent spaces', () => {
 		const contents = `
-			<div *ngIf="!isLoading && studentsToGrid && studentsToGrid.length == 0" class="no-students" mt-rtl translate>There 
+			<div *ngIf="!isLoading && studentsToGrid && studentsToGrid.length == 0" class="no-students" mt-rtl translate>There
-				are currently no students in this class. The good news is, adding students is really easy! Just use the options 
+				are currently no students in this class. The good news is, adding students is really easy! Just use the options
 				at the top.
 			</div>
 		`;
@@ -118,23 +137,17 @@ describe('DirectiveParser', () => {
 		]);
 	});
 
-	it('should extract contents without indent spaces', () => {
-		const contents = `<button mat-button (click)="search()" translate>client.search.searchBtn</button>`;
-		const keys = parser.extract(contents, templateFilename).keys();
-		expect(keys).to.deep.equal(['client.search.searchBtn']);
-	});
-
 	it('should extract contents without indent spaces and trim leading/trailing whitespace', () => {
 		const contents = `
 			<div translate>
-				this is an example 
+				this is an example
-				of a long label 
+				of a long label
 			</div>
 
 			<div>
 				<p translate>
-					this is an example 
+					this is an example
-					of a long label 
+					of a long label
 				</p>
 			</div>
 		`;