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

This commit is contained in:
Manu MA
2019-07-18 10:26:54 +02:00
committed by GitHub
parent 26e6d6f115
commit 97fec92365
10 changed files with 112 additions and 63 deletions

View File

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

View File

@ -31,7 +31,7 @@ export class StackController {
createView(ref: ComponentRef<any>, activatedRoute: ActivatedRoute): RouteView { createView(ref: ComponentRef<any>, activatedRoute: ActivatedRoute): RouteView {
const url = getUrl(this.router, activatedRoute); const url = getUrl(this.router, activatedRoute);
const element = (ref && ref.location && ref.location.nativeElement) as HTMLElement; 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 { return {
id: this.nextId++, id: this.nextId++,
stackId: computeStackId(this.tabsPrefix, url), stackId: computeStackId(this.tabsPrefix, url),
@ -90,7 +90,18 @@ export class StackController {
} }
} }
const reused = this.views.includes(enteringView);
const views = this.insertView(enteringView, direction); const views = this.insertView(enteringView, direction);
// 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.wait(() => {
return this.transition(enteringView, leavingView, animation, this.canGoBack(1), false) return this.transition(enteringView, leavingView, animation, this.canGoBack(1), false)
.then(() => cleanupAsync(enteringView, views, viewsSnapshot, this.location)) .then(() => cleanupAsync(enteringView, views, viewsSnapshot, this.location))
@ -101,6 +112,7 @@ export class StackController {
tabSwitch tabSwitch
})); }));
}); });
});
} }
canGoBack(deep: number, stackId = this.getActiveStackId()): boolean { canGoBack(deep: number, stackId = this.getActiveStackId()): boolean {
@ -199,11 +211,11 @@ export class StackController {
if (enteringView) { if (enteringView) {
enteringView.ref.changeDetectorRef.reattach(); 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 // reduce jank during the page transition
// if (leavingView) { if (leavingView) {
// leavingView.ref.changeDetectorRef.detach(); leavingView.ref.changeDetectorRef.detach();
// } }
const enteringEl = enteringView ? enteringView.element : undefined; const enteringEl = enteringView ? enteringView.element : undefined;
const leavingEl = leavingView ? leavingView.element : undefined; const leavingEl = leavingView ? leavingView.element : undefined;
const containerEl = this.containerEl; const containerEl = this.containerEl;
@ -213,13 +225,13 @@ export class StackController {
containerEl.appendChild(enteringEl); containerEl.appendChild(enteringEl);
} }
return this.zone.runOutsideAngular(() => containerEl.commit(enteringEl, leavingEl, { return containerEl.commit(enteringEl, leavingEl, {
deepWait: true, deepWait: true,
duration: direction === undefined ? 0 : undefined, duration: direction === undefined ? 0 : undefined,
direction, direction,
showGoBack, showGoBack,
progressAnimation progressAnimation
})); });
} }
return Promise.resolve(false); return Promise.resolve(false);
} }

View File

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

View File

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

View File

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

View File

@ -1,10 +1,10 @@
<ion-header> <ion-header>
<ion-toolbar> <ion-toolbar>
<ion-title> <ion-title>
Modal test Alert test
</ion-title> </ion-title>
</ion-toolbar> </ion-toolbar>
</ion-header> </ion-header>
<ion-content padding> <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> </ion-content>

View File

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

View File

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

View File

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

View File

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