fix(router-outlet): attach entering view before first change detection (#18821)

Author: manucorporat

angular/src/directives/navigation/ion-router-outlet.ts

@@ -205,7 +205,6 @@ export class IonRouterOutlet implements OnDestroy, OnInit {
       // Calling `markForCheck` to make sure we will run the change detection when the
       // `RouterOutlet` is inside a `ChangeDetectionStrategy.OnPush` component.
       enteringView = this.stackCtrl.createView(this.activated, activatedRoute);
-      enteringView.ref.changeDetectorRef.detectChanges();
 
       // Store references to the proxy by component
       this.proxyMap.set(cmpRef.instance, activatedRouteProxy);

angular/src/directives/navigation/stack-controller.ts

@@ -31,7 +31,7 @@ export class StackController {
   createView(ref: ComponentRef<any>, activatedRoute: ActivatedRoute): RouteView {
     const url = getUrl(this.router, activatedRoute);
     const element = (ref && ref.location && ref.location.nativeElement) as HTMLElement;
-    const unlistenEvents = bindLifecycleEvents(ref.instance, element);
+    const unlistenEvents = bindLifecycleEvents(this.zone, ref.instance, element);
     return {
       id: this.nextId++,
       stackId: computeStackId(this.tabsPrefix, url),
@@ -90,16 +90,28 @@ export class StackController {
       }
     }
 
+    const reused = this.views.includes(enteringView);
     const views = this.insertView(enteringView, direction);
-    return this.wait(() => {
-      return this.transition(enteringView, leavingView, animation, this.canGoBack(1), false)
-        .then(() => cleanupAsync(enteringView, views, viewsSnapshot, this.location))
-        .then(() => ({
-          enteringView,
-          direction,
-          animation,
-          tabSwitch
-        }));
+
+    // Trigger change detection before transition starts
+    // This will call ngOnInit() the first time too, just after the view
+    // was attached to the dom, but BEFORE the transition starts
+    if (!reused) {
+      enteringView.ref.changeDetectorRef.detectChanges();
+    }
+
+    // Wait until previous transitions finish
+    return this.zone.runOutsideAngular(() => {
+      return this.wait(() => {
+        return this.transition(enteringView, leavingView, animation, this.canGoBack(1), false)
+          .then(() => cleanupAsync(enteringView, views, viewsSnapshot, this.location))
+          .then(() => ({
+            enteringView,
+            direction,
+            animation,
+            tabSwitch
+          }));
+      });
     });
   }
 
@@ -199,11 +211,11 @@ export class StackController {
     if (enteringView) {
       enteringView.ref.changeDetectorRef.reattach();
     }
-    // TODO: disconnect leaving page from change detection to
+    // disconnect leaving page from change detection to
     // reduce jank during the page transition
-    // if (leavingView) {
-    //   leavingView.ref.changeDetectorRef.detach();
-    // }
+    if (leavingView) {
+      leavingView.ref.changeDetectorRef.detach();
+    }
     const enteringEl = enteringView ? enteringView.element : undefined;
     const leavingEl = leavingView ? leavingView.element : undefined;
     const containerEl = this.containerEl;
@@ -213,13 +225,13 @@ export class StackController {
         containerEl.appendChild(enteringEl);
       }
 
-      return this.zone.runOutsideAngular(() => containerEl.commit(enteringEl, leavingEl, {
+      return containerEl.commit(enteringEl, leavingEl, {
         deepWait: true,
         duration: direction === undefined ? 0 : undefined,
         direction,
         showGoBack,
         progressAnimation
-      }));
+      });
     }
     return Promise.resolve(false);
   }

angular/src/providers/angular-delegate.ts

@@ -34,10 +34,10 @@ export class AngularFrameworkDelegate implements FrameworkDelegate {
   ) {}
 
   attachViewToDom(container: any, component: any, params?: any, cssClasses?: string[]): Promise<any> {
-    return new Promise(resolve => {
-      this.zone.run(() => {
+    return this.zone.run(() => {
+      return new Promise(resolve => {
         const el = attachView(
-          this.resolver, this.injector, this.location, this.appRef,
+          this.zone, this.resolver, this.injector, this.location, this.appRef,
           this.elRefMap, this.elEventsMap,
           container, component, params, cssClasses
         );
@@ -47,8 +47,8 @@ export class AngularFrameworkDelegate implements FrameworkDelegate {
   }
 
   removeViewFromDom(_container: any, component: any): Promise<void> {
-    return new Promise(resolve => {
-      this.zone.run(() => {
+    return this.zone.run(() => {
+      return new Promise(resolve => {
         const componentRef = this.elRefMap.get(component);
         if (componentRef) {
           componentRef.destroy();
@@ -66,6 +66,7 @@ export class AngularFrameworkDelegate implements FrameworkDelegate {
 }
 
 export function attachView(
+  zone: NgZone,
   resolver: ComponentFactoryResolver,
   injector: Injector,
   location: ViewContainerRef | undefined,
@@ -93,7 +94,7 @@ export function attachView(
       hostElement.classList.add(clazz);
     }
   }
-  const unbindEvents = bindLifecycleEvents(instance, hostElement);
+  const unbindEvents = bindLifecycleEvents(zone, instance, hostElement);
   container.appendChild(hostElement);
 
   if (!location) {
@@ -113,15 +114,17 @@ const LIFECYCLES = [
   LIFECYCLE_WILL_UNLOAD
 ];
 
-export function bindLifecycleEvents(instance: any, element: HTMLElement) {
-  const unregisters = LIFECYCLES
-    .filter(eventName => typeof instance[eventName] === 'function')
-    .map(eventName => {
-      const handler = (ev: any) => instance[eventName](ev.detail);
-      element.addEventListener(eventName, handler);
-      return () => element.removeEventListener(eventName, handler);
-    });
-  return () => unregisters.forEach(fn => fn());
+export function bindLifecycleEvents(zone: NgZone, instance: any, element: HTMLElement) {
+  return zone.run(() => {
+    const unregisters = LIFECYCLES
+      .filter(eventName => typeof instance[eventName] === 'function')
+      .map(eventName => {
+        const handler = (ev: any) => instance[eventName](ev.detail);
+        element.addEventListener(eventName, handler);
+        return () => element.removeEventListener(eventName, handler);
+      });
+    return () => unregisters.forEach(fn => fn());
+  });
 }
 
 const NavParamsToken = new InjectionToken<any>('NavParamsToken');

angular/test/test-app/e2e/src/router-link.e2e-spec.ts

@@ -1,5 +1,5 @@
 import { browser, element, by, protractor } from 'protractor';
-import { waitTime, testStack, testLifeCycle, handleErrorMessages } from './utils';
+import { waitTime, testStack, testLifeCycle, handleErrorMessages, getText } from './utils';
 
 const EC = protractor.ExpectedConditions;
 
@@ -23,12 +23,13 @@ describe('router-link params and fragments', () => {
 
   it('should return to a page with preserved query param and fragment', async () => {
     await browser.get('/router-link?ionic:_testing=true');
+    await waitTime(30);
     await element(by.css('#queryParamsFragment')).click();
-    await waitTime(200);
+    await waitTime(400);
     await element(by.css('#goToPage3')).click();
 
     browser.wait(EC.urlContains('router-link-page3'), 5000);
-    await waitTime(200);
+    await waitTime(400);
 
     await element(by.css('#goBackFromPage3')).click();
 
@@ -38,6 +39,7 @@ describe('router-link params and fragments', () => {
 
   it('should preserve query param and fragment with defaultHref string', async () => {
     await browser.get('/router-link-page3?ionic:_testing=true');
+    await waitTime(30);
 
     await element(by.css('#goBackFromPage3')).click();
 
@@ -71,18 +73,6 @@ describe('router-link', () => {
     it('should go forward with ion-button[routerLink]', async () => {
       await element(by.css('#routerLink')).click();
       await testForward();
-
-      // test go back
-      await element(by.css('ion-back-button')).click();
-      await waitTime(500);
-
-      await testStack('ion-router-outlet', ['app-router-link']);
-      await testLifeCycle('app-router-link', {
-        ionViewWillEnter: 2,
-        ionViewDidEnter: 2,
-        ionViewWillLeave: 1,
-        ionViewDidLeave: 1,
-      });
     });
 
     it('should go forward with a[routerLink]', async () => {
@@ -140,18 +130,23 @@ describe('router-link', () => {
 async function testForward() {
   await waitTime(2500);
   await testStack('ion-router-outlet', ['app-router-link', 'app-router-link-page']);
-  await testLifeCycle('app-router-link', {
-    ionViewWillEnter: 1,
-    ionViewDidEnter: 1,
-    ionViewWillLeave: 1,
-    ionViewDidLeave: 1,
-  });
   await testLifeCycle('app-router-link-page', {
     ionViewWillEnter: 1,
     ionViewDidEnter: 1,
     ionViewWillLeave: 0,
     ionViewDidLeave: 0,
   });
+  expect(await getText(`app-router-link-page #canGoBack`)).toEqual('true');
+
+  await browser.navigate().back();
+  await waitTime(100);
+  await testStack('ion-router-outlet', ['app-router-link']);
+  await testLifeCycle('app-router-link', {
+    ionViewWillEnter: 2,
+    ionViewDidEnter: 2,
+    ionViewWillLeave: 1,
+    ionViewDidLeave: 1,
+  });
 }
 
 async function testRoot() {
@@ -163,6 +158,8 @@ async function testRoot() {
     ionViewWillLeave: 0,
     ionViewDidLeave: 0,
   });
+  expect(await getText(`app-router-link-page #canGoBack`)).toEqual('false');
+
   await browser.navigate().back();
   await waitTime(100);
   await testStack('ion-router-outlet', ['app-router-link']);
@@ -183,4 +180,15 @@ async function testBack() {
     ionViewWillLeave: 0,
     ionViewDidLeave: 0,
   });
+  expect(await getText(`app-router-link-page #canGoBack`)).toEqual('false');
+
+  await browser.navigate().back();
+  await waitTime(100);
+  await testStack('ion-router-outlet', ['app-router-link']);
+  await testLifeCycle('app-router-link', {
+    ionViewWillEnter: 1,
+    ionViewDidEnter: 1,
+    ionViewWillLeave: 0,
+    ionViewDidLeave: 0,
+  });
 }

angular/test/test-app/e2e/src/utils.ts

@@ -48,11 +48,19 @@ export function handleErrorMessages() {
 
 export async function testLifeCycle(selector: string, expected: LifeCycleCount) {
   await waitTime(50);
-  expect(await getText(`${selector} #ngOnInit`)).toEqual('1');
-  expect(await getText(`${selector} #ionViewWillEnter`)).toEqual(expected.ionViewWillEnter.toString());
-  expect(await getText(`${selector} #ionViewDidEnter`)).toEqual(expected.ionViewDidEnter.toString());
-  expect(await getText(`${selector} #ionViewWillLeave`)).toEqual(expected.ionViewWillLeave.toString());
-  expect(await getText(`${selector} #ionViewDidLeave`)).toEqual(expected.ionViewDidLeave.toString());
+  const results = await Promise.all([
+    getText(`${selector} #ngOnInit`),
+    getText(`${selector} #ionViewWillEnter`),
+    getText(`${selector} #ionViewDidEnter`),
+    getText(`${selector} #ionViewWillLeave`),
+    getText(`${selector} #ionViewDidLeave`),
+  ]);
+
+  expect(results[0]).toEqual('1');
+  expect(results[1]).toEqual(expected.ionViewWillEnter.toString());
+  expect(results[2]).toEqual(expected.ionViewDidEnter.toString());
+  expect(results[3]).toEqual(expected.ionViewWillLeave.toString());
+  expect(results[4]).toEqual(expected.ionViewDidLeave.toString());
 }
 
 export async function testStack(selector: string, expected: string[]) {

angular/test/test-app/src/app/alert/alert.component.html

@@ -1,10 +1,10 @@
 <ion-header>
   <ion-toolbar>
     <ion-title>
-      Modal test
+      Alert test
     </ion-title>
   </ion-toolbar>
 </ion-header>
 <ion-content padding>
-  <ion-button (click)="openAlert()" id="action-button">Open Alert</ion-button>
+  <p>Change Detections: <span id="counter">{{counter()}}</span></p>
 </ion-content>

angular/test/test-app/src/app/alert/alert.component.ts

@@ -8,10 +8,17 @@ import { NavComponent } from '../nav/nav.component';
 })
 export class AlertComponent {
 
+  changes = 0;
+
   constructor(
     private alertCtrl: AlertController
   ) { }
 
+  counter() {
+    this.changes++;
+    return Math.floor(this.changes / 2);
+  }
+
   async openAlert() {
     const alert = await this.alertCtrl.create({
       header: 'Hello',

angular/test/test-app/src/app/router-link-page/router-link-page.component.html

@@ -9,6 +9,7 @@
 
 <ion-content padding>
   <p>ngOnInit: <span id="ngOnInit">{{onInit}}</span></p>
+  <p>canGoBack: <span id="canGoBack">{{canGoBack}}</span></p>
   <p>ionViewWillEnter: <span id="ionViewWillEnter">{{willEnter}}</span></p>
   <p>ionViewDidEnter: <span id="ionViewDidEnter">{{didEnter}}</span></p>
   <p>ionViewWillLeave: <span id="ionViewWillLeave">{{willLeave}}</span></p>

angular/test/test-app/src/app/router-link-page/router-link-page.component.ts

@@ -1,4 +1,5 @@
 import { Component, OnInit, NgZone } from '@angular/core';
+import { IonRouterOutlet } from '@ionic/angular';
 
 @Component({
   selector: 'app-router-link-page',
@@ -11,20 +12,32 @@ export class RouterLinkPageComponent implements OnInit {
   didEnter = 0;
   willLeave = 0;
   didLeave = 0;
+  canGoBack: boolean = null;
+
+  constructor(
+    private ionRouterOutlet: IonRouterOutlet
+  ) {}
 
   ngOnInit() {
     NgZone.assertInAngularZone();
+    this.canGoBack = this.ionRouterOutlet.canGoBack();
     this.onInit++;
   }
 
   ionViewWillEnter() {
     if (this.onInit !== 1) {
       throw new Error('ngOnInit was not called');
     }
+    if (this.canGoBack !== this.ionRouterOutlet.canGoBack()) {
+      throw new Error('canGoBack() changed');
+    }
     NgZone.assertInAngularZone();
     this.willEnter++;
   }
   ionViewDidEnter() {
+    if (this.canGoBack !== this.ionRouterOutlet.canGoBack()) {
+      throw new Error('canGoBack() changed');
+    }
     NgZone.assertInAngularZone();
     this.didEnter++;
   }

core/scripts/swiper.rollup.config.js

@@ -7,8 +7,6 @@ export default {
     format: 'es'
   },
   plugins: [
-    resolve({
-      module: true
-    })
+    resolve()
   ]
 };