From 680a296c6611bc32a8f994cea5d0b5a2d546d639 Mon Sep 17 00:00:00 2001 From: vakrilov Date: Mon, 6 Jun 2016 13:56:44 +0300 Subject: [PATCH 1/2] Detached loader tests --- tests/app/tests/detached-loader-tests.ts | 105 +++++++++++++++++++++++ 1 file changed, 105 insertions(+) create mode 100644 tests/app/tests/detached-loader-tests.ts diff --git a/tests/app/tests/detached-loader-tests.ts b/tests/app/tests/detached-loader-tests.ts new file mode 100644 index 000000000..0bfd03b24 --- /dev/null +++ b/tests/app/tests/detached-loader-tests.ts @@ -0,0 +1,105 @@ +//make sure you import mocha-config before @angular/core +import {assert} from "./test-config"; +import {Component, ElementRef, Renderer, AfterViewInit, OnInit, ViewChild, ChangeDetectionStrategy} from "@angular/core"; +import {ProxyViewContainer} from "ui/proxy-view-container"; +import {Red} from "color/known-colors"; +import {dumpView} from "./test-utils"; +import {TestApp} from "./test-app"; +import {LayoutBase} from "ui/layouts/layout-base"; +import {StackLayout} from "ui/layouts/stack-layout"; +import {DetachedLoader} from "nativescript-angular/common/detached-loader"; + +@Component({ + template: `` +}) +class TestComponent { } + + +class LoaderComponentBase { + @ViewChild(DetachedLoader) public loader: DetachedLoader; + + public ready: Promise; + private resolve; + constructor() { + this.ready = new Promise((reslove, reject) => { + this.resolve = reslove; + }) + } + ngAfterViewInit() { + this.resolve(this); + } +} + +@Component({ + selector: 'loader-component-on-push', + directives: [DetachedLoader], + template: ` + + + + ` +}) +export class LoaderComponent extends LoaderComponentBase { } + +@Component({ + selector: 'loader-component-on-push', + directives: [DetachedLoader], + changeDetection: ChangeDetectionStrategy.OnPush, + template: ` + + + + ` +}) +export class LoaderComponentOnPush extends LoaderComponentBase { } + +describe ('DetachedLoader', () => { + let testApp: TestApp = null; + + before(() => { + return TestApp.create().then((app) => { + testApp = app; + }) + }); + + after(() => { + testApp.dispose(); + }); + + afterEach(() => { + testApp.disposeComponents(); + }); + + it("creates component", (done) => { + testApp.loadComponent(LoaderComponent) + .then((componentRef) => { + // wait for the ngAfterViewInit + return (componentRef.instance).ready; + }) + .then((comp) => { + // load test component with loader + return comp.loader.loadComponent(TestComponent); + }) + .then((compRef) => { + done(); + }) + .catch(done); + }); + + + it("creates component when ChangeDetectionStrategy is OnPush", (done) => { + testApp.loadComponent(LoaderComponentOnPush) + .then((componentRef) => { + // wait for the ngAfterViewInit + return (componentRef.instance).ready; + }) + .then((comp) => { + // load test component with loader + return comp.loader.loadComponent(TestComponent); + }) + .then((compRef) => { + done(); + }) + .catch(done); + }); +}) From ba324cda81b8381e88c11bb0b640f67776ef5b19 Mon Sep 17 00:00:00 2001 From: vakrilov Date: Mon, 6 Jun 2016 14:34:07 +0300 Subject: [PATCH 2/2] DetachedLoader ChangeDetection fix --- .../common/detached-loader.ts | 31 ++++++++++++++++--- 1 file changed, 27 insertions(+), 4 deletions(-) diff --git a/nativescript-angular/common/detached-loader.ts b/nativescript-angular/common/detached-loader.ts index d0d059310..1734ceefc 100644 --- a/nativescript-angular/common/detached-loader.ts +++ b/nativescript-angular/common/detached-loader.ts @@ -1,4 +1,5 @@ -import {ComponentRef, ViewContainerRef, Component, Type, ViewChild, ComponentResolver} from '@angular/core'; +import {ComponentRef, ViewContainerRef, Component, Type, ViewChild, ComponentResolver, ChangeDetectorRef, Host} from '@angular/core'; +import trace = require("trace"); type AnyComponentRef = ComponentRef; interface PendingLoadEntry { @@ -6,6 +7,12 @@ interface PendingLoadEntry { resolveCallback: (AnyComponentRef) => void; } +export const CATEGORY = "detached-loader"; +function log(message: string) { + trace.write(message, CATEGORY); +} + + /** * Wrapper component used for loading components when navigating * It uses DetachedContainer as selector so that it is containerRef is not attached to the visual tree. @@ -20,10 +27,11 @@ export class DetachedLoader { private viewLoaded = false; private pendingLoads: PendingLoadEntry[] = []; - constructor(private compiler: ComponentResolver) { - } + constructor(private compiler: ComponentResolver, private changeDetector: ChangeDetectorRef) { } public ngAfterViewInit() { + log("DetachedLoader.ngAfterViewInit"); + this.viewLoaded = true; this.pendingLoads.forEach(loadEntry => { this.loadInLocation(loadEntry.componentType).then(loadedRef => { @@ -35,15 +43,30 @@ export class DetachedLoader { private loadInLocation(componentType: Type): Promise> { return this.compiler.resolveComponent(componentType).then((componentFactory) => { return this.containerRef.createComponent(componentFactory, this.containerRef.length, this.containerRef.parentInjector, null); - }); + }).then((compRef) => { + log("DetachedLoader.loadInLocation component loaded -> markForCheck"); + // Component is created, buit may not be checked if we are loading + // inside component with OnPush CD strategy. Mark us for check to be sure CD will reach us. + // We are inside a promise here so no need for setTimeout - CD should trigger after the promise. + this.changeDetector.markForCheck(); + return compRef; + }) } public loadComponent(componentType: Type): Promise> { + log("DetachedLoader.loadComponent viewLoaded: " + this.viewLoaded); + // Check if called before placeholder is initialized. // Delay load if so. if (this.viewLoaded) { return this.loadInLocation(componentType); } else { + // loadComponent called, but detached-loader is still not initialized. + // Mark it for change and trigger change detection to be sure it will be initialized, + // so that loading can conitionue. + log("DetachedLoader.loadComponent -> markForCheck(with setTimeout())") + setTimeout(() => this.changeDetector.markForCheck(), 0); + return new Promise((resolve, reject) => { this.pendingLoads.push({ componentType: componentType,