Skip to content

feat(router): detach change detection on navigation #1507

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 12 commits into from
Jan 22, 2019
2 changes: 1 addition & 1 deletion e2e/modal-navigation-ng/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@
"mocha-junit-reporter": "^1.18.0",
"mocha-multi": "^1.0.1",
"nativescript-dev-appium": "next",
"nativescript-dev-typescript": "~0.7.8",
"nativescript-dev-typescript": "next",
"nativescript-dev-webpack": "next",
"typescript": "~3.1.1"
},
Expand Down
2 changes: 1 addition & 1 deletion e2e/nested-router-tab-view/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@
"mocha-junit-reporter": "^1.18.0",
"mocha-multi": "^1.0.1",
"nativescript-dev-appium": "next",
"nativescript-dev-typescript": "~0.7.4",
"nativescript-dev-typescript": "next",
"nativescript-dev-webpack": "next",
"typescript": "~3.1.1"
},
Expand Down
2 changes: 1 addition & 1 deletion e2e/renderer/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@
"mocha-junit-reporter": "~1.17.0",
"mocha-multi": "~1.0.0",
"nativescript-dev-appium": "next",
"nativescript-dev-typescript": "~0.7.1",
"nativescript-dev-typescript": "next",
"nativescript-dev-webpack": "next",
"tslib": "^1.7.1",
"typescript": "~3.1.1"
Expand Down
2 changes: 1 addition & 1 deletion e2e/router-tab-view/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@
"mocha-junit-reporter": "~1.17.0",
"mocha-multi": "~1.0.0",
"nativescript-dev-appium": "next",
"nativescript-dev-typescript": "~0.6.0",
"nativescript-dev-typescript": "next",
"typescript": "~3.1.1"
},
"scripts": {
Expand Down
16 changes: 14 additions & 2 deletions e2e/router/app/first/first.component.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { Component, OnInit, OnDestroy, OnChanges } from "@angular/core";
import { Component, OnInit, OnDestroy, OnChanges, DoCheck } from "@angular/core";
import { ActivatedRoute, Router, Route } from "@angular/router";
import { Location } from "@angular/common";
import { RouterExtensions } from "nativescript-angular/router";
Expand All @@ -18,11 +18,14 @@ import { Page } from "tns-core-modules/ui/page";
<Button text="GO TO C-LESS LAZY" automationText="GO TO C-LESS LAZY" [nsRouterLink]="['/lazy','nest','more']"></Button>

<Button text="BACK" automationText="BACK" (tap)="goBack()"></Button>
<Button text="RESET" automationText="RESET" (tap)="reset()"></Button>
<Label [text]="message"></Label>
<Label [text]="'CHECK: ' + doCheckCount"></Label>
</StackLayout>`
})
export class FirstComponent implements OnInit, OnDestroy {
export class FirstComponent implements OnInit, OnDestroy, DoCheck {
public message: string = "";
public doCheckCount: number = 0;
constructor(private routerExt: RouterExtensions, page: Page) {
console.log("FirstComponent - constructor() page: " + page);
}
Expand All @@ -35,6 +38,15 @@ export class FirstComponent implements OnInit, OnDestroy {
console.log("FirstComponent - ngOnDestroy()");
}

ngDoCheck() {
this.doCheckCount++;
console.log("FirstComponent - ngDoCheck(): " + this.doCheckCount);
}

reset() {
this.doCheckCount = 0;
}

goBack() {
this.message = "";
if (this.routerExt.canGoBack()) {
Expand Down
31 changes: 31 additions & 0 deletions e2e/router/e2e/router.e2e-spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -406,6 +406,37 @@ describe("Navigate to componentless lazy module route", () => {
});
});

describe("Simple navigate and back should trigger only one CD on FirstComponent", () => {
let driver: AppiumDriver;

before(async () => {
driver = await createDriver();
await driver.resetApp();
});

it("should find First", async () => {
await assureFirstComponent(driver);
});

it("should reset counter", async () => {
await findAndClick(driver, "RESET");
await driver.waitForElement("CHECK: 1");
});

it("should navigate to Second(1)/master", async () => {
await findAndClick(driver, "GO TO SECOND");

await assureSecondComponent(driver, 1);
await assureNestedMasterComponent(driver);
});

it("should navigate back to First", async () => {
await goBack(driver);
await assureFirstComponent(driver);
await driver.waitForElement("CHECK: 2");
});
});

async function assureFirstComponent(driver: AppiumDriver) {
await driver.findElementByAutomationText("FirstComponent");
}
Expand Down
23 changes: 13 additions & 10 deletions e2e/router/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,10 @@
"readme": "NativeScript Application",
"repository": "<fill-your-repository-here>",
"nativescript": {
"id": "org.nativescript.router"
"id": "org.nativescript.router",
"tns-android": {
"version": "5.1.0"
}
},
"dependencies": {
"@angular/animations": "~7.2.0",
Expand All @@ -24,10 +27,10 @@
"zone.js": "^0.8.4"
},
"devDependencies": {
"@ngtools/webpack": "~7.2.0",
"@angular/compiler-cli": "~7.2.0",
"@types/chai": "^4.0.2",
"@types/mocha": "^5.2.5",
"@ngtools/webpack": "~7.2.0",
"@types/chai": "~4.1.7",
"@types/mocha": "~5.2.5",
"@types/node": "^10.12.12",
"babel-traverse": "6.25.0",
"babel-types": "6.25.0",
Expand All @@ -36,14 +39,14 @@
"chai-as-promised": "~7.1.1",
"colors": "^1.1.2",
"lazy": "1.0.11",
"mocha": "~3.5.0",
"mocha-junit-reporter": "^1.13.0",
"mocha-multi": "^0.11.0",
"nativescript-dev-appium": "next",
"nativescript-dev-typescript": "~0.4.0",
"mocha": "~5.2.0",
"mocha-junit-reporter": "~1.18.0",
"mocha-multi": "~1.0.1",
"nativescript-dev-appium": "^4.0.10-2019-01-16-01",
"nativescript-dev-typescript": "^0.7.10-next-2019-01-07-183215-03",
"nativescript-dev-webpack": "next",
"tslib": "^1.7.1",
"typescript": "~3.1.1"
"typescript": "^3.1.6"
},
"scripts": {
"e2e": "tsc -p e2e && mocha --opts ../config/mocha.opts --recursive e2e --appiumCapsLocation ../config/appium.capabilities.json",
Expand Down
1 change: 1 addition & 0 deletions e2e/router/references.d.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
/// <reference path="./node_modules/tns-core-modules/tns-core-modules.d.ts" /> Needed for autocompletion and compilation.
7 changes: 7 additions & 0 deletions e2e/router/tsconfig.tns.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"extends": "./tsconfig",
"compilerOptions": {
"module": "es2015",
"moduleResolution": "node"
}
}
6 changes: 6 additions & 0 deletions nativescript-angular/router/page-router-outlet.ts
Original file line number Diff line number Diff line change
Expand Up @@ -242,6 +242,9 @@ export class PageRouterOutlet implements OnDestroy { // tslint:disable-line:dire
log(`PageRouterOutlet.detach() - ${routeToString(this._activatedRoute)}`);
}

// Detach from ChangeDetection
this.activated.hostView.detach();

const component = this.activated;
this.activated = null;
this._activatedRoute = null;
Expand All @@ -257,6 +260,9 @@ export class PageRouterOutlet implements OnDestroy { // tslint:disable-line:dire
}

this.activated = ref;

// reattach to ChangeDetection
this.activated.hostView.reattach();
this._activatedRoute = activatedRoute;
this.markActivatedRoute(activatedRoute);

Expand Down
2 changes: 1 addition & 1 deletion tests/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@
"karma-mocha-reporter": "2.2.5",
"karma-nativescript-launcher": "0.4.0",
"mocha": "5.2.0",
"nativescript-dev-typescript": "~0.7.8",
"nativescript-dev-typescript": "next",
"babel-traverse": "6.8.0",
"babel-types": "6.8.1",
"babylon": "6.8.0",
Expand Down