FIX: ListView infinite change detection

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

@@ -1,28 +1,27 @@
 import {
-    Component, 
-    DoCheck, 
-    ElementRef, 
+    Component,
+    DoCheck,
+    OnDestroy,
+    ElementRef,
     ViewContainerRef,
-    TemplateRef, 
-    ContentChild, 
+    TemplateRef,
+    ContentChild,
     EmbeddedViewRef,
-    HostListener, 
-    IterableDiffers, 
+    IterableDiffers,
     IterableDiffer,
     ChangeDetectorRef,
     EventEmitter,
     ViewChild,
     Output,
-    ChangeDetectionStrategy} from '@angular/core';
+    ChangeDetectionStrategy } from '@angular/core';
 import {isBlank} from '@angular/core/src/facade/lang';
 import {isListLikeIterable} from '@angular/core/src/facade/collection';
-import {Observable as RxObservable} from 'rxjs'
 import {ListView} from 'ui/list-view';
 import {View} from 'ui/core/view';
-import {NgView} from '../view-util';
 import {ObservableArray} from 'data/observable-array';
 import {LayoutBase} from 'ui/layouts/layout-base';
-import {rendererLog, rendererError} from "../trace";
+import {listViewLog} from "../trace";
+
 const NG_VIEW = "_ngViewRef";
 
 export class ListItemContext {
@@ -51,15 +50,15 @@ export interface SetupItemViewArgs {
     inputs: ['items'],
     changeDetection: ChangeDetectionStrategy.OnPush
 })
-export class ListViewComponent {
+export class ListViewComponent implements DoCheck, OnDestroy {
     public get nativeElement(): ListView {
         return this.listView;
     }
-    
+
     private listView: ListView;
     private _items: any;
     private _differ: IterableDiffer;
-    
+
     @ViewChild('loader', { read: ViewContainerRef }) loader: ViewContainerRef;
 
     @Output() public setupItemView: EventEmitter<SetupItemViewArgs> = new EventEmitter<SetupItemViewArgs>();
@@ -73,21 +72,24 @@ export class ListViewComponent {
             needDiffer = false;
         }
         if (needDiffer && !this._differ && isListLikeIterable(value)) {
-            this._differ = this._iterableDiffers.find(this._items).create(this._cdr, (index, item) => { return item;});
+            this._differ = this._iterableDiffers.find(this._items).create(this._cdr, (index, item) => { return item; });
         }
+
         this.listView.items = this._items;
     }
 
-    private timerId: number;
-    private doCheckDelay = 5;
-
     constructor(private _elementRef: ElementRef,
-                private _iterableDiffers: IterableDiffers,
-                private _cdr: ChangeDetectorRef) {
+        private _iterableDiffers: IterableDiffers,
+        private _cdr: ChangeDetectorRef) {
         this.listView = _elementRef.nativeElement;
+
+        this.listView.on("itemLoading", this.onItemLoading, this);
+    }
+
+    ngOnDestroy() {
+        this.listView.off("itemLoading", this.onItemLoading, this);
     }
 
-    @HostListener("itemLoading", ['$event'])
     public onItemLoading(args) {
         if (!this.itemTemplate) {
             return;
@@ -99,20 +101,22 @@ export class ListViewComponent {
         let viewRef: EmbeddedViewRef<ListItemContext>;
 
         if (args.view) {
-            rendererLog("ListView.onItemLoading: " + index + " - Reusing existing view");
+            listViewLog("onItemLoading: " + index + " - Reusing existing view");
             viewRef = args.view[NG_VIEW];
             // getting angular view from original element (in cases when ProxyViewContainer is used NativeScript internally wraps it in a StackLayout)
             if (!viewRef) {
                 viewRef = (args.view._subViews && args.view._subViews.length > 0) ? args.view._subViews[0][NG_VIEW] : undefined;
             }
         }
         else {
-            rendererLog("ListView.onItemLoading: " + index + " - Creating view from template");
+            listViewLog("onItemLoading: " + index + " - Creating view from template");
             viewRef = this.loader.createEmbeddedView(this.itemTemplate, new ListItemContext(), 0);
             args.view = getSingleViewFromViewRef(viewRef);
             args.view[NG_VIEW] = viewRef;
         }
         this.setupViewRef(viewRef, currentItem, index);
+
+        this.detectChangesOnChild(viewRef, index);
     }
 
     public setupViewRef(viewRef: EmbeddedViewRef<ListItemContext>, data: any, index: number): void {
@@ -126,49 +130,61 @@ export class ListViewComponent {
         context.even = (index % 2 == 0);
         context.odd = !context.even;
 
-        this.setupItemView.next({view: viewRef, data: data, index: index, context: context});
+        this.setupItemView.next({ view: viewRef, data: data, index: index, context: context });
     }
 
-    ngDoCheck() {
-        if (this.timerId) {
-            clearTimeout(this.timerId);
-        }
+    private detectChangesOnChild(viewRef: EmbeddedViewRef<ListItemContext>, index: number) {
+        // Manually detect changes in child view ref
+        // TODO: Is there a better way of getting viewRef's change detector
+        const childChangeDetector = <ChangeDetectorRef>(<any>viewRef);
 
-        this.timerId = setTimeout(() => {
-            clearTimeout(this.timerId);
-            if (this._differ) {
-                var changes = this._differ.diff(this._items);
-                if (changes) {
-                    this.listView.refresh();
-                }
+        listViewLog("Manually detect changes in child: " + index)
+        childChangeDetector.markForCheck();
+        childChangeDetector.detectChanges();
+    }
+
+    ngDoCheck() {
+        if (this._differ) {
+            listViewLog("ngDoCheck() - execute differ")
+            const changes = this._differ.diff(this._items);
+            if (changes) {
+                listViewLog("ngDoCheck() - refresh")
+                this.listView.refresh();
             }
-        }, this.doCheckDelay);
+        }
     }
 }
 
-function getSingleViewFromViewRef(viewRef: EmbeddedViewRef<any>): View {
-    var getSingleViewRecursive = (nodes: Array<any>, nestLevel: number) => {
-        var actualNodes = nodes.filter((n) => !!n && n.nodeName !== "#text");
 
-        if (actualNodes.length === 0) {
-            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);
+function getSingleViewRecursive(nodes: Array<any>, nestLevel: number) {
+    const actualNodes = nodes.filter((n) => !!n && n.nodeName !== "#text");
+
+    if (actualNodes.length === 0) {
+        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 {
-            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)
-            }
+            return getSingleViewRecursive(actualNodes[0].children, nestLevel + 1)
         }
     }
+}
 
+function getSingleViewFromViewRef(viewRef: EmbeddedViewRef<any>): View {
     return getSingleViewRecursive(viewRef.rootNodes, 0);
 }
+
+const changeDetectorMode = ["CheckOnce", "Checked", "CheckAlways", "Detached", "OnPush", "Default"];
+const changeDetectorStates = ["Never", "CheckedBefore", "Error"];
+function getChangeDetectorState(cdr: any) {
+    return "Mode: " + changeDetectorMode[parseInt(cdr.cdMode)] + " State: " + changeDetectorStates[parseInt(cdr.cdState)];
+}

nativescript-angular/trace.ts

@@ -2,6 +2,7 @@ import {write, categories, messageType} from "trace";
 
 export const rendererTraceCategory = "ns-renderer";
 export const routerTraceCategory = "ns-router";
+export const listViewTraceCategory = "ns-list-view";
 
 export function rendererLog(msg): void {
     write(msg, rendererTraceCategory);
@@ -18,3 +19,7 @@ export function routerLog(message: string): void {
 export function styleError(message: string): void {
     write(message, categories.Style, messageType.error);
 }
+
+export function listViewLog(message: string): void {
+    write(message, listViewTraceCategory);
+}

ng-sample/app/app.ts

@@ -8,17 +8,19 @@
 // this import should be first in order to load some required settings (like globals and reflect-metadata)
 import { nativeScriptBootstrap } from "nativescript-angular/application";
 import { NS_ROUTER_PROVIDERS } from "nativescript-angular/router";
-import { rendererTraceCategory, routerTraceCategory } from "nativescript-angular/trace";
+import { rendererTraceCategory, routerTraceCategory, listViewTraceCategory } from "nativescript-angular/trace";
 
 import trace = require("trace");
-trace.setCategories(routerTraceCategory);
+// trace.setCategories(rendererTraceCategory);
+// trace.setCategories(routerTraceCategory);
+trace.setCategories(listViewTraceCategory);
 trace.enable();
 
 import {RendererTest} from './examples/renderer-test';
 import {TabViewTest} from './examples/tab-view/tab-view-test';
 import {Benchmark} from './performance/benchmark';
 import {ListTest} from './examples/list/list-test';
-import {ListTestAsync} from "./examples/list/list-test-async";
+import {ListTestAsync, ListTestFilterAsync} from "./examples/list/list-test-async";
 import {ImageTest} from "./examples/image/image-test";
 import {NavigationTest} from "./examples/navigation/navigation-test";
 import {ActionBarTest} from "./examples/action-bar/action-bar-test";
@@ -30,13 +32,14 @@ import {LoginTest} from "./examples/navigation/login-test";
 //nativeScriptBootstrap(RendererTest);
 //nativeScriptBootstrap(TabViewTest);
 //nativeScriptBootstrap(Benchmark);
-//nativeScriptBootstrap(ListTest);
-//nativeScriptBootstrap(ListTestAsync);
+// nativeScriptBootstrap(ListTest);
+// nativeScriptBootstrap(ListTestAsync);
+nativeScriptBootstrap(ListTestFilterAsync);
 //nativeScriptBootstrap(ImageTest);
 //nativeScriptBootstrap(NavigationTest, [NS_ROUTER_PROVIDERS]);
 //nativeScriptBootstrap(ActionBarTest, [NS_ROUTER_PROVIDERS], { startPageActionBarHidden: false });
 //nativeScriptBootstrap(ActionBarTest, [NS_ROUTER_PROVIDERS]);
 //nativeScriptBootstrap(ModalTest);
 //nativeScriptBootstrap(PlatfromDirectivesTest);
 //nativeScriptBootstrap(RouterOutletTest, [NS_ROUTER_PROVIDERS]);
-nativeScriptBootstrap(LoginTest, [NS_ROUTER_PROVIDERS]);
+// nativeScriptBootstrap(LoginTest, [NS_ROUTER_PROVIDERS]);

ng-sample/app/examples/list/data.service.ts

@@ -0,0 +1,54 @@
+import { Injectable } from '@angular/core';
+import { BehaviorSubject } from "rxjs/BehaviorSubject";
+
+export class DataItem {
+    constructor(public id: number, public name: string) { }
+}
+
+@Injectable()
+export class DataService {
+    private _intervalId;
+    private _counter = 0;
+    private _currentItems: Array<DataItem>;
+
+    public items$: BehaviorSubject<Array<DataItem>>;
+
+    constructor() {
+        this._currentItems = [];
+        for (var i = 0; i < 3; i++) {
+            this.appendItem()
+        }
+
+        this.items$ = new BehaviorSubject(this._currentItems);
+    }
+
+    public startAsyncUpdates() {
+        if (this._intervalId) {
+            throw new Error("Updates are already started");
+        }
+
+        this._intervalId = setInterval(() => {
+            this.appendItem();
+            this.publishUpdates();
+        }, 200);
+
+    }
+
+    public stopAsyncUpdates() {
+        if (!this._intervalId) {
+            throw new Error("Updates are not started");
+        }
+
+        clearInterval(this._intervalId);
+        this._intervalId = undefined;
+    }
+
+    private publishUpdates() {
+        this.items$.next([...this._currentItems]);
+    }
+
+    private appendItem() {
+        this._currentItems.push(new DataItem(this._counter, "data item " + this._counter));
+        this._counter++;
+    }
+}