fix(renderer): set templateParent to comment and text nodes (#785)

* fix(renderer): set templateParent for newly create DetachedText nodes

DetachedText is not inserted in the UI components tree, so we need to
add it's parent manually.

* fix(renderer): respect templateParent in parentNode() if one is set

fixes #777, fixes #787

* test: add unit test for NgIfElse and NgIfThenElse

* chore: target 'next' tag of tns-core-modules

* fix(renderer): stop attaching comments to visual tree

* refactor(renderer): create DetachedElements instead for comments and
text nodes

* refactor: move NgView, NgElement and similar to separate module

nativescript-angular/animations/animation-driver.ts

@@ -1,6 +1,6 @@
 import { AnimationPlayer } from "@angular/animations";
-import { NgView } from "../element-registry";
 
+import { NgView } from "../element-types";
 import { NativeScriptAnimationPlayer } from "./animation-player";
 import { Keyframe } from "./utils";
 

nativescript-angular/animations/animation-engine.ts

@@ -1,7 +1,7 @@
 import { ɵDomAnimationEngine as DomAnimationEngine } from "@angular/animations/browser";
 import { AnimationEvent, AnimationPlayer } from "@angular/animations";
 
-import { NgView } from "../element-registry";
+import { NgView } from "../element-types";
 import {
     copyArray,
     cssClasses,

nativescript-angular/animations/animation-player.ts

@@ -4,7 +4,7 @@ import {
     KeyframeAnimationInfo,
 } from "tns-core-modules/ui/animation/keyframe-animation";
 
-import { NgView } from "../element-registry";
+import { NgView } from "../element-types";
 import { Keyframe, getAnimationCurve, parseAnimationKeyframe } from "./utils";
 
 export class NativeScriptAnimationPlayer implements AnimationPlayer {

nativescript-angular/animations/dom-utils.ts

@@ -7,7 +7,7 @@ import {
 } from "@angular/animations";
 import { unsetValue } from "tns-core-modules/ui/core/view";
 
-import { NgView } from "../element-registry";
+import { NgView } from "../element-types";
 
 // overriden to use the default 'unsetValue'
 // instead of empty string ''

nativescript-angular/directives/action-bar.ts

@@ -1,13 +1,15 @@
 import { Directive, Component, ElementRef, Optional, OnDestroy } from "@angular/core";
 import { ActionItem, ActionBar, NavigationButton } from "tns-core-modules/ui/action-bar";
-import { isBlank } from "../lang-facade";
 import { Page } from "tns-core-modules/ui/page";
 import { View } from "tns-core-modules/ui/core/view";
-import { registerElement, ViewClassMeta, NgView } from "../element-registry";
+
+import { isBlank } from "../lang-facade";
+import { registerElement } from "../element-registry";
+import { ViewClassMeta, NgView } from "../element-types";
 
 const actionBarMeta: ViewClassMeta = {
     skipAddToDom: true,
-    insertChild: (parent: NgView, child: NgView, atIndex: number) => {
+    insertChild: (parent: NgView, child: NgView) => {
         const bar = <ActionBar>(<any>parent);
         const childView = <any>child;
 
@@ -17,8 +19,6 @@ const actionBarMeta: ViewClassMeta = {
         } else if (child instanceof ActionItem) {
             bar.actionItems.addItem(childView);
             childView.parent = bar;
-        } else if (child.nodeName === "#comment") {
-            bar._addView(childView, atIndex);
         } else if (child instanceof View) {
             bar.titleView = childView;
         }

nativescript-angular/directives/list-view-comp.ts

@@ -1,29 +1,31 @@
 import {
+    AfterContentInit,
+    ChangeDetectionStrategy,
+    ChangeDetectorRef,
     Component,
+    ContentChild,
     Directive,
-    Input,
     DoCheck,
-    OnDestroy,
-    AfterContentInit,
     ElementRef,
-    ViewContainerRef,
-    TemplateRef,
-    ContentChild,
     EmbeddedViewRef,
-    IterableDiffers,
-    IterableDiffer,
-    ChangeDetectorRef,
     EventEmitter,
-    ViewChild,
-    Output,
     Host,
-    ChangeDetectionStrategy
+    Input,
+    IterableDiffer,
+    IterableDiffers,
+    OnDestroy,
+    Output,
+    TemplateRef,
+    ViewChild,
+    ViewContainerRef,
 } from "@angular/core";
-import { isListLikeIterable } from "../collection-facade";
 import { ListView, ItemEventData } from "tns-core-modules/ui/list-view";
 import { View, KeyedTemplate } from "tns-core-modules/ui/core/view";
 import { ObservableArray } from "tns-core-modules/data/observable-array";
 import { LayoutBase } from "tns-core-modules/ui/layouts/layout-base";
+
+import { CommentNode } from "../element-types";
+import { isListLikeIterable } from "../collection-facade";
 import { listViewLog, listViewError } from "../trace";
 
 const NG_VIEW = "_ngViewRef";
@@ -212,23 +214,27 @@ export class ListViewComponent implements DoCheck, OnDestroy, AfterContentInit {
 }
 
 function getSingleViewRecursive(nodes: Array<any>, nestLevel: number): View {
-    const actualNodes = nodes.filter((n) => !!n && n.nodeName !== "#text");
+    const actualNodes = nodes.filter(node => !(node instanceof CommentNode));
 
     if (actualNodes.length === 0) {
-        throw new Error("No suitable views found in list template! Nesting level: " + nestLevel);
+        throw new Error(`No suitable views found in list template! ` +
+            `Nesting level: ${nestLevel}`);
     } else if (actualNodes.length > 1) {
-        throw new Error("More than one view found in list template! Nesting level: " + nestLevel);
-    } else {
-        if (actualNodes[0]) {
-            let parentLayout = actualNodes[0].parent;
-            if (parentLayout instanceof LayoutBase) {
-                parentLayout.removeChild(actualNodes[0]);
-            }
-            return actualNodes[0];
-        } else {
-            return getSingleViewRecursive(actualNodes[0].children, nestLevel + 1);
-        }
+        throw new Error(`More than one view found in list template!` +
+            `Nesting level: ${nestLevel}`);
+    }
+
+    const rootLayout = actualNodes[0];
+    if (!rootLayout) {
+        return getSingleViewRecursive(rootLayout.children, nestLevel + 1);
+    }
+
+    let parentLayout = rootLayout.parent;
+    if (parentLayout instanceof LayoutBase) {
+        parentLayout.removeChild(rootLayout);
     }
+
+    return rootLayout;
 }
 
 export interface ComponentView {

nativescript-angular/directives/tab-view.ts

@@ -1,5 +1,15 @@
-import { ElementRef, Directive, Input, TemplateRef, ViewContainerRef, OnInit, AfterViewInit } from "@angular/core";
+import {
+    AfterViewInit,
+    Directive,
+    ElementRef,
+    Input,
+    OnInit,
+    TemplateRef,
+    ViewContainerRef,
+} from "@angular/core";
 import { TabView, TabViewItem } from "tns-core-modules/ui/tab-view";
+
+import { CommentNode } from "../element-types";
 import { convertToInt } from "../common/utils";
 import { rendererLog } from "../trace";
 import { isBlank } from "../lang-facade";
@@ -94,9 +104,9 @@ export class TabViewItemDirective implements OnInit {
         }
 
         const viewRef = this.viewContainer.createEmbeddedView(this.templateRef);
-        // Filter out text nodes, etc
-        const realViews = viewRef.rootNodes.filter((node) =>
-                            node.nodeName && node.nodeName !== "#text");
+        // Filter out text nodes and comments
+        const realViews = viewRef.rootNodes.filter(node =>
+                            !(node instanceof CommentNode));
 
         if (realViews.length > 0) {
             this.item.view = realViews[0];

nativescript-angular/element-registry.ts

@@ -1,31 +1,10 @@
-import { View } from "tns-core-modules/ui/core/view";
+import { ViewClass, ViewClassMeta } from "./element-types";
 
 export type ViewResolver = () => ViewClass;
-export type NgView = View & ViewExtensions;
-export interface ViewClassMeta {
-    skipAddToDom?: boolean;
-    insertChild?: (parent: NgView, child: NgView, atIndex: number) => void;
-    removeChild?: (parent: NgView, child: NgView) => void;
-}
-
-export interface ViewExtensions {
-    nodeType: number;
-    nodeName: string;
-    templateParent: NgView;
-    ngCssClasses: Map<string, boolean>;
-    meta: ViewClassMeta;
-}
-
-export interface ViewClass {
-    new (): View;
-}
-
-const defaultViewMeta: ViewClassMeta = {
-    skipAddToDom: false,
-};
 
 const elementMap = new Map<string, { resolver: ViewResolver, meta?: ViewClassMeta }>();
 const camelCaseSplit = /([a-z0-9])([A-Z])/g;
+const defaultViewMeta: ViewClassMeta = { skipAddToDom: false };
 
 export function registerElement(
     elementName: string,
@@ -48,6 +27,7 @@ export function getViewClass(elementName: string): ViewClass {
     if (!entry) {
         throw new TypeError(`No known component for element ${elementName}.`);
     }
+
     try {
         return entry.resolver();
     } catch (e) {
@@ -56,12 +36,8 @@ export function getViewClass(elementName: string): ViewClass {
 }
 
 export function getViewMeta(nodeName: string): ViewClassMeta {
-    let meta = defaultViewMeta;
     const entry = elementMap.get(nodeName) || elementMap.get(nodeName.toLowerCase());
-    if (entry && entry.meta) {
-        meta = entry.meta;
-    }
-    return meta;
+    return (entry && entry.meta) || defaultViewMeta;
 }
 
 export function isKnownView(elementName: string): boolean {
@@ -110,10 +86,3 @@ registerElement("Span", () => require("tns-core-modules/text/span").Span);
 
 registerElement("DetachedContainer", () => require("tns-core-modules/ui/proxy-view-container").ProxyViewContainer,
     { skipAddToDom: true });
-
-registerElement("DetachedText", () => require("ui/placeholder").Placeholder,
-    { skipAddToDom: true });
-
-registerElement("Comment", () => require("ui/placeholder").Placeholder,
-    { skipAddToDom: false });
-

nativescript-angular/element-types.ts

@@ -0,0 +1,32 @@
+import { View } from "tns-core-modules/ui/core/view";
+
+export type NgView = (View & ViewExtensions);
+export type NgElement = NgView | CommentNode;
+
+export interface ViewExtensions {
+    nodeType: number;
+    nodeName: string;
+    templateParent: NgView;
+    ngCssClasses: Map<string, boolean>;
+    meta: ViewClassMeta;
+}
+
+export interface ViewClass {
+    new (): View;
+}
+
+// used for creating comments and text nodes in the renderer
+export class CommentNode {
+    meta: { skipAddToDom: true };
+    templateParent: NgView;
+}
+
+export interface ViewClassMeta {
+    skipAddToDom?: boolean;
+    insertChild?: (parent: NgView, child: NgView, atIndex: number) => void;
+    removeChild?: (parent: NgView, child: NgView) => void;
+}
+
+export function isDetachedElement(element): boolean {
+    return (element && element.meta && element.meta.skipAddToDom);
+}

nativescript-angular/index.ts

@@ -1,4 +1,4 @@
-import "application";
+import "tns-core-modules/application";
 
 export * from "./platform-common";
 export * from "./platform";
@@ -15,13 +15,18 @@ export * from "./modal-dialog";
 export * from "./renderer";
 export * from "./view-util";
 export * from "./resource-loader";
+
 export {
     ViewResolver,
-    ViewClass,
-    ViewClassMeta,
     registerElement,
     getViewClass,
     getViewMeta,
     isKnownView,
 } from "./element-registry";
+
+export {
+    ViewClass,
+    ViewClassMeta,
+} from "./element-types";
+
 export * from "./value-accessors/base-value-accessor";

nativescript-angular/renderer.ts

@@ -12,7 +12,7 @@ import { topmost } from "tns-core-modules/ui/frame";
 import { APP_ROOT_VIEW, DEVICE, getRootPage } from "./platform-providers";
 import { isBlank } from "./lang-facade";
 import { ViewUtil } from "./view-util";
-import { NgView } from "./element-registry";
+import { NgView, CommentNode } from "./element-types";
 import { rendererLog as traceLog } from "./trace";
 
 // CONTENT_ATTR not exported from NativeScript_renderer - we need it for styles application.
@@ -43,6 +43,7 @@ export class NativeScriptRendererFactory implements RendererFactory2 {
         if (!rootView) {
             rootView = getRootPage() || topmost().currentPage;
         }
+
         rootView.nodeName = "NONE";
         this.rootNgView = rootView;
     }
@@ -83,26 +84,17 @@ export class NativeScriptRenderer extends Renderer2 {
 
     appendChild(parent: any, newChild: NgView): void {
         traceLog(`NativeScriptRenderer.appendChild child: ${newChild} parent: ${parent}`);
-
-        if (parent) {
-            this.viewUtil.insertChild(parent, newChild);
-        }
+        this.viewUtil.insertChild(parent, newChild);
     }
 
     insertBefore(parent: NgView, newChild: NgView, refChildIndex: number): void {
         traceLog(`NativeScriptRenderer.insertBefore child: ${newChild} parent: ${parent}`);
-
-        if (parent) {
-            this.viewUtil.insertChild(parent, newChild, refChildIndex);
-        }
+        this.viewUtil.insertChild(parent, newChild, refChildIndex);
     }
 
     removeChild(parent: any, oldChild: NgView): void {
         traceLog(`NativeScriptRenderer.removeChild child: ${oldChild} parent: ${parent}`);
-
-        if (parent) {
-            this.viewUtil.removeChild(parent, oldChild);
-        }
+        this.viewUtil.removeChild(parent, oldChild);
     }
 
     selectRootElement(selector: string): NgView {
@@ -111,15 +103,15 @@ export class NativeScriptRenderer extends Renderer2 {
     }
 
     parentNode(node: NgView): any {
-        return node.parent;
+        return node.parent || node.templateParent;
     }
 
     nextSibling(node: NgView): number {
         traceLog(`NativeScriptRenderer.nextSibling ${node}`);
         return this.viewUtil.nextSiblingIndex(node);
     }
 
-    createComment(_value: any) {
+    createComment(_value: any): CommentNode {
         traceLog(`NativeScriptRenderer.createComment ${_value}`);
         return this.viewUtil.createComment();
     }
@@ -129,7 +121,7 @@ export class NativeScriptRenderer extends Renderer2 {
         return this.viewUtil.createView(name);
     }
 
-    createText(_value: string) {
+    createText(_value: string): CommentNode {
         traceLog(`NativeScriptRenderer.createText ${_value}`);
         return this.viewUtil.createText();
     }